mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-12 19:44:48 +01:00
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.
This commit is contained in:
1 parent
c3d63db6e7
commit
3a74e67140
3 files changed
+22
-45
No files matched your search
+8
-12
@@ -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: <N>`) AND the PR delivers
|
||||
the full spec (acceptance criteria checked off per `/ship`'s existing
|
||||
plan-completion gate), `/ship` adds `Closes #<N>` 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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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 `<body>` below
|
||||
**Sanitized body to the archive.** If auto-redact fired, the `<body>` 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:-}
|
||||
---
|
||||
|
||||
# <title>
|
||||
@@ -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.
|
||||
@@ -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/);
|
||||
|
||||
Reference in new issue
Block a user