25 KiB
Phase 5: Tech Debt - Research
Researched: 2026-03-30 Domain: React wizard refactoring — conditional rendering, registry-driven UI, test quality Confidence: HIGH
<user_constraints>
User Constraints (from CONTEXT.md)
Locked Decisions
scriptTargets filtering (TECH-01)
- ReviewStep conditionally renders output blocks based on
state.deployment.scriptTargets— blocks are unmounted entirely when their target is deselected (not CSS-hidden) - Intune maps to: intuneInstall + intuneDetection blocks
- RMM maps to: rmmScript block
- rclone.conf OutputBlock is always shown regardless of scriptTargets
- ZIP bundle also respects scriptTargets — only files for selected targets are included (not always 4)
- Edge case: if both targets are deselected, only rclone.conf is shown and ZIP contains only rclone.conf (no blocking or warning needed)
Back button (TECH-02)
- Must preserve all form data (wizard state is in the store — navigation does not clear it)
- Back dispatches
SET_STEP(2)to return to DeploymentStep
Registry structure (TECH-03)
- Enrich
BACKEND_REGISTRYtoRecord<BackendType, { displayName: string; description: string; fields: FieldDef[] }> - All consumers of
BACKEND_REGISTRY[type]that access field arrays must update toBACKEND_REGISTRY[type].fields BackendSelectionStepderives the card list fromObject.entries(BACKEND_REGISTRY)— no hardcodedBACKENDSarray- Display metadata stays in the registry entry so Phase 6 additions auto-surface with zero additional code
- Existing test assertions on card text ('Azure Blob Storage', etc.) remain valid — source of truth just moves
Dead export removal (TECH-04)
- Remove
BackendFormValues<T>export fromsrc/schemas/index.ts(lines 27-28) - Verify no TypeScript errors arise after removal (no consumers expected)
act() warnings (TECH-05)
- Claude's Discretion — fix the act() warnings in
BackendSelectionStep.test.tsxusing appropriate testing-library patterns
Claude's Discretion
- Back button (TECH-02): placement and styling left to planner
- act() warnings (TECH-05): fix approach left to planner
Deferred Ideas (OUT OF SCOPE)
None — discussion stayed within phase scope. </user_constraints>
<phase_requirements>
Phase Requirements
| ID | Description | Research Support |
|---|---|---|
| TECH-01 | User sees only the script output blocks matching their selected deployment targets (Intune and/or RMM) in ReviewStep | Conditional render pattern; ZIP filter array construction; existing state.deployment.scriptTargets already tracks selections |
| TECH-02 | User can navigate back from ReviewStep using an explicit Back button | SET_STEP(2) action exists; DeploymentStep Back button is the established style pattern |
| TECH-03 | Backend list in BackendSelectionStep is automatically derived from BACKEND_REGISTRY keys (no hardcoded list) | Registry shape change; three consumers need .fields access update; BackendSelectionStep replaces BACKENDS constant |
| TECH-04 | Dead BackendFormValues<T> export is removed from src/schemas/index.ts |
Lines 27-28 confirmed; no consumers found via code audit |
| TECH-05 | BackendSelectionStep test suite runs without act() warnings |
Warnings confirmed in live test run; root cause is fireEvent triggering async state updates without act() wrapping |
| </phase_requirements> |
Summary
This phase is purely internal code quality work — no new user-facing features except the Back button on ReviewStep and the scriptTargets filtering. All changes are surgical: modify four files (registry.ts, schemas/index.ts, BackendSelectionStep.tsx, ReviewStep.tsx) and update three test files to keep assertions aligned.
The codebase is in clean, idiomatic React/TypeScript with Vitest + Testing Library. Patterns are already established: conditional render (not CSS-hide) for content that has no preserved state, store dispatch for navigation, registry-driven loops for field rendering. Phase 5 applies these patterns to the remaining places that weren't wired up in v1.0.
The act() warnings are real and confirmed to be in both BackendSelectionStep.test.tsx (warnings about BackendSelectionStep and WizardProvider components) and ReviewStep.test.tsx (warnings about OutputBlock). The root cause is fireEvent calling React Hook Form's handleSubmit, which triggers async state updates and setTimeout (in OutputBlock.handleCopy) without act() wrapping. The fix is userEvent (which wraps in act()) or explicit act() calls around async sequences.
Primary recommendation: Address each requirement as a focused, independent task. TECH-03 (registry enrichment) has the most ripple effect — do it first so TECH-01 and the BackendSelectionStep work have the final registry shape available.
Standard Stack
Core (already in use — no new installs)
| Library | Version | Purpose | Why Standard |
|---|---|---|---|
| React | 18.3.1 | UI rendering, conditional render, hooks | Project baseline |
| TypeScript | 5.5.3 | Type safety for registry shape change | Project baseline |
| Vitest | 4.1.1 | Test runner | Project baseline |
| @testing-library/react | 16.3.2 | Component testing | Project baseline |
| @testing-library/user-event | 14.6.1 | User interaction simulation (wraps in act()) |
Already installed; fixes act() warnings |
Installation: No new packages required. @testing-library/user-event is already in devDependencies.
Architecture Patterns
Recommended File Touch Map
src/
├── schemas/
│ ├── registry.ts # TECH-03: enrich shape + add displayName/description to each entry
│ └── index.ts # TECH-03: update .fields access in buildZodSchema; TECH-04: remove BackendFormValues<T>
├── components/wizard/
│ ├── BackendSelectionStep.tsx # TECH-03: replace BACKENDS const with Object.entries(BACKEND_REGISTRY)
│ ├── BackendSelectionStep.test.tsx # TECH-05: replace fireEvent with userEvent; TECH-03: verify assertions still pass
│ ├── RemoteConfigStep.tsx # TECH-03: update BACKEND_REGISTRY[backendType] → BACKEND_REGISTRY[backendType].fields
│ └── ReviewStep.tsx # TECH-01: conditional renders; TECH-02: Back button
└── (no new files needed)
Pattern 1: Conditional Render by scriptTargets (TECH-01)
What: Unmount OutputBlocks whose target is not in state.deployment.scriptTargets
When to use: When the content has no local state worth preserving between show/hide
Why not CSS-hide: OutputBlock has no internal state that must survive deselection (unlike AzureAuthToggle's field values)
// In ReviewStep — derive booleans from state
const showIntune = state.deployment.scriptTargets.includes('intune');
const showRmm = state.deployment.scriptTargets.includes('rmm');
// Conditional render (NOT CSS hidden)
{showIntune && (
<OutputBlock label="Intune Install Script" content={intuneInstall} filename="intune-install.ps1" disabled={!acknowledged} />
)}
{showIntune && (
<OutputBlock label="Intune Detection Script" content={intuneDetection} filename="intune-detection.ps1" disabled={!acknowledged} />
)}
{showRmm && (
<OutputBlock label="RMM Script" content={rmmScript} filename="rmm-script.ps1" disabled={!acknowledged} />
)}
Pattern 2: ZIP Filter by scriptTargets (TECH-01)
What: Build the files array dynamically before passing to downloadZip()
async function handleDownloadZip() {
const files: { name: string; content: string }[] = [
{ name: 'rclone.conf', content: rcloneConf }, // always included
];
if (showIntune) {
files.push({ name: 'intune-install.ps1', content: intuneInstall });
files.push({ name: 'intune-detection.ps1', content: intuneDetection });
}
if (showRmm) {
files.push({ name: 'rmm-script.ps1', content: rmmScript });
}
await downloadZip(files, 'rclone-deployment.zip');
}
Pattern 3: Back Button in ReviewStep (TECH-02)
What: Follow DeploymentStep's exact button layout — Back + action button side-by-side at the bottom
Established reference: DeploymentStep.tsx lines 90-105 — flex gap-3 mt-6 container, px-4 py-2 text-sm border border-gray-300 rounded-md for Back, blue variant for primary
// Bottom of ReviewStep JSX, before closing </div>
<div className="flex gap-3 mt-6">
<button
type="button"
onClick={() => dispatch({ type: 'SET_STEP', payload: 2 })}
className="px-4 py-2 text-sm border border-gray-300 rounded-md hover:bg-gray-50"
>
Back
</button>
</div>
Note: dispatch is not currently destructured from useWizard() in ReviewStep — must add it alongside state.
Pattern 4: Registry Shape Enrichment (TECH-03)
What: Change BACKEND_REGISTRY value type from FieldDef[] to { displayName: string; description: string; fields: FieldDef[] }
// registry.ts — new shape
export const BACKEND_REGISTRY: Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }> = {
azureblob: {
displayName: 'Azure Blob Storage',
description: 'Microsoft Azure cloud storage',
fields: [ /* existing FieldDef array */ ],
},
s3: {
displayName: 'Amazon S3',
description: 'AWS Simple Storage Service',
fields: [ /* existing FieldDef array */ ],
},
's3-compatible': {
displayName: 'S3-Compatible',
description: 'Wasabi, MinIO, Cloudflare R2, and others',
fields: [ /* existing FieldDef array */ ],
},
};
Consumers that break and must be updated:
src/schemas/index.tsline 10:const fields = BACKEND_REGISTRY[backendType];→const fields = BACKEND_REGISTRY[backendType].fields;src/components/wizard/RemoteConfigStep.tsxline 58:BACKEND_REGISTRY.azureblob.find(...)→BACKEND_REGISTRY.azureblob.fields.find(...)src/components/wizard/RemoteConfigStep.tsxline 73:BACKEND_REGISTRY[backendType].map(...)→BACKEND_REGISTRY[backendType].fields.map(...)
BackendSelectionStep replacement:
// Replace hardcoded BACKENDS const entirely
// Object.entries preserves insertion order in V8 (azureblob first, consistent with WIZD-01 test)
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
<BackendCard
key={type}
name={entry.displayName}
description={entry.description}
selected={state.remote.backendType === type}
onClick={() => handleCardClick(type as BackendType)}
/>
))}
Pattern 5: Fixing act() Warnings (TECH-05)
Root cause confirmed in live test run:
BackendSelectionStep.test.tsx:fireEvent.click(azureButton)triggershandleSubmit→ async form validation →dispatchin store → state update inWizardProvider/BackendSelectionStepwithout act() wrappingReviewStep.test.tsx:fireEvent.click(copyButtons[0])triggershandleCopy→setCopied(true)→setTimeout(() => setCopied(false), 2000)— the timeout fires after test assertion, causing theOutputBlockwarning
Fix strategy for BackendSelectionStep.test.tsx:
Replace fireEvent with userEvent from @testing-library/user-event. userEvent.setup() + await user.click() wraps all state updates in act() automatically.
// Before (produces act() warnings)
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
fireEvent.click(azureButton);
// After (no act() warnings)
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
const user = userEvent.setup();
// ...
await user.click(azureButton);
Fix strategy for ReviewStep act() warnings (OutputBlock setTimeout): Two options (both valid, Claude's Discretion):
- Use
vi.useFakeTimers()inbeforeEachof the clipboard copy tests sosetTimeoutnever fires unexpectedly - Use
userEventfor click interactions (same as BackendSelectionStep fix)
Option 1 (fake timers) is more surgical for the OutputBlock case since the setTimeout is a 2-second "Copied!" display reset — it's not a test concern.
Anti-Patterns to Avoid
- CSS-hiding OutputBlocks: Tempting to add
hiddenclass based on scriptTargets, but decision is locked to unmount — conditional render is the correct approach. - Hardcoding the BACKENDS array after registry enrichment: The whole point of TECH-03 is
Object.entries(BACKEND_REGISTRY)— do not leave a parallel BACKENDS constant. - Wrapping every fireEvent in act(): The correct fix is userEvent, not wrapping
fireEventcalls manually — manualact()wrapping produces verbose, harder-to-maintain tests. - Removing useMemo calls for conditionally rendered content:
intuneInstall,intuneDetection,rmmScriptuseMemo calls can stay even if the block is hidden — the computation is cheap and the memoized value is used in the ZIP handler too.
Don't Hand-Roll
| Problem | Don't Build | Use Instead | Why |
|---|---|---|---|
| Act()-safe user interactions | Manual act() wrappers around fireEvent |
userEvent from @testing-library/user-event |
userEvent simulates real browser events and automatically wraps in act(); already installed |
| Fake timers for setTimeout | Custom timer management | vi.useFakeTimers() / vi.useRealTimers() |
Vitest built-in; handles all setTimeout/setInterval cleanly |
| Registry-driven UI ordering | Custom sort/filter logic | Object.entries() (V8 insertion order) |
Object.entries() on a Record<BackendType, ...> preserves declaration order — Azure first matches WIZD-01 requirement |
Common Pitfalls
Pitfall 1: Forgetting dispatch in ReviewStep
What goes wrong: ReviewStep currently only destructures state from useWizard(). Adding a Back button requires dispatch too. Easy to miss.
Why it happens: ReviewStep was read-only in v1.0 — no navigation actions needed.
How to avoid: Change const { state } = useWizard(); to const { state, dispatch } = useWizard(); at the top of ReviewStep.
Pitfall 2: registry.test.ts breaks after TECH-03
What goes wrong: registry.test.ts accesses BACKEND_REGISTRY[backend] and calls .length and .find() directly on the registry entry — these will fail after the shape change to { displayName, description, fields }.
Lines affected: Lines 14 (BACKEND_REGISTRY[backend].length), 19 (for (const field of BACKEND_REGISTRY[backend])), 37/43/50 (BACKEND_REGISTRY.azureblob.find(...), etc.)
How to avoid: Update registry.test.ts to access .fields — e.g., BACKEND_REGISTRY[backend].fields.length, BACKEND_REGISTRY[backend].fields.find(...).
Pitfall 3: ReviewStep.test.tsx asserts exactly 4 OutputBlocks / 4 downloads
What goes wrong: Tests DOWN-02, DOWN-03, DOWN-04, DOWN-05 use positional indexing (downloadButtons[1], downloadButtons[2], downloadButtons[3]) and expect(files).toHaveLength(4). After TECH-01, when scriptTargets is both (default), 4 blocks still render — but tests for individual downloads will break if called with scriptTargets that hide blocks.
Current status: Default INITIAL_STATE has scriptTargets: ['intune', 'rmm'] so existing tests still pass with all 4 blocks. Tests specifically for filtered rendering are new (Wave 0 stubs needed).
How to avoid: Existing ReviewStep tests can remain as-is (they use default state = both targets). Add new test cases for filtered scenarios (intune-only, rmm-only, neither) — these are new Wave 0 stubs.
Pitfall 4: BackendSelectionStep.test.tsx assertions on card text remain valid
What goes wrong: After TECH-03, tests check for 'Azure Blob Storage', 'Amazon S3', 'S3-Compatible' text. These strings move from the hardcoded BACKENDS array to BACKEND_REGISTRY[type].displayName. As long as displayName values match, tests pass without any change.
How to avoid: Use the exact same display strings when adding displayName to registry entries.
Pitfall 5: TypeScript error after removing BackendFormValues
What goes wrong: If any file imports BackendFormValues from src/schemas/index.ts, removing it causes a TS compile error.
Evidence: Code audit found no consumers — BackendFormValues only appears in src/schemas/index.ts lines 27-28. But TypeScript compile check is the definitive verification step.
How to avoid: After removal, run tsc --noEmit or npm run build to confirm zero errors.
Pitfall 6: userEvent requires setup() call
What goes wrong: userEvent.click() was called directly in older versions of @testing-library/user-event. In v14 (installed: 14.6.1), the correct API is const user = userEvent.setup() then await user.click().
How to avoid: Always use userEvent.setup() pattern — not the legacy userEvent.click() shorthand.
Code Examples
Verified pattern: userEvent v14 API
// Source: @testing-library/user-event v14 official API
import userEvent from '@testing-library/user-event';
describe('BackendSelectionStep', () => {
it('clicking a backend card dispatches SET_BACKEND_TYPE and SET_STEP', async () => {
const user = userEvent.setup();
renderStep();
const nameInput = screen.getByRole('textbox');
await user.type(nameInput, 'my-remote');
const azureButton = screen.getAllByRole('button').find(b => b.textContent?.includes('Azure Blob Storage'))!;
await user.click(azureButton);
expect(screen.queryByRole('alert')).toBeNull();
});
});
Verified pattern: vi.useFakeTimers() for setTimeout-based act() warnings
// Source: Vitest official docs — timer mocking
import { beforeEach, afterEach, vi } from 'vitest';
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it('clicking Copy calls clipboard.writeText', async () => {
renderStep();
const checkbox = screen.getByRole('checkbox');
fireEvent.click(checkbox);
const copyButtons = screen.getAllByText('Copy');
fireEvent.click(copyButtons[0]);
expect(navigator.clipboard.writeText).toHaveBeenCalledTimes(1);
// setTimeout(() => setCopied(false)) never fires during test — no act() warning
});
Verified pattern: Object.entries() on enriched registry
// Source: MDN Object.entries() — insertion order preserved for string keys
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
<BackendCard
key={type}
name={entry.displayName}
description={entry.description}
selected={state.remote.backendType === type}
onClick={() => handleCardClick(type as BackendType)}
/>
))}
State of the Art
| Old Approach | Current Approach | When Changed | Impact |
|---|---|---|---|
fireEvent for all test interactions |
userEvent.setup() + await user.click() |
@testing-library/user-event v14 | Eliminates act() warnings from async state updates triggered by simulated events |
BACKEND_REGISTRY: Record<BackendType, FieldDef[]> |
Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }> |
Phase 5 (TECH-03) | BackendSelectionStep no longer needs a parallel BACKENDS array; Phase 6 additions auto-surface |
Deprecated/outdated:
fireEventfor click interactions that trigger state updates: still works but causes act() warnings; replace with userEvent where state updates occur- Hardcoded
BACKENDSarray inBackendSelectionStep.tsx: dead after TECH-03; replaced byObject.entries(BACKEND_REGISTRY) BackendFormValues<T>type export: dead code — no consumers exist; removed in TECH-04
Open Questions
-
ReviewStep.test.tsx: DOWN-05 asserts
files.toHaveLength(4)— does this test need updating?- What we know: Default INITIAL_STATE has scriptTargets = ['intune', 'rmm'], so with default state the ZIP still gets 4 files.
- What's unclear: Should the TECH-01 implementation also add new test cases covering filtered scenarios (2 files for intune-only, etc.)?
- Recommendation: Keep the existing DOWN-05 test intact (it documents correct behavior when both targets are selected). Add new test cases in Wave 0 for filtered scenarios (intune-only, rmm-only, neither).
-
act() warnings in ReviewStep.test.tsx — fix in this phase or leave?
- What we know: TECH-05 requirement text says "BackendSelectionStep test suite" specifically. ReviewStep warnings are from OutputBlock's setTimeout.
- What's unclear: Whether the phase goal implicitly includes ReviewStep warnings given CONTEXT.md says "eliminate act() warnings from the BackendSelectionStep test suite."
- Recommendation: TECH-05 scope is BackendSelectionStep. Fix ReviewStep act() warnings as a bonus in the same task (low effort — add
vi.useFakeTimers()to the relevant ReviewStep tests). If time-constrained, BackendSelectionStep warnings are the acceptance criterion.
Validation Architecture
Test Framework
| Property | Value |
|---|---|
| Framework | Vitest 4.1.1 |
| Config file | vitest.config.ts (root) |
| Quick run command | npx vitest run src/components/wizard/BackendSelectionStep.test.tsx src/components/wizard/ReviewStep.test.tsx src/schemas/registry.test.ts |
| Full suite command | npx vitest run |
Phase Requirements → Test Map
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|---|---|---|---|---|
| TECH-01 | Intune blocks absent when scriptTargets excludes 'intune' | unit | npx vitest run src/components/wizard/ReviewStep.test.tsx |
❌ Wave 0 (new cases needed) |
| TECH-01 | RMM block absent when scriptTargets excludes 'rmm' | unit | npx vitest run src/components/wizard/ReviewStep.test.tsx |
❌ Wave 0 (new cases needed) |
| TECH-01 | ZIP contains only rclone.conf when both targets deselected | unit | npx vitest run src/components/wizard/ReviewStep.test.tsx |
❌ Wave 0 (new cases needed) |
| TECH-02 | Back button dispatches SET_STEP(2) | unit | npx vitest run src/components/wizard/ReviewStep.test.tsx |
❌ Wave 0 (new case needed) |
| TECH-03 | BackendSelectionStep renders cards from BACKEND_REGISTRY | unit | npx vitest run src/components/wizard/BackendSelectionStep.test.tsx |
✅ (existing assertions still valid) |
| TECH-03 | registry.test.ts field access via .fields | unit | npx vitest run src/schemas/registry.test.ts |
❌ Wave 0 (update existing) |
| TECH-04 | No TypeScript error after BackendFormValues removal | type-check | npx tsc --noEmit |
✅ (compile check) |
| TECH-05 | BackendSelectionStep test suite zero act() warnings | unit | npx vitest run src/components/wizard/BackendSelectionStep.test.tsx 2>&1 | grep -c "act(" |
✅ (existing tests, modified) |
Sampling Rate
- Per task commit:
npx vitest run(full suite — only 3.4s, fast enough for every commit) - Per wave merge:
npx vitest run+npx tsc --noEmit - Phase gate: Full suite green + zero TypeScript errors before
/gsd:verify-work
Wave 0 Gaps
src/components/wizard/ReviewStep.test.tsx— add cases for TECH-01: intune-only, rmm-only, neither (requires WizardProvider accepting initial state, or dispatching SET_DEPLOYMENT in test setup)src/components/wizard/ReviewStep.test.tsx— add case for TECH-02: Back button renders and dispatches SET_STEP(2)src/schemas/registry.test.ts— update field access to.fieldsafter TECH-03 shape change (existing tests break without this update)
Note on ReviewStep test state injection: Current renderStep() wraps with bare WizardProvider which uses INITIAL_STATE. To test filtered rendering, tests need a way to render with custom deployment state. Options: (a) export a WizardProvider variant accepting initialState prop, or (b) render and dispatch SET_DEPLOYMENT in test setup before assertions. Option (b) requires no code changes to the provider — preferred.
Sources
Primary (HIGH confidence)
- Direct code audit of
src/components/wizard/ReviewStep.tsx— confirmed no dispatch, no Back button, always 4 OutputBlocks - Direct code audit of
src/components/wizard/BackendSelectionStep.tsx— confirmed hardcoded BACKENDS array - Direct code audit of
src/schemas/registry.ts— confirmed current shape isRecord<BackendType, FieldDef[]> - Direct code audit of
src/schemas/index.ts— confirmed BackendFormValues at lines 27-28 with no consumers - Live test run (
npx vitest run) — confirmed act() warnings present in both BackendSelectionStep and ReviewStep test files, all 98 tests passing src/store/types.ts— confirmed SET_STEP action exists, scriptTargets in WizardState.deploymentpackage.json— confirmed @testing-library/user-event 14.6.1 already installed
Secondary (MEDIUM confidence)
- @testing-library/user-event v14 API:
userEvent.setup()pattern verified against installed version and established community usage
Tertiary (LOW confidence)
- None
Metadata
Confidence breakdown:
- Standard stack: HIGH — all libraries already in use, confirmed from package.json
- Architecture: HIGH — all patterns derived from direct code audit of existing files
- Pitfalls: HIGH — most pitfalls discovered by cross-referencing actual test files with planned changes
- act() fix: HIGH — userEvent.setup() pattern confirmed against installed v14.6.1
Research date: 2026-03-30 Valid until: 2026-05-30 (stable libraries — React 18, Vitest 4, userEvent 14)