/office-hours: design doc passes review without verifying premises against code (incident + 3 fixes)
- Dominant language
- TypeScript
- Stars
- 133k
- Forks
- 19.9k
- Avg merge
- 18h 46m
- Merged PRs (30d)
- 26
Description
## Incident
Used `/office-hours` to design an admin UI refactor for a Next.js + Supabase app
(content authoring tool, ~3 admin pages affected). Design doc reached **APPROVED**
status with a 9/10 newcomer-readability self-eval. The doc proposed a UI-only
refactor with explicit premise: "no schema migration needed".
Then ran `/codex consult` adversarial on the doc. Codex (which actually reads
files) returned **15 findings: 7 P1 critical + 8 P2 advisory**, invalidating
3 of the doc's 7 premises:
- Premise "no schema migration needed" — false. A DB trigger repopulates the
new canonical column from legacy columns whenever the new column is set
to NULL. The unpublish server action as designed is a silent no-op.
- Premise "legacy column is orphan" — false. Existing `actions.ts` still
inserts and updates the legacy column. The reconciliation work was
understated.
- Change "Move to Draft" button on lesson edit page — semantically wrong.
The doc explicitly keeps child entities as Ready, so the lesson returns
to **Ready state**, not Draft. The new button label recreates the exact
ambiguity the design was meant to eliminate.
Effort estimate moved from ~6-7h CC to ~10-13h CC after Codex pass.
The doc was 9/10 narrative-quality but 4/10 implementation-readiness.
The skill itself did NOT catch this gap — three structural Phase issues below.
## Root cause
Three Phase methodology gaps let me ship a doc that flew away from codebase reality:
### Gap 1 — Phase 1 Context Gathering doesn't require migration/RLS map
Phase 1 reads `CLAUDE.md`, `git log`, lists prior design docs. Does NOT require
mapping migrations, RLS policies, or DB triggers relevant to the area being
designed. Result: premises get formulated against memory/CLAUDE.md narrative,
not against actual schema + policies. For any refactor touching DB
visibility/state, this is the gap that produces "no schema change needed"
claims that turn out to be false.
### Gap 2 — Phase 5 Spec Review Loop subagent prompt is anti-rigor
The Spec Review Loop dispatches a subagent reviewer with this instruction
(from the skill's own prompt template):
> "Codebase paths to verify mentally (**do NOT read them yourself**, just
> check consistency within the doc)"
This is exactly the wrong direction. It forbids the one action that would
catch the doc-vs-reality drift. The reviewer can only flag internal
inconsistency (doc says X here and Y there), not external (doc says X
but the codebase says Z). In my incident the reviewer returned PASS
with quality 7/10 while Codex, reading the same files, found 7 P1 in the
same doc.
### Gap 3 — Phase 3 Premise Challenge asks user instead of code
Premise Challenge presents premises to the user via AskUserQuestion and
asks them to confirm/reject. But the user is rarely the source of truth
on existing codebase state — for "this DB column is orphan", "this RLS
policy hides X", "this trigger doesn't fire here", the source of truth
is the migrations/policies/types, not the user's recollection.
In my incident, the user confirmed all 7 premises because they trusted
my interpretation. But my interpretation was assumed, not verified. The
result was recursive trust without verification.
## Proposed fixes
### Fix 1 — Phase 1: add "Codebase Surface Map" step
Before Phase 2 questioning, add explicit step that maps the area being
designed:
- For schema-touching designs: list relevant migrations (most recent N
touching affected tables), read them, summarize triggers + constraints
+ check columns.
- For visibility/auth-touching designs: read RLS policies for affected tables.
- For server-action designs: list and read existing server actions in scope.
Output: a "Codebase Surface" section the design doc, which any premise can
be checked against.
### Fix 2 — Phase 5 Spec Review Loop: REVERSE the subagent prompt instruction
Change subagent prompt from:
> "do NOT read them yourself, just check consistency within the doc"
to:
> "READ the referenced source files. Verify each premise in the design
> doc against the actual code. Flag any premise that the code contradicts.
> Pay special attention to: schema columns that the doc claims are unused,
> RLS policies the doc claims gate something, triggers that might fire on
> the proposed UPDATEs, server actions still writing to columns the doc
> claims are orphan."
This turns the Spec Review from a doc-internal-consistency check into a
doc-vs-reality check, which is the actual quality dimension that matters.
### Fix 3 — Phase 3 Premise Challenge: verify premise candidates against code first
Before asking the user to confirm a premise, the model should verify it in
code:
- Premise about schema (column orphan, table unused): grep for write paths.
- Premise about RLS (policy hides X): read the policy.
- Premise about behavior (X doesn't fire when Y): trace the code path or
run a small probe.
Only premises that survive code verification get presented to the user.
The user confirms the **scope** decision (do we want this?), not the
**factual** assertion (is this true today?). Today the two are conflated
in a single AskUserQuestion.
## Why this matters
The current skill produces design docs that look 9/10 to a newcomer but
ship with implementation-time landmines that double the effort estimate
post-Codex. For solo developers without a cross-model habit (Codex consult,
adversarial review), these landmines explode during PR review or production
incidents.
The fix is mechanical: tighten Phase 1 + Phase 3 + Phase 5 to require
code-grounded premises. Cost ~30 min more CC per session, saves ~6h of
refactor that the wrong premises cause downstream.
Happy to PR the SKILL.md changes if useful — let me know preferred direction
(separate PRs per Phase fix, or one PR with all three).
---
Reported by raffoz, captured during a real `/office-hours` → `/codex consult` flow.
Contributor guide
Assessment
This issue has not been assessed yet.