diff --git a/bin/gstack-review-scope b/bin/gstack-review-scope index 35f5a3910..2e1833acb 100755 --- a/bin/gstack-review-scope +++ b/bin/gstack-review-scope @@ -43,7 +43,8 @@ const HELP = `gstack-review-scope — finding scope for /ship's review loop (doc summary --since [--out ] the invocation's passes from this branch's review log: REVIEW_LOOP: passes=… blocking_found=… deferred=… noted=… (the ship receipt's --review-loop input) -Defaults: --policy from .gstack/ship-policy.json review_scope (frozen); --findings - reads stdin; --branch the current branch. +Defaults: --policy from the base branch's .gstack/ship-policy.json review_scope (frozen); an unresolvable base refuses + (POLICY_SOURCE_UNAVAILABLE) unless --policy is passed; --findings - reads stdin; --branch the current branch. ${exitTableHelp()}`; function flag(args: string[], name: string): string | undefined { diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index baaae166c..3a6676712 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4237,12 +4237,18 @@ output again. **Meaning.** `gstack-review-scope` (the finding scope of /ship's review loop, [ship-policy.md#review-scope](ship-policy.md#review-scope)) was given input it cannot classify: the findings were not a JSON array of objects, `--pass` or -`--policy` was outside its values, or a pass 2+ call came without -`--range-base`. Nothing was classified, deferred or summarized. +`--policy` was outside its values, a pass 2+ call came without +`--range-base`, `--range-base` was not a commit id (7 to 64 hex digits), or +`summary --since` matched no review-log row of this branch written via /ship +(so no `REVIEW_LOOP:` line could be derived, and the ship receipt's +`--review-loop` input was not written). Nothing was classified, deferred or +summarized. **Fix.** Pass the Step 9.3 findings as a JSON array (`--findings ` or stdin), `--pass ` from the invocation record, and from pass 2 on -`--range-base `, the HEAD the previous pass saved when it started. +`--range-base `, the HEAD the previous pass saved when it started, as a +hex commit id. For `summary`, check `REVIEW_SINCE` against the invocation +record's start time and that each pass was finished with `via:"ship"`. ### `REVIEW_SCOPE_RANGE` @@ -4261,12 +4267,15 @@ rebased; /ship's own rule is to merge the base in, never rebase a reviewed head. **Meaning.** `gstack-ship-receipt carry` could not carry the PR's /ship receipt to HEAD: the PR body has no valid receipt, the receipt's head is not an ancestor -of HEAD (a rebase or force-push dropped it), or git could not compare the two -product diffs. The receipt in the body is unchanged and still names the old -head, so `gstack-ship-queue ready` refuses `QUEUE_RECEIPT_STALE`. +of HEAD (a rebase or force-push dropped it), the working tree has uncommitted +or untracked changes (a carry certifies a commit, never a tree that differs from +it), or git could not compare the two product diffs or read the working tree's +status. The receipt in the body is unchanged and still names the old head, so +`gstack-ship-queue ready` refuses `QUEUE_RECEIPT_STALE`. **Fix.** Keep the reviewed head in HEAD's history (merge the base in instead of -rebasing), then rerun `gstack-ship-receipt carry --pr `. When the history is +rebasing), commit or stash every working-tree change, then rerun +`gstack-ship-receipt carry --pr ` at the new head. When the history is already rewritten, run /ship on the new head. diff --git a/test/fixtures/workflow-required-steps.json b/test/fixtures/workflow-required-steps.json index 50c3ddfc8..5c65ce590 100644 --- a/test/fixtures/workflow-required-steps.json +++ b/test/fixtures/workflow-required-steps.json @@ -91,10 +91,11 @@ "#### 1. Parse outputs", "#### 2. Validate severity", "#### 3. Identify and merge", - "#### 4. Apply specialist confidence gates", - "#### 5. Score and present specialists", - "#### 6. Save specialist activity", - "#### 7. Hand off to Fix-First" + "#### 4. Lead judgment", + "#### 5. Apply specialist confidence gates", + "#### 6. Score and present specialists", + "#### 7. Save specialist activity", + "#### 8. Hand off to Fix-First" ] }, "ship": { @@ -148,10 +149,11 @@ "#### 1. Parse outputs", "#### 2. Validate severity", "#### 3. Identify and merge", - "#### 4. Apply specialist confidence gates", - "#### 5. Score and present specialists", - "#### 6. Save specialist activity", - "#### 7. Hand off to Fix-First", + "#### 4. Lead judgment", + "#### 5. Apply specialist confidence gates", + "#### 6. Score and present specialists", + "#### 7. Save specialist activity", + "#### 8. Hand off to Fix-First", "### Step 9.2.1: Exploratory QA (before Fix-First)", "### Step 9.3: Cross-review finding dedup", "## Step 9.4: Fix-First and persistence" diff --git a/test/review-loop-budget-wording.test.ts b/test/review-loop-budget-wording.test.ts index 1b6894dea..0f1682630 100644 --- a/test/review-loop-budget-wording.test.ts +++ b/test/review-loop-budget-wording.test.ts @@ -8,12 +8,16 @@ import { describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { generateReviewArmy } from '../scripts/resolvers/review-army'; +import { HOST_PATHS } from '../scripts/resolvers/types'; const ROOT = path.resolve(import.meta.dir, '..'); const read = (p: string) => fs.readFileSync(path.join(ROOT, p), 'utf8').replace(/\s+/g, ' '); describe('review-loop budget wording', () => { test('ship Step 9.4 budgets by blocker / should / nit and the repeat decisions carry the two stop rules', () => { + const authored = read('ship/sections/review-army.md.tmpl'); + expect(authored).toContain('a **blocker** is the intended case gone wrong'); const army = read('ship/sections/review-army.md'); expect(army).toContain('a **blocker** is the intended case gone wrong'); expect(army).toContain('a **should** is a real bug a caller will hit, in the intended case'); @@ -34,8 +38,9 @@ describe('review-loop budget wording', () => { }); test('both review armies apply lead judgment before a merged finding counts, and every specialist prompt offers the cause anchor', () => { - for (const p of ['ship/sections/review-army.md', 'review/sections/review-army.md']) { - const text = read(p); + const rendered = (['ship', 'review'] as const).map(skillName => + generateReviewArmy({ skillName, tmplPath: '', host: 'claude', paths: HOST_PATHS.claude }).replace(/\s+/g, ' ')); + for (const text of [...rendered, read('ship/sections/review-army.md'), read('review/sections/review-army.md')]) { const judgment = text.slice(text.indexOf('#### 4. Lead judgment'), text.indexOf('#### 5. Apply specialist confidence gates')); expect(judgment.length).toBeGreaterThan(200); for (const filter of ['Nit gravity', 'Hypothetical, not actual', 'Structure for a case that does not exist', 'A different design is not a defect', 'Missing context']) expect(judgment).toContain(filter); diff --git a/test/shared-libs-rendering.test.ts b/test/shared-libs-rendering.test.ts index 8373470aa..a75cd71fa 100644 --- a/test/shared-libs-rendering.test.ts +++ b/test/shared-libs-rendering.test.ts @@ -89,7 +89,7 @@ describe('shared-code skill distribution', () => { } if (host.name !== 'codex' && !host.suppressedResolvers?.includes('REVIEW_ARMY')) { const stages = ['#### 1. Parse outputs', '#### 2. Validate severity', '#### 3. Identify and merge', - '#### 4. Apply specialist confidence gates', '#### 5. Score and present specialists']; + '#### 4. Lead judgment', '#### 5. Apply specialist confidence gates', '#### 6. Score and present specialists']; const positions = stages.map(stage => parent.indexOf(stage)); expect(positions.every(position => position >= 0)).toBe(true); expect(positions).toEqual([...positions].sort((a, b) => a - b));