mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix: pin the renumbered review-army stages and point the wording test at product code
CI at c3e27fc failed three free-tier tests, all this branch's own: the
workflow-required-steps fixture and shared-libs-rendering still pinned the
pre-Lead-judgment stage numbers, and test-of-test-ratchet counted
review-loop-budget-wording.test.ts as test-only code because it read only
generated markdown. The fixture and rendering test now pin stages 1-8, and the
wording test renders both armies through generateReviewArmy and reads the
authored ship template as well as the generated sections.
Pass-2 maintainability notes folded in: docs/troubleshooting.md names the two
new RECEIPT_CARRY_REFUSED causes (dirty tree, unreadable status) and the two new
REVIEW_SCOPE_INPUT causes (non-hex --range-base, summary with no matching
pass); gstack-review-scope's help states that classify reads the base branch's
policy and refuses an unresolvable base unless --policy is passed.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
c3e27fc2b8
commit
5bace0f5ba
5 files changed
+36
-19
No files matched your search
@@ -43,7 +43,8 @@ const HELP = `gstack-review-scope — finding scope for /ship's review loop (doc
|
||||
summary --since <iso> [--out <file>] 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 {
|
||||
|
||||
+16
-7
@@ -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 <file>` or
|
||||
stdin), `--pass <n>` from the invocation record, and from pass 2 on
|
||||
`--range-base <sha>`, the HEAD the previous pass saved when it started.
|
||||
`--range-base <sha>`, 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"`.
|
||||
|
||||
<a id="review-scope-range"></a>
|
||||
### `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 <n>`. When the history is
|
||||
rebasing), commit or stash every working-tree change, then rerun
|
||||
`gstack-ship-receipt carry --pr <n>` at the new head. When the history is
|
||||
already rewritten, run /ship on the new head.
|
||||
|
||||
<a id="receipt-carry-product-changed"></a>
|
||||
|
||||
+10
-8
@@ -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"
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in new issue
Block a user