ui/_docs/LESSONS.md

# Lessons

Short, actionable retros from past sessions. Newest at top. Ring buffer of
the last 15 entries. The `autodev` orchestrator surfaces the top 3 entries
on every invocation.

Categories: estimation · architecture · testing · dependencies · tooling · process

---

- [2026-05-13] [testing] When inserting a new control (button, input, link)
  into an existing DOM row or region that already holds other controls, audit
  the test corpus *before* the commit for non-disambiguated selectors
  targeting that region (`querySelector('button')`, `getByRole('button')`
  without `name`/`text`, indexed `querySelectorAll('button')[0]`) and either
  update them with disambiguating text/role/name in the same affordance
  commit or give the new control a stable `data-testid` — otherwise the new
  control silently rebinds existing assertions to the wrong element and the
  tests ship green-but-meaningless, as cycle 4's `destructive_ux.test.tsx`
  did when the AZ-512 ✎ button became the new first button in the class-row
  action cell.
  Source: _docs/06_metrics/retro_2026-05-13_cycle4.md

- [2026-05-13] [testing] When a new test mounts a container component
  end-to-end, run it once with the project's default test fixtures only (no
  per-test override) and explicitly name any natural crashes ("`users.map is
  not a function`") in the batch report as "Pre-existing bug noted" — never
  silently apply a local fixture workaround without recording the latent
  drift, because each silent workaround hides a source-vs-fixture mismatch
  that future authors will re-encounter as a "mysterious test setup", and
  cycle 4's `tests/admin_class_edit.test.tsx` was the second cycle to
  surface one through this route.
  Source: _docs/06_metrics/retro_2026-05-13_cycle4.md

- [2026-05-13] [process] When the user explicitly overrides a
  spec-conservative cycle-defer decision (the AZ-512 Option B authorization:
  "implement now, write mocks for backend"), the autodev MUST preserve every
  downstream gate that the conservative path would have enforced — re-record
  the override rationale in the leftover entry, keep the cross-workspace
  deploy gate visible at Step 16, mark the carried tickets distinctly from
  cycle-internal carries, and surface the override as a first-class
  retrospective trend ("Cycles where user overrode a spec-conservative
  default") — so the operating cost of the override stays measurable and
  the user's downstream visibility is unchanged from the conservative path.
  Source: _docs/06_metrics/retro_2026-05-13_cycle4.md

- [2026-05-13] [process] When a task spec defines a Cross-Workspace Verification
  BLOCKING gate and the user skips the choice prompt, the autodev MUST default
  to the most conservative spec-aligned option (Option A: file prerequisite
  ticket on the sibling workspace, park the task in `backlog/`) — never invent
  a workaround that bypasses the missing dependency, never silently ship a UI
  affordance against a non-existent endpoint, and always preserve the user's
  ability to override at the next invocation (AZ-512 → AZ-513 pattern).
  Source: _docs/06_metrics/retro_2026-05-13_cycle3.md

- [2026-05-13] [architecture] Introducing a module-scoped state guard in
  production source (e.g., a top-level `let bootstrapInflight: Promise | null
  = null` for React 18 StrictMode dedupe) requires the same batch to ship 4
  coupled changes — (a) a test-only reset hook re-exported via the public
  barrel (STC-ARCH-01 compliance), (b) an `afterEach` reset in
  `tests/setup.ts`, (c) a defensive default-fixture invariant check (e.g.,
  MSW handler must seed required nullable fields the helper consumes), (d) a
  planned ripple swap in handler mocks for any HTTP method or wire-shape
  change — skipping any one costs a separate test-stabilization loop, as
  AZ-510's ~4-attempt arc demonstrated.
  Source: _docs/06_metrics/retro_2026-05-13_cycle3.md

- [2026-05-13] [process] Track "user-action backlog at cycle close" as a
  first-class retrospective metric (count of leftover items broken down by
  manual-third-party / cross-workspace-prerequisite / cross-workspace-deploy
  / push-pending categories) — backlog grew monotonically 0 → 3 → 7 across
  cycles 1-3 and that accumulation is a process-shape signal, not noise;
  surfacing it makes the cost of conservative-path defaults visible per
  cycle and creates pressure for an explicit drain mechanism.
  Source: _docs/06_metrics/retro_2026-05-13_cycle3.md

- [2026-05-12] [process] When externalizing a committed API key, always follow
  the 4-step rotation discipline: (a) extract to env-var via a service module
  so unit tests can stub it, (b) add a literal-scan static gate (STC-SECx)
  against the rotated value as defense-in-depth, (c) document in
  `.env.example` using the established `<your-...>` placeholder convention,
  (d) leave the actual key revocation as a manual deliverable AC with
  evidence-attachment requirement — never assume the static gate alone
  neutralizes the leaked credential.
  Source: _docs/06_metrics/retro_2026-05-12_cycle2.md

- [2026-05-12] [dependencies] When `bun audit` reports advisories on a
  transitive dep that direct `bun update <dep>` does not clear (because
  nested copies persist under sibling tools, e.g.
  `vitest/node_modules/<dep>`), use `package.json` `"overrides"` to floor
  the resolution AND clean reinstall (`rm -rf node_modules bun.lock &&
  bun install`) — a direct update alone cannot displace nested copies, and
  Bun honors the npm-compatible `overrides` field exactly as npm does.
  Source: _docs/06_metrics/retro_2026-05-12_cycle2.md

- [2026-05-12] [tooling] When the autodev orchestrator delegates to a
  sub-skill that ends in a HIGH-severity blocking gate (e.g. security audit
  FAIL → user picks "fix inline"), capture the inline-fix sub-step results
  as a separate batch report (`batch_NN_report.md`) — not as an extension
  of the prior batch — so the cycle metrics correctly attribute findings,
  ACs, and complexity to the work boundary that produced them.
  Source: _docs/06_metrics/retro_2026-05-12_cycle2.md

- [2026-05-12] [architecture] When adding an architecture gate (STC-ARCH-*),
  extend the existing single-script dispatcher with a new `--mode` flag
  instead of forking a second script; same walker, same comment-skip, same
  test harness — half the drift surface.
  Source: _docs/06_metrics/retro_2026-05-12.md

- [2026-05-12] [architecture] When a barrel re-export causes a runtime
  circular import, treat the carve-out as a structural exemption documented
  in five coupled places (barrel, consumer, script regex, layout doc, gate
  test), not as a re-order hack — the exemption clears when the deeper
  structural fix lands and never silently drifts in the meantime.
  Source: _docs/06_metrics/retro_2026-05-12.md

- [2026-05-12] [process] When autodev detects state ↔ working-tree
  disagreement on session resume (`state.cycle` / `state.step` ≠ on-disk
  artifact set), ALWAYS surface as a Choose block before resuming work —
  never silently merge or restart; the rule in `state.md` "trust folders
  over state file" worked end-to-end on the AZ-486 resume.
  Source: _docs/06_metrics/retro_2026-05-12.md

---

## 2026-05-11 — Don't replace `URL` via `vi.stubGlobal('URL', { ...URL, ... })`

When stubbing `URL.createObjectURL` / `URL.revokeObjectURL` for a JSDOM-backed
test, **patch the methods on the constructor directly**. Never do
`vi.stubGlobal('URL', { ...URL, createObjectURL })` — the spread copies only
own enumerable properties of the `URL` *function object*, not its prototype, so
the global `URL` becomes a plain object. `new URL(...)` then throws / returns
garbage in MSW handlers and the SPA's API helper, and the test silently sees
"no fetch was made" instead of the real failure. Pattern in
`tests/upload_size_cap.test.tsx` is the canonical fix.

---