garrytan / garrytan/gstack

/office-hours: design doc passes review without verifying premises against code (incident + 3 fixes)

Open
#1,644 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.