From 3a74e671403533acbf49f20527a44b83f03c8953 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 2 Oct 2026 23:52:51 +0000 Subject: [PATCH] spec: plain Phase 3 gate, drop duplicates, dead telemetry and review IDs (s07 H4, M1, M2, M3, M4, M5, M6) - H4: Standard template heading no longer names nonexistent --bug/--feature/--refactor flags. - M1: Phase 3 read-code-first gate at normal volume, constraint and reason kept. - M2: drop the in-phase repeat of the read-first rule (Rules recap stays). - M3: drop the duplicate audit-sink paragraph. - M4: drop TTHW telemetry block and its never-set frontmatter keys. - M5/M6: drop plan-review finding IDs from Phase 5 headings and the /ship handoff. W7a pins: spec-template-invariants:314 (Mandatory/MUST) becomes a Phase 3 section check for the read-evidence-before-asking constraint; :203 matches 'raw spec must not be persisted' across line wraps so no duplicate paragraph is needed to satisfy it. Template-only commit: generated SKILL.md is expected red until the regeneration commit. --- spec/SKILL.md.tmpl | 20 ++++++------- spec/sections/gate-and-file.md.tmpl | 41 +++++++-------------------- test/spec-template-invariants.test.ts | 6 ++-- 3 files changed, 22 insertions(+), 45 deletions(-) diff --git a/spec/SKILL.md.tmpl b/spec/SKILL.md.tmpl index fdfff41df..bfb59007f 100644 --- a/spec/SKILL.md.tmpl +++ b/spec/SKILL.md.tmpl @@ -144,13 +144,12 @@ Ask until you can answer: Do NOT proceed until scope is locked. -### Phase 3: Technical Interrogation (HARD requirement: read code first) +### Phase 3: Technical Interrogation (read code first) -**Mandatory:** Before asking ANY Phase 3 question, you MUST read at least one -piece of evidence from the codebase via Grep, Glob, or Read. This is the magical -moment for the user: they see you grounded in their actual code, not generic -checklists. Do NOT skip. Do NOT ask "what file should I look at?" first — find -it yourself. +Before asking any Phase 3 question, read at least one piece of evidence from the +codebase via Grep, Glob, or Read, and cite it. This is the magical moment for the +user: they see you grounded in their actual code, not generic checklists. Find the +relevant file yourself rather than asking which one to look at. Mapping the user's request to evidence: @@ -176,9 +175,6 @@ Then ask about whichever categories apply (skip ones that clearly don't): - **Infrastructure** — IaC changes, secrets, cost impact - **Testing** — how to test at each layer, regression risk -Don't ask questions you can answer by reading the code. Read first, then ask -the questions whose answers aren't in the code. - ### Phase 4: Draft Review Present a full draft issue and ask: **"Does this accurately capture what you want? @@ -334,7 +330,7 @@ this? Even "revert the PR" is worth stating explicitly. ## Issue Structure Templates -### Standard Issues (default; also used for `--bug`, `--feature`, `--refactor` framings) +### Standard Issues (default; bug, feature and refactor framings adapt within it) ``` ## Context @@ -482,8 +478,8 @@ Add to the standard template: a `/spec` archive (frontmatter `spec_issue_number: `) AND the PR delivers the full spec (acceptance criteria checked off per `/ship`'s existing plan-completion gate), `/ship` adds `Closes #` to the PR body so merging - auto-closes the source issue. Conditional — partial PRs do NOT auto-close - (codex F4). Branch-name inference is NOT used (codex F3). + auto-closes the source issue. Conditional — partial PRs do not auto-close, and + the source issue is never inferred from the branch name. --- diff --git a/spec/sections/gate-and-file.md.tmpl b/spec/sections/gate-and-file.md.tmpl index 9bd0ec7f8..3c48f745b 100644 --- a/spec/sections/gate-and-file.md.tmpl +++ b/spec/sections/gate-and-file.md.tmpl @@ -84,12 +84,6 @@ Missing/broken CLI, authentication failure, timeout, refusal, nonzero exit, inva Max 3 dispatches total. If still <7 after iter 3, AskUserQuestion same options. - - -**Audit-sink invariant:** When the redaction gate fires, the raw spec must NOT -be persisted anywhere downstream (no archive write, no transcript log). The -`spec-quality-gate-secret-sink.test.ts` enforces this. - ### Phase 5: File the Spec (+ optional --execute) Produce the final spec using the structure defined below. Use `--audit` to @@ -148,7 +142,7 @@ is consumed by `/ship` for auto-close. {{REDACT_INVOCATION_BLOCK:pre-archive:brief}} -**D2 — sanitized body to the archive.** If auto-redact fired, the `` below +**Sanitized body to the archive.** If auto-redact fired, the `` below MUST be the sanitized body (`$REDACT_FILE`), not the original draft — one body for all sinks. The user's on-disk source draft keeps the original. @@ -173,8 +167,6 @@ spec_branch: $(git branch --show-current 2>/dev/null || echo unknown) spec_plan_mode: ${GSTACK_PLAN_MODE:-unset} spec_executed: ${WILL_EXECUTE:-false} spec_worktree_path: -ttfc_ms: ${TTFC_MS:-} -tthw_ms: ${TTHW_MS:-} --- # @@ -189,14 +181,14 @@ The PID suffix and atomic rename prevent collisions when two `/spec` invocations run in the same second. **Sync default:** `/specs/` is auto-excluded from the artifacts-sync allowlist — -archives stay local unless the user opts in via `--sync-archive` (privacy default -per codex review). If `--sync-archive` is passed, append `/specs/<archive_name>` +archives stay local unless the user opts in via `--sync-archive` (privacy default). +If `--sync-archive` is passed, append `/specs/<archive_name>` to the artifacts-sync allowlist (or symlink into the synced dir, depending on implementation). #### Spawn the agent (`--execute` path only) -**E2 dirty-worktree gate:** +**Dirty-worktree gate:** ```bash DIRTY=$(git status --porcelain 2>/dev/null) @@ -209,7 +201,7 @@ If `$DIRTY` is non-empty, AskUserQuestion: - B) Stash and restore (auto-stash now, restore after spawn returns) - C) Cancel spawn (stop here; issue stays filed, archive stays written) -**E2 TOCTOU re-check (F1):** After the user answers, IMMEDIATELY re-run +**TOCTOU re-check:** After the user answers, IMMEDIATELY re-run `git status --porcelain` before any worktree operation. If state diverged from the answer, re-prompt the AskUserQuestion. The check must happen INSIDE the spawn workflow, not be cached from earlier. @@ -222,21 +214,21 @@ git stash push -u -m "spec-execute-auto-$$" # untracked YES, ignored NO STASH_REF="spec-execute-auto-$$" ``` -F2 stash policy: `-u` includes untracked; we deliberately do NOT use `--all` +Stash policy: `-u` includes untracked; we deliberately do NOT use `--all` because ignored files (build artifacts, .env caches) are usually local-by-design and should stay in the current worktree. If C: print "Cancelled spawn. Issue filed: $ISSUE_URL, archive: $ARCHIVE_PATH." Exit /spec. -**F4 SHA pin:** Capture the exact SHA AFTER the final dirty check. Use this +**SHA pin:** Capture the exact SHA AFTER the final dirty check. Use this SHA (not "HEAD") for the worktree: ```bash PIN_SHA=$(git rev-parse HEAD) ``` -**F5 unique branch + worktree path:** Suffix with `$$` to avoid concurrent +**Unique branch + worktree path:** Suffix with `$$` to avoid concurrent collisions: ```bash @@ -245,7 +237,7 @@ SPAWN_PATH="${WORKTREE_PARENT:-../worktrees}/${SLUG_TITLE}-$$" mkdir -p "$(dirname "$SPAWN_PATH")" ``` -**D16 mandatory final-confirm gate:** AskUserQuestion: "Spawn agent now? Last +**Final-confirm gate (required):** AskUserQuestion: "Spawn agent now? Last chance to revise the spec." Options: A) Spawn. B) Cancel (issue stays filed, archive stays written). @@ -272,22 +264,9 @@ echo "Follow with: cd $SPAWN_PATH && claude --resume" Update archive frontmatter with `spec_worktree_path: $SPAWN_PATH` and `spec_executed: true` (atomic re-write). -**F3 stash restore safety (when B path was chosen):** Do NOT auto-restore inline +**Stash restore safety (when B path was chosen):** Do NOT auto-restore inline — the spawned agent may take hours. Instead print: "Stash preserved as `$STASH_REF`. Restore later with `git stash list` then `git stash apply stash^{/$STASH_REF}`. Before restore, re-run `git status` to make sure your worktree is clean." Do NOT drop the stash; user owns it. -#### TTHW telemetry (DX11/F7) - -Capture timestamps at three checkpoints, write to telemetry envelope at /spec -exit: - -- `T_PHASE1_START` — Phase 1 first AskUserQuestion or first text emit -- `T_FIRST_CITATION` — first file/symbol reference in Phase 3 prose -- `T_FILE_OR_SPAWN` — issue filed OR agent spawned, whichever ends Phase 5 - -Append the captured timestamps to the local analytics line that the preamble's -end-of-skill telemetry write emits, as `ttfc_ms` (Phase 1 → first citation) and -`tthw_ms` (Phase 1 → file/spawn) JSON fields. Surfacing the aggregates in -`/retro` is a separate follow-up. diff --git a/test/spec-template-invariants.test.ts b/test/spec-template-invariants.test.ts index f135a77f3..9380492e1 100644 --- a/test/spec-template-invariants.test.ts +++ b/test/spec-template-invariants.test.ts @@ -200,7 +200,7 @@ describe('/spec redaction at every sink (scan-at-sink, carved: gate-and-file sec describe('/spec quality gate secret-sink invariant (carved: gate-and-file section)', () => { test('declares "raw spec must NOT be persisted" when the scan BLOCKS', () => { - expect(SEC_TMPL).toMatch(/raw spec must NOT[\s\S]*be persisted/i); + expect(SEC_TMPL).toMatch(/raw spec must not\s+be\s+persisted/i); }); test('BLOCK path stops before dispatch/archive/file', () => { expect(SEC_TMPL).toMatch(/no archive write, no transcript log, no outside\s*\n?\s*dispatch/i); @@ -311,7 +311,9 @@ describe('/spec plan-mode-aware Phase 5 (DX7/DX11/F1, carved: gate-and-file sect describe('/spec Phase 3 hard-grep with fallback', () => { test('Phase 3 mandates reading evidence before asking', () => { - expect(TMPL).toMatch(/Mandatory:[\s\S]*MUST read at least one[\s\S]*evidence/i); + const phase3 = TMPL.slice(TMPL.indexOf('### Phase 3:'), TMPL.indexOf('### Phase 4:')); + expect(phase3).toMatch(/before asking[\s\S]{0,40}question/i); + expect(phase3).toMatch(/read at least one\s+piece of evidence/i); }); test('project-level fallback prose for prompts with no concrete file', () => { expect(TMPL).toMatch(/Project-level prompt/);