From 26dc71d586774c4a0d47652ef16af31de6903c08 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 6 Oct 2026 00:19:36 +0000 Subject: [PATCH] fix(skills): resolve judge-cited ambiguities in /qa and /plan-eng-review - tasks JSONL keeps the raw branch as GIT_BRANCH instead of overwriting gstack-slug's sanitized BRANCH used for artifact names - the Codex outside-voice block runs for ready or unverified, matching its routing rule - plan-eng startup names the preamble's own setup prompts; the cross-project question waits for Step 0 - /qa 8c: a fix that applies but leaves the finding reproducing gets one revised fix, else it is reverted; browser verification names its before screenshot; the exploratory loop drops its duplicate 3-step summary --- plan-ceo-review/sections/review-sections.md | 8 ++++---- plan-design-review/sections/review-sections.md | 6 +++--- plan-devex-review/sections/review-sections.md | 6 +++--- plan-eng-review/SKILL.md | 2 +- plan-eng-review/SKILL.md.tmpl | 2 +- plan-eng-review/sections/review-sections.md | 8 ++++---- qa-only/sections/exploratory.md | 3 --- qa/SKILL.md | 5 +++-- qa/SKILL.md.tmpl | 5 +++-- qa/sections/browser-verify.md | 3 ++- qa/sections/browser-verify.md.tmpl | 3 ++- qa/sections/exploratory.md | 3 --- scripts/resolvers/outside-voice-steps.ts | 2 +- scripts/resolvers/qa.ts | 3 --- scripts/resolvers/tasks-section.ts | 6 +++--- 15 files changed, 30 insertions(+), 35 deletions(-) diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index f67c9b62f..68c800aed 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -471,7 +471,7 @@ THE PLAN: **If `CODEX_MODE: ready` (or `unverified`) — run Codex:** -Run this block only for `ready`, in the one foreground Bash call described below. +Run this block for `ready` or `unverified`, in the one foreground Bash call described below. Its opening harness guard rechecks the fresh shell: exit 78 uses the same Native fallback below, never a replacement provider. Finish termination before fallback and consume only completed output. Use private temporary paths, with no background jobs. @@ -794,13 +794,13 @@ backslashes serialize cleanly — never use hand-rolled `echo` / `printf`. ```bash GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" -BRANCH=$(~/.claude/skills/gstack/bin/gstack-slug --get BRANCH 2>/dev/null) SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) TASKS_DIR="$GSTACK_STATE_ROOT/projects/${SLUG:-unknown}" mkdir -p "$TASKS_DIR" TASKS_FILE="$TASKS_DIR/tasks-ceo-review-$(date +%Y%m%d-%H%M%S).jsonl" COMMIT=$(git rev-parse HEAD 2>/dev/null || echo unknown) -BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) +# The task record keeps the raw git branch; artifact filenames keep gstack-slug's sanitized BRANCH. +GIT_BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" # Repeat ONE jq invocation per task identified during this review. @@ -811,7 +811,7 @@ RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" jq -nc \ --arg phase 'ceo-review' \ --arg run_id "$RUN_ID" \ - --arg branch "$BRANCH" \ + --arg branch "$GIT_BRANCH" \ --arg commit "$COMMIT" \ --arg id "$TASK_ID" \ --arg priority "$PRIORITY" \ diff --git a/plan-design-review/sections/review-sections.md b/plan-design-review/sections/review-sections.md index 90ef2321d..103ffcf51 100644 --- a/plan-design-review/sections/review-sections.md +++ b/plan-design-review/sections/review-sections.md @@ -335,13 +335,13 @@ backslashes serialize cleanly — never use hand-rolled `echo` / `printf`. ```bash GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" -BRANCH=$(~/.claude/skills/gstack/bin/gstack-slug --get BRANCH 2>/dev/null) SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) TASKS_DIR="$GSTACK_STATE_ROOT/projects/${SLUG:-unknown}" mkdir -p "$TASKS_DIR" TASKS_FILE="$TASKS_DIR/tasks-design-review-$(date +%Y%m%d-%H%M%S).jsonl" COMMIT=$(git rev-parse HEAD 2>/dev/null || echo unknown) -BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) +# The task record keeps the raw git branch; artifact filenames keep gstack-slug's sanitized BRANCH. +GIT_BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" # Repeat ONE jq invocation per task identified during this review. @@ -352,7 +352,7 @@ RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" jq -nc \ --arg phase 'design-review' \ --arg run_id "$RUN_ID" \ - --arg branch "$BRANCH" \ + --arg branch "$GIT_BRANCH" \ --arg commit "$COMMIT" \ --arg id "$TASK_ID" \ --arg priority "$PRIORITY" \ diff --git a/plan-devex-review/sections/review-sections.md b/plan-devex-review/sections/review-sections.md index 29cd1aa7c..dbb475dd9 100644 --- a/plan-devex-review/sections/review-sections.md +++ b/plan-devex-review/sections/review-sections.md @@ -709,13 +709,13 @@ backslashes serialize cleanly — never use hand-rolled `echo` / `printf`. ```bash GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" -BRANCH=$(~/.claude/skills/gstack/bin/gstack-slug --get BRANCH 2>/dev/null) SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) TASKS_DIR="$GSTACK_STATE_ROOT/projects/${SLUG:-unknown}" mkdir -p "$TASKS_DIR" TASKS_FILE="$TASKS_DIR/tasks-devex-review-$(date +%Y%m%d-%H%M%S).jsonl" COMMIT=$(git rev-parse HEAD 2>/dev/null || echo unknown) -BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) +# The task record keeps the raw git branch; artifact filenames keep gstack-slug's sanitized BRANCH. +GIT_BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" # Repeat ONE jq invocation per task identified during this review. @@ -726,7 +726,7 @@ RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" jq -nc \ --arg phase 'devex-review' \ --arg run_id "$RUN_ID" \ - --arg branch "$BRANCH" \ + --arg branch "$GIT_BRANCH" \ --arg commit "$COMMIT" \ --arg id "$TASK_ID" \ --arg priority "$PRIORITY" \ diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index 552f92c44..191693e8e 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -65,7 +65,7 @@ After target selection, use the preamble's full decision brief, transport and co **Format precedence:** Copy required command, output and question formats exactly. Apply Voice to newly composed prose. **Startup sequence** (after target selection): -1. Run the Preamble command and its startup instructions (Context Recovery and setup questions). Defer Operational Self-Improvement, Telemetry and Plan Status Footer to finish; format/transport rules apply throughout. +1. Run the Preamble command and its startup instructions (Context Recovery and the preamble's own one-time setup prompts; the Prior Learnings cross-project question waits for Step 0's Review preparation). Defer Operational Self-Improvement, Telemetry and Plan Status Footer to finish; format/transport rules apply throughout. 2. Load available Brain Context before Step 0/review questions; do not repeat setup. 3. Check web-research readiness at **Web research runs in Aside**. 4. Run **Design Doc Check**, then **Prerequisite Skill Offer**. diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index 0f0d612d1..0dcfaa234 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -63,7 +63,7 @@ After target selection, use the preamble's full decision brief, transport and co **Format precedence:** Copy required command, output and question formats exactly. Apply Voice to newly composed prose. **Startup sequence** (after target selection): -1. Run the Preamble command and its startup instructions (Context Recovery and setup questions). Defer Operational Self-Improvement, Telemetry and Plan Status Footer to finish; format/transport rules apply throughout. +1. Run the Preamble command and its startup instructions (Context Recovery and the preamble's own one-time setup prompts; the Prior Learnings cross-project question waits for Step 0's Review preparation). Defer Operational Self-Improvement, Telemetry and Plan Status Footer to finish; format/transport rules apply throughout. 2. Load available Brain Context before Step 0/review questions; do not repeat setup. 3. Check web-research readiness at **Web research runs in Aside**. 4. Run **Design Doc Check**, then **Prerequisite Skill Offer**. diff --git a/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index fab5731c8..6a35b9c0b 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -913,7 +913,7 @@ THE PLAN: **If `CODEX_MODE: ready` (or `unverified`) — run Codex:** -Run this block only for `ready`, in the one foreground Bash call described below. +Run this block for `ready` or `unverified`, in the one foreground Bash call described below. Its opening harness guard rechecks the fresh shell: exit 78 uses the same Native fallback below, never a replacement provider. Finish termination before fallback and consume only completed output. Use private temporary paths, with no background jobs. @@ -1181,13 +1181,13 @@ backslashes serialize cleanly — never use hand-rolled `echo` / `printf`. ```bash GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" -BRANCH=$(~/.claude/skills/gstack/bin/gstack-slug --get BRANCH 2>/dev/null) SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) TASKS_DIR="$GSTACK_STATE_ROOT/projects/${SLUG:-unknown}" mkdir -p "$TASKS_DIR" TASKS_FILE="$TASKS_DIR/tasks-eng-review-$(date +%Y%m%d-%H%M%S).jsonl" COMMIT=$(git rev-parse HEAD 2>/dev/null || echo unknown) -BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) +# The task record keeps the raw git branch; artifact filenames keep gstack-slug's sanitized BRANCH. +GIT_BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" # Repeat ONE jq invocation per task identified during this review. @@ -1198,7 +1198,7 @@ RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" jq -nc \ --arg phase 'eng-review' \ --arg run_id "$RUN_ID" \ - --arg branch "$BRANCH" \ + --arg branch "$GIT_BRANCH" \ --arg commit "$COMMIT" \ --arg id "$TASK_ID" \ --arg priority "$PRIORITY" \ diff --git a/qa-only/sections/exploratory.md b/qa-only/sections/exploratory.md index cdc580b25..9322d695a 100644 --- a/qa-only/sections/exploratory.md +++ b/qa-only/sections/exploratory.md @@ -56,9 +56,6 @@ Never reset DEADLINE_FILE/bypass DEADLINE_TOOL. Expiry or an invalid/missing DEA Each probe is one native command/interaction plus checks, excluding bookkeeping. Never batch probes. -How one probe works: (1) if bounded, check the clock and choose the next command; -(2) publish a checkpoint naming it (functional: `capture --after` publishes it); -(3) run exactly that command via DEADLINE_TOOL or EVIDENCE_TOOL, read its result, repeat. 1. First demonstrate success: output AND durable effects. Guard if bounded; await completion. 2. **Decide whether another probe is needed.** If bounded, run `bun DEADLINE_TOOL status DEADLINE_FILE`. diff --git a/qa/SKILL.md b/qa/SKILL.md index 92a2dfed9..8570a96f1 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -662,7 +662,8 @@ Read the surrounding source and make the **minimal fix**. No unrelated refactors ### 8c. Re-test Re-run the regression, original failing probe and adjacent happy path. Inspect each -final state; acceptance alone cannot verify a worker repair. Failed/unavailable rechecks stay unresolved. +final state; acceptance alone cannot verify a worker repair. Failed/unavailable rechecks stay unresolved: +try one revised minimal fix, else classify it reverted. For browser defects only: @@ -684,7 +685,7 @@ repairs and valid red regressions/evidence uncommitted; tell the user what remai - **verified**: passed 8c (native regression when available); disclose missing test coverage - **best-effort**: fix applied but couldn't fully verify (e.g., needs auth state, external service) -- **reverted**: regression detected → undo only this run's repair (revert its commit if already committed), retain the valid regression/evidence, and mark the issue "deferred". Never discard user changes. +- **reverted**: regression detected or finding unresolved → undo only this run's repair (revert its commit if already committed), retain the valid regression/evidence, and mark the issue "deferred". Never discard user changes. ### 8e.5. Regression Test record diff --git a/qa/SKILL.md.tmpl b/qa/SKILL.md.tmpl index 22030876d..5b56d11ad 100644 --- a/qa/SKILL.md.tmpl +++ b/qa/SKILL.md.tmpl @@ -208,7 +208,8 @@ Read the surrounding source and make the **minimal fix**. No unrelated refactors ### 8c. Re-test Re-run the regression, original failing probe and adjacent happy path. Inspect each -final state; acceptance alone cannot verify a worker repair. Failed/unavailable rechecks stay unresolved. +final state; acceptance alone cannot verify a worker repair. Failed/unavailable rechecks stay unresolved: +try one revised minimal fix, else classify it reverted. For browser defects only: @@ -228,7 +229,7 @@ repairs and valid red regressions/evidence uncommitted; tell the user what remai - **verified**: passed 8c (native regression when available); disclose missing test coverage - **best-effort**: fix applied but couldn't fully verify (e.g., needs auth state, external service) -- **reverted**: regression detected → undo only this run's repair (revert its commit if already committed), retain the valid regression/evidence, and mark the issue "deferred". Never discard user changes. +- **reverted**: regression detected or finding unresolved → undo only this run's repair (revert its commit if already committed), retain the valid regression/evidence, and mark the issue "deferred". Never discard user changes. ### 8e.5. Regression Test record diff --git a/qa/sections/browser-verify.md b/qa/sections/browser-verify.md index f7add469d..b0be1de92 100644 --- a/qa/sections/browser-verify.md +++ b/qa/sections/browser-verify.md @@ -3,7 +3,8 @@ # Browser repair verification Use this section only for a browser defect. Re-run the original interaction and an -adjacent happy path. The Phase 5 evidence is the before; capture the after now. +adjacent happy path. The Phase 5 evidence is the before (`issue-NNN-result.jpg` for an +interactive defect, the annotated `issue-NNN.png` for a static one); capture the after now. Use the Phase 3 read/flow script in qa-patterns with `flow = true` for interaction bugs; set its actions and waits to the original reproduction. For static defects use `flow = false`. diff --git a/qa/sections/browser-verify.md.tmpl b/qa/sections/browser-verify.md.tmpl index 6cfbdbe55..8d90c8fb9 100644 --- a/qa/sections/browser-verify.md.tmpl +++ b/qa/sections/browser-verify.md.tmpl @@ -1,7 +1,8 @@ # Browser repair verification Use this section only for a browser defect. Re-run the original interaction and an -adjacent happy path. The Phase 5 evidence is the before; capture the after now. +adjacent happy path. The Phase 5 evidence is the before (`issue-NNN-result.jpg` for an +interactive defect, the annotated `issue-NNN.png` for a static one); capture the after now. Use the Phase 3 read/flow script in qa-patterns with `flow = true` for interaction bugs; set its actions and waits to the original reproduction. For static defects use `flow = false`. diff --git a/qa/sections/exploratory.md b/qa/sections/exploratory.md index 906cdee43..0d309e6ca 100644 --- a/qa/sections/exploratory.md +++ b/qa/sections/exploratory.md @@ -50,9 +50,6 @@ Never reset DEADLINE_FILE/bypass DEADLINE_TOOL. Expiry or an invalid/missing DEA Each probe is one native command/interaction plus checks, excluding bookkeeping. Never batch probes. -How one probe works: (1) if bounded, check the clock and choose the next command; -(2) publish a checkpoint naming it (functional: `capture --after` publishes it); -(3) run exactly that command via DEADLINE_TOOL or EVIDENCE_TOOL, read its result, repeat. 1. First demonstrate success: output AND durable effects. Guard if bounded; await completion. 2. **Decide whether another probe is needed.** If bounded, run `bun DEADLINE_TOOL status DEADLINE_FILE`. diff --git a/scripts/resolvers/outside-voice-steps.ts b/scripts/resolvers/outside-voice-steps.ts index d489c4410..1121a400f 100644 --- a/scripts/resolvers/outside-voice-steps.ts +++ b/scripts/resolvers/outside-voice-steps.ts @@ -409,7 +409,7 @@ THE PLAN: function codexPlanReviewRun(ctx: TemplateContext, ceo: boolean, needsApprovalReadiness: boolean): string { return `**If \`CODEX_MODE: ready\` (or \`unverified\`) — run ${outsideVoiceFor(ctx).label}:** -${['plan-ceo-review', 'plan-eng-review'].includes(ctx.skillName) ? `Run this block only for \`ready\`, in the one foreground Bash call described below. +${['plan-ceo-review', 'plan-eng-review'].includes(ctx.skillName) ? `Run this block for \`ready\` or \`unverified\`, in the one foreground Bash call described below. Its opening harness guard rechecks the fresh shell: exit 78 uses the same Native fallback below, never a replacement provider. Finish termination before fallback and consume only completed output. Use private temporary paths, with no background jobs.` : `Run the selected backend in the one foreground Bash invocation described below. diff --git a/scripts/resolvers/qa.ts b/scripts/resolvers/qa.ts index b758f830f..0a09c5cbf 100644 --- a/scripts/resolvers/qa.ts +++ b/scripts/resolvers/qa.ts @@ -97,9 +97,6 @@ Never reset DEADLINE_FILE/bypass DEADLINE_TOOL. Expiry or an invalid/missing DEA Each probe is one native command/interaction plus checks, excluding bookkeeping. Never batch probes. -How one probe works: (1) if bounded, check the clock and choose the next command; -(2) publish a checkpoint naming it (functional: \`capture --after\` publishes it); -(3) run exactly that command via DEADLINE_TOOL or EVIDENCE_TOOL, read its result, repeat. 1. First demonstrate success: output AND durable effects. Guard if bounded; await completion. 2. **Decide whether another probe is needed.** If bounded, run \`bun DEADLINE_TOOL status DEADLINE_FILE\`. diff --git a/scripts/resolvers/tasks-section.ts b/scripts/resolvers/tasks-section.ts index 42981ee5a..cbe72c747 100644 --- a/scripts/resolvers/tasks-section.ts +++ b/scripts/resolvers/tasks-section.ts @@ -60,13 +60,13 @@ backslashes serialize cleanly — never use hand-rolled \`echo\` / \`printf\`. \`\`\`bash GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "\${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" -BRANCH=$(~/.claude/skills/gstack/bin/gstack-slug --get BRANCH 2>/dev/null) SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) TASKS_DIR="$GSTACK_STATE_ROOT/projects/\${SLUG:-unknown}" mkdir -p "$TASKS_DIR" TASKS_FILE="$TASKS_DIR/tasks-${phase}-$(date +%Y%m%d-%H%M%S).jsonl" COMMIT=$(git rev-parse HEAD 2>/dev/null || echo unknown) -BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) +# The task record keeps the raw git branch; artifact filenames keep gstack-slug's sanitized BRANCH. +GIT_BRANCH=$(git branch --show-current 2>/dev/null || echo unknown) RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" # Repeat ONE jq invocation per task identified during this review. @@ -77,7 +77,7 @@ RUN_ID="$(date -u +%Y%m%dT%H%M%SZ)-$$" jq -nc \\ --arg phase '${phase}' \\ --arg run_id "$RUN_ID" \\ - --arg branch "$BRANCH" \\ + --arg branch "$GIT_BRANCH" \\ --arg commit "$COMMIT" \\ --arg id "$TASK_ID" \\ --arg priority "$PRIORITY" \\