From de04a789c15dd40d471567ef7dd669642cc7be54 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 01:28:36 +0000 Subject: [PATCH] GSTA-258 C: GST-1129 spend-binding lines class money, --declare-risk on plan and receipt (only raises), GEVA-221 case Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-plan | 8 +- bin/gstack-ship-receipt | 7 +- changelog.d/gsta258-c-review-depth.md | 2 + docs/review-policy.md | 11 ++- lib/headless-artifacts.ts | 4 +- lib/review-depth.ts | 19 ++-- lib/review-risk.ts | 19 +++- lib/review-targeted.ts | 6 +- lib/ship-receipt-depth.ts | 12 ++- ship/sections/review-army.md | 4 +- ship/sections/review-army.md.tmpl | 4 +- test/review-declared-risk.test.ts | 95 +++++++++++++++++++ .../ship/gsta258-c-review-depth.json | 4 +- 13 files changed, 167 insertions(+), 28 deletions(-) create mode 100644 test/review-declared-risk.test.ts diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index b8bf4ebfb..fa224a4b1 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -36,6 +36,7 @@ import { platformReviewAt } from '../lib/review-platform'; import { loadPolicyAt } from '../lib/ship-policy-at'; import { classifyDiff, diffCommand, keepCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth'; import { classList, targetedBriefSha, targetedClasses, targetedPatchId } from '../lib/review-targeted'; +import { RISK_CLASSES, type RiskClass } from '../lib/review-risk'; import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check'; const HELP = `gstack-review-plan — the review depth of the next /ship step (docs/review-policy.md#review-depth) @@ -70,6 +71,8 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do gate-class edit plans that fix-check instead; a fix-check never earns another; any other row is refused (exit 3) --fix-checks fix-checks already run in this invocation (R65's lean loop: the owner's grant adds one only after the first) --pr the PR whose platform review counts under review.platform on (default: this branch's PR) + --declare-risk risk classes the lane declares for the diff (comma list of ${RISK_CLASSES.join(', ')}): they join + the detected classes and only raise the depth; RISK_DECLARED: prints both (GST-1129) --fix-check-prompt print the fix-check reviewer's prompt from --fix-list, --hunks, --tests and --callers files Mode (R65, the base policy's review key): lean (default) runs one reader: risk class gets the merged reader on review.risk_model @@ -121,6 +124,8 @@ function main(): number { const rangeBase = flag('--range-base'); if (has('--range-base') && (!rangeBase || rangeBase.startsWith('-'))) { process.stderr.write('gstack-review-plan: --range-base needs the fix range\'s base commit (this pass\'s PASS_HEAD)\n'); return EXIT.usage; } if (grant && !rangeBase) { process.stderr.write('gstack-review-plan: --grant needs --range-base (round 2\'s PASS_HEAD), which binds the grant to the stop it answers\n'); return EXIT.usage; } + const declared = list('--declare-risk'); + if (has('--declare-risk') && (!declared.length || declared.some(c => !(RISK_CLASSES as readonly string[]).includes(c)))) { process.stderr.write(`gstack-review-plan: --declare-risk ${flag('--declare-risk') ?? ''}: each class is one of ${RISK_CLASSES.join(', ')} (a declaration only raises the depth)\n`); return EXIT.usage; } const root = git(['rev-parse', '--show-toplevel'])?.trim(); if (!root) { process.stderr.write('gstack-review-plan: run it inside a git checkout\n'); return EXIT.fail; } process.chdir(root); @@ -200,7 +205,7 @@ function main(): number { const patch = collected.mergeBase ? targetedPatchId(process.cwd(), collected.mergeBase) : 'none'; const brief = targetedBriefSha(); const plan = planReview({ - ...collected, ...exclusionInput, ...riskPaths, shape: { panel: review.panel, readerModel: review.reader_model, platform: review.platform, head, platformReview: platform?.ok ? platform.review : null }, scope, gateTagged, targeted: { classes: targeted.classes, ran: !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch }, numbers: scanNumbers(kept(added)), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host', + ...collected, ...exclusionInput, ...riskPaths, declared: declared as RiskClass[], shape: { panel: review.panel, readerModel: review.reader_model, platform: review.platform, head, platformReview: platform?.ok ? platform.review : null }, scope, gateTagged, targeted: { classes: targeted.classes, ran: !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch }, numbers: scanNumbers(kept(added)), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host', criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'), history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below') && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate, pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines, granted: grant, fixChecks }, }); @@ -215,6 +220,7 @@ function main(): number { `TARGETED: classes=${classList(targeted.classes)} rules=${targeted.rules} brief=${brief} targeted_patch_id=${patch}${targeted.reasons.length ? ` reasons=${targeted.reasons.join('; ')}` : ''}`]; out.push(...excludeLines(exclusion, base)); out.push(`REVIEW_MODE: ${plan.mode} panel=${review.panel} reader_model=${review.reader_model} platform=${review.platform}`); + if (declared.length) out.push(`RISK_DECLARED: classes=${plan.declared.join(',')} detected=${plan.detected.join(',') || 'none'}`); if (platform) out.push(platform.ok ? `PLATFORM_REVIEW: ${platform.review.id} reviewer=${platform.review.reviewer} head=${platform.review.head}` : `PLATFORM_REVIEW: none (${platform.reason})`); if (fixRange) out.push(`FIX_RANGE: ${fixRange.runtime_lines === null ? `unread (${fixRange.error})` : `runtime_lines=${fixRange.runtime_lines}`} base=${fixRange.base}`); out.push(reviewPlanLine(plan, pass + (plan.next.step === 'full' || plan.next.step === 'scoped' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`); diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 2fda0afcb..2a7674a3f 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -50,6 +50,7 @@ import { import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch'; import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, platformReceiptDepth, receiptExclude, receiptReviewAtHead, reviewLoopExceptionLine } from '../lib/ship-receipt-depth'; +import { RISK_CLASSES, type RiskClass } from '../lib/review-risk'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; import { readPause, receiptBaseRed } from '../lib/base-red'; @@ -99,6 +100,8 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate (the digest= on gstack-review-dispatch verify's REVIEW_EXCLUDE_RECORD: line, which the reviewers' briefs name; none without that line): must equal the digest HEAD has under the base policy's review_exclude_paths, else RECEIPT_INVALID field=review_exclude and nothing is written (rerun the review; write alone cannot clear it); recorded as review_exclude. Carry and the queue compare that digest. + --declare-risk (the same list gstack-review-plan --declare-risk took): joins review_depth's risk and only raises + its depth; review_depth.declared and .detected record both lists (GST-1129). An unknown class is RECEIPT_INVALID. --platform-review (NEXT: platform's PLATFORM_REVIEW: id): records review_depth platform with capy-ai[bot]'s review at HEAD; refused (RECEIPT_INVALID) unless the base policy has review.platform on, HEAD needs light review of a docs or tests diff with no numeric, results or prereg claims, and that review is on the PR at HEAD. review_depth.mode records lean, panel or platform. @@ -224,7 +227,9 @@ function cmdWrite(rest: string[], json: boolean): number { const line = reviewLoopExceptionLine(loopException.exception); if (bodyFile && !bodyText.split(/\r?\n/).some(l => l.trim() === line)) return refuse(`${bodyFile} does not carry the exception line verbatim: ${JSON.stringify(line)}`); } - const review = receiptReviewAtHead(repoRoot, base, path.dirname(Bun.fileURLToPath(import.meta.url)), headSha ?? head); + const declared = (flag(rest, '--declare-risk') ?? '').split(',').map(s => s.trim()).filter(Boolean); + if (rest.includes('--declare-risk') && (!declared.length || declared.some(c => !(RISK_CLASSES as readonly string[]).includes(c)))) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `--declare-risk ${flag(rest, '--declare-risk') ?? ''}: each class is one of ${RISK_CLASSES.join(', ')}; nothing written`) + '\n'); return EXIT.fail; } + const review = receiptReviewAtHead(repoRoot, base, path.dirname(Bun.fileURLToPath(import.meta.url)), headSha ?? head, declared as RiskClass[]); let reviewDepth = review.depth; const platformId = flag(rest, '--platform-review'); if (platformId !== undefined) { diff --git a/changelog.d/gsta258-c-review-depth.md b/changelog.d/gsta258-c-review-depth.md index 34bc4d15b..0710321a2 100644 --- a/changelog.d/gsta258-c-review-depth.md +++ b/changelog.d/gsta258-c-review-depth.md @@ -6,6 +6,8 @@ - **The platform depth.** Under `review.platform on`, a docs or tests diff with no numeric, results or prereg claims, whose PR carries capy-ai[bot]'s review at the exact head, plans `NEXT: platform` with a planned `SKIP:` for every reader. Step 11.5 accepts it, and `gstack-ship-receipt write --platform-review ` records `review_depth: platform` with that review's id and head. It never applies to runtime code, and never where platform review is off. - **New risk classes**: `eval` (scorer, judge and grader paths) and `policy` (`review.risk_paths`). A docs line that can print a credential (an `Authorization:` header, a bearer value, a key-shaped literal, `mcp get`) is security risk (gbrain#8417). - **The receipt records `review_depth.mode`** (lean, panel or platform). +- **Declared risk classes** (GST-1129). `gstack-review-plan --declare-risk ` and `gstack-ship-receipt write --declare-risk ` add classes the matchers missed. A declaration only raises the depth, the plan prints `RISK_DECLARED:`, and the receipt keeps `review_depth.declared` beside `review_depth.detected`. +- **Code that reserves, binds, leases or settles spend is money** (GST-1129): `bindCheck(ledger, …)`, `reserveSpend` or `bindCap` in a code file now plans full depth. gstack-evals#170 got its full review only because an unrelated `UBICLOUD_API_KEY` line was in the diff. ### Changed - **Non-risk code gets one native adversarial reader** at the lane's reasoning, carrying the code checklists and the Red Team checklist, in place of the batched specialist plus the native pass. One fix-check follows a fixed blocker; below-floor fixes ship as notes. `review.panel all` restores the old plan and loop everywhere. diff --git a/docs/review-policy.md b/docs/review-policy.md index 2bffa673e..4355b38f3 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -205,8 +205,12 @@ judged by their path, so a `lease_usd` value or an `os.path.join(sys.argv[1], stats script names no class (HARN-1, harness-evals#231), and binary data such as `*.sqlite` is data wherever it sits; a docs line that can print a credential (an `Authorization:` header, a bearer or key-shaped value, an environment key set to a value, -a get or show command for auth config such as `mcp get`) is security (gbrain#8417). The -outside voice reads its diff from a file the brief names with its sha256; past +a get or show command for auth config such as `mcp get`) is security (gbrain#8417); code +that reserves, binds, leases or settles spend against a ledger, budget or cap (`bindCheck(ledger, …)`, +`reserveSpend`, `bindCap`) is money (GST-1129, gstack-evals#170). When the matchers miss a class the +lane knows the diff has, `gstack-review-plan --declare-risk ` adds it: a declaration joins the +detected classes, only raises the depth, and prints `RISK_DECLARED: classes= detected=`. +The outside voice reads its diff from a file the brief names with its sha256; past 2,000,000 bytes it reads a path-scoped slice whose brief lists the files left out. `--fix-checks ` tells the lean loop how many fix-checks ran, so the owner's grant adds one fix-check after the first and never a second. @@ -501,7 +505,8 @@ loop prints the old line. Beside the counters, `write` records `review_depth` (the depth, runtime lines, rules hash, base and registry commits, the gate scan bound to the head, the risk classes and -`mode`: lean, panel or platform) and the queue refuses a receipt below the depth its head needs +`mode`: lean, panel or platform; with `--declare-risk`, `declared` and `detected` beside `risk`, so a +class the lane added shows apart from what the matchers found) and the queue refuses a receipt below the depth its head needs ([ship-policy.md#ship-receipt](ship-policy.md#ship-receipt)). After `NEXT: platform`, `write --platform-review ` records `review_depth: platform` with `platform_review` (capy-ai[bot]'s review id, reviewer and head); write and the queue accept it only under diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 358c99b8d..5646c790f 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -114,7 +114,7 @@ export interface ShipReceipt { head_tree?: string; product_patch_id?: string; /** The gstack that wrote the receipt, and the review depth the reviewed head needs, recomputed at write (lib/ship-receipt-depth.ts receiptDepthAtHead). */ gstack_version?: string; - review_depth?: { depth: 'platform' | 'light' | 'standard' | 'full'; class: 'docs' | 'tests' | 'code' | 'gate'; runtime_lines: number; rules: string; base: string; registry: string | null; gate_scan: { tagged: number | null; head: string }; reasons: string[]; risk?: string[]; mode?: 'lean' | 'panel' | 'platform'; platform_review?: { id: string; head: string; reviewer: string } }; + review_depth?: { depth: 'platform' | 'light' | 'standard' | 'full'; class: 'docs' | 'tests' | 'code' | 'gate'; runtime_lines: number; rules: string; base: string; registry: string | null; gate_scan: { tagged: number | null; head: string }; reasons: string[]; risk?: string[]; mode?: 'lean' | 'panel' | 'platform'; platform_review?: { id: string; head: string; reviewer: string }; declared?: string[]; detected?: string[] }; /** The owner's acceptance of a stop after a fix-check edit under frozen scope (#3559); the PR body repeats it word for word. */ review_loop_exception?: { accepted_by: string; date: string; via: string; reason: string; fix_check: { head: string; findings: number; files?: string[] } }; /** The targeted pass on money, gate, auth and secret code (lib/review-targeted.ts), bound to the receipt's product_patch_id; `done` only with its output collected. */ @@ -247,7 +247,7 @@ export const ARTIFACT_SCHEMAS = { carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList, trees: strList, ancestry: { enum: ['linear', 'rewritten'] } } }, head_tree: str, product_patch_id: str, gstack_version: str, base_red: { type: 'object', required: ['cases', 'files', 'base', 'lane', 'expires_at'], properties: { cases: strList, files: strList, base: str, lane: str, expires_at: str } }, - review_depth: { type: 'object', required: ['depth', 'class', 'runtime_lines', 'rules', 'base', 'gate_scan'], properties: { depth: { enum: ['platform', 'light', 'standard', 'full'] }, mode: { enum: ['lean', 'panel', 'platform'] }, platform_review: { type: 'object', required: ['id', 'head', 'reviewer'], properties: { id: str, head: str, reviewer: str } }, class: { enum: ['docs', 'tests', 'code', 'gate'] }, runtime_lines: { type: 'integer', minimum: 0 }, rules: str, base: str, registry: nullableStr, gate_scan: { type: 'object', required: ['tagged', 'head'], properties: { tagged: { type: ['integer', 'null'], minimum: 0 }, head: str } }, reasons: strList, risk: { type: 'array', items: { enum: ['money', 'auth', 'secret', 'security', 'data', 'eval', 'policy'] } } } }, + review_depth: { type: 'object', required: ['depth', 'class', 'runtime_lines', 'rules', 'base', 'gate_scan'], properties: { depth: { enum: ['platform', 'light', 'standard', 'full'] }, mode: { enum: ['lean', 'panel', 'platform'] }, platform_review: { type: 'object', required: ['id', 'head', 'reviewer'], properties: { id: str, head: str, reviewer: str } }, class: { enum: ['docs', 'tests', 'code', 'gate'] }, runtime_lines: { type: 'integer', minimum: 0 }, rules: str, base: str, registry: nullableStr, gate_scan: { type: 'object', required: ['tagged', 'head'], properties: { tagged: { type: ['integer', 'null'], minimum: 0 }, head: str } }, reasons: strList, risk: { type: 'array', items: { enum: ['money', 'auth', 'secret', 'security', 'data', 'eval', 'policy'] } }, declared: { type: 'array', items: { enum: ['money', 'auth', 'secret', 'security', 'data', 'eval', 'policy'] } }, detected: { type: 'array', items: { enum: ['money', 'auth', 'secret', 'security', 'data', 'eval', 'policy'] } } } }, review_targeted: { type: 'object', required: ['classes', 'rules', 'brief_sha', 'product_patch_id', 'status', 'findings', 'raised_by'], properties: { classes: { type: 'array', items: { enum: ['money', 'gate', 'auth', 'secret'] } }, rules: str, brief_sha: str, product_patch_id: str, status: { enum: ['done', 'failed', 'not-run'] }, findings: { type: 'integer', minimum: 0 }, raised_by: str } }, review_exclude: { type: 'object', required: ['source', 'patterns', 'applied', 'excluded', 'excluded_bytes', 'kept', 'gate_tagged', 'patterns_sha256', 'list_sha256', 'digest'], properties: { source: str, patterns: strList, applied: strList, excluded: { type: 'integer', minimum: 0 }, excluded_bytes: { type: 'integer', minimum: 0 }, kept: { type: 'integer', minimum: 0 }, gate_tagged: strList, patterns_sha256: str, list_sha256: str, digest: str } }, review_loop_exception: { type: 'object', required: ['accepted_by', 'date', 'via', 'reason', 'fix_check'], properties: { accepted_by: str, date: str, via: str, reason: str, fix_check: { type: 'object', required: ['head', 'findings'], properties: { head: str, findings: { type: 'integer', minimum: 0 }, files: strList } } } }, diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 7dfb5814c..43c6c7415 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -233,8 +233,11 @@ export interface ClassInput { excluded?: ReadonlySet; /** The base policy's review.risk_paths: a code path they match is risk class `policy`; data and binary files under them never are. */ riskPaths?: readonly string[]; + /** Risk classes the lane declares for the diff (GST-1129, --declare-risk): they join the detected ones and can only raise the depth. */ + declared?: readonly RiskClass[]; } -export interface ClassResult { class: DiffClass; reasons: string[]; errors: DepthError[]; runtimeLines: number | null; risk: RiskClass[] } +/** risk is detected plus declared; `detected` is what the matchers found, `declared` what the lane added. */ +export interface ClassResult { class: DiffClass; reasons: string[]; errors: DepthError[]; runtimeLines: number | null; risk: RiskClass[]; detected: RiskClass[]; declared: RiskClass[] } const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200); const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p); @@ -274,16 +277,20 @@ export function classifyDiff(i: ClassInput): ClassResult { const riskHits = CLASS_RANK[cls] >= CLASS_RANK.code ? matchRisk({ codePaths: i.paths.filter(p => pathClass(i, p) === 'code'), added: i.added, removed: i.removed, scopeAuth: s.SCOPE_AUTH === 'true', ...(i.riskPaths?.length ? { isRiskPath: (p: string) => matchesAny(i.riskPaths!, p) && !DATA_PATH.test(p) } : {}) }) : new Map(); const docSecret = i.paths.find(p => pathClass(i, p) === 'docs' && DOC_PATH.test(p) && (i.added?.[p] ?? []).some(l => DOC_SECRET_LINE.some(r => r.test(l)))); if (docSecret && !riskHits.has('security')) riskHits.set('security', `docs that can print a credential: ${docSecret}`); - const risk = [...riskHits.keys()].sort((a, b) => RISK_CLASSES.indexOf(a) - RISK_CLASSES.indexOf(b)); + const byRank = (a: RiskClass, b: RiskClass) => RISK_CLASSES.indexOf(a) - RISK_CLASSES.indexOf(b); + const detected = [...riskHits.keys()].sort(byRank); + const declared = [...new Set(i.declared ?? [])].sort(byRank); + for (const d of declared) if (!riskHits.has(d)) riskHits.set(d, `declared: ${d}`); + const risk = [...riskHits.keys()].sort(byRank); reasons.push(...[...riskHits.values()].map(r => `risk: ${r}`)); - if (!i.perPath) return { class: cls, reasons, errors, runtimeLines: null, risk }; + if (!i.perPath) return { class: cls, reasons, errors, runtimeLines: null, risk, detected, declared }; let runtimeLines = 0; for (const e of i.perPath) { if (pathClass(i, e.path) !== 'code' || isOutput(i, e.path)) continue; if (e.binary) errors.push({ code: 'binary_code_path', detail: 'git reports a binary change, so its lines cannot be counted', input: e.path, repair: "register it as an output of an entry with a command in the base branch's .gstack/generated.json, or accept the full review" }); else runtimeLines += e.lines; } - return { class: cls, reasons, errors, runtimeLines, risk }; + return { class: cls, reasons, errors, runtimeLines, risk, detected, declared }; } /** Risk, not size (R51): any classifier error, gate class or risk class is full; everything else is light. A floor only raises; a fixed blocker or a CRITICAL raises light to standard for the next step (planReview). */ @@ -292,14 +299,14 @@ export function depthFor(o: { cls: DiffClass; runtimeLines: number; errors: read return o.floor && DEPTH_RANK[o.floor] > DEPTH_RANK[d] ? o.floor : d; } -export interface DepthResult { depth: Depth; class: DiffClass; runtimeLines: number; reasons: string[]; errors: DepthError[]; codes: ResultCodeName[]; rules: string; risk: RiskClass[] } +export interface DepthResult { depth: Depth; class: DiffClass; runtimeLines: number; reasons: string[]; errors: DepthError[]; codes: ResultCodeName[]; rules: string; risk: RiskClass[]; detected: RiskClass[]; declared: RiskClass[] } function decideDepth(c: ClassResult, runtimeLines: number, floor?: Depth): DepthResult { const unfloored = depthFor({ cls: c.class, runtimeLines, errors: c.errors, risk: c.risk }); const reasons = [...c.errors.map(e => `error:${e.code}`), ...(c.class === 'gate' ? ['gate-class'] : []), ...c.risk.map(r => `risk:${r}`), `runtime=${runtimeLines}`]; if (floor && DEPTH_RANK[floor] > DEPTH_RANK[unfloored]) reasons.push(`floor:${floor}`); const codes = [...new Set(c.errors.flatMap(e => (e.result ? [e.result] : [])))]; - return { depth: depthFor({ cls: c.class, runtimeLines, errors: c.errors, risk: c.risk, floor }), class: c.class, runtimeLines, reasons, errors: c.errors, codes, rules: DEPTH_RULES, risk: c.risk }; + return { depth: depthFor({ cls: c.class, runtimeLines, errors: c.errors, risk: c.risk, floor }), class: c.class, runtimeLines, reasons, errors: c.errors, codes, rules: DEPTH_RULES, risk: c.risk, detected: c.detected, declared: c.declared }; } /** The depth of a collected range (lib/review-collect.ts) plus a recorded gate-scan result: what the queue recomputes at a head. */ diff --git a/lib/review-risk.ts b/lib/review-risk.ts index f34203d13..7a0a4511c 100644 --- a/lib/review-risk.ts +++ b/lib/review-risk.ts @@ -22,6 +22,15 @@ export const MONEY_PATH_TOKENS = ['cost', 'costs', 'spend', 'spending', 'budget' export const MONEY_PAIRS: readonly (readonly [string, readonly string[]])[] = [ ['usage', ['token', 'tokens', 'cost', 'costs', 'spend', 'billing', 'price', 'pricing', 'budget']], ['proxy', ['billing', 'bill']], ['client', ['cost', 'costs', 'charge', 'charges', 'billing', 'price', 'pricing', 'spend']], ]; +/** + * Spend a run reserves, binds, leases or settles against a ledger, a budget or a cap (GST-1129: gstack-evals#170 bound paid spend + * with `bindCheck(ledger, …)`): a verb beside a spend word on one changed code line. Prose lines (PROSE_PATH) never count: skill + * text says "binds" and "ledger" in passing (#3698 and #3798 would have read as money). + */ +const SPEND_BIND = ['reserve', 'reserves', 'reserved', 'reservation', 'reservations', 'bind', 'binds', 'lease', 'leases', 'leased', 'settle', 'settles', 'settled']; +const SPEND_OF = ['ledger', 'ledgers', 'spend', 'budget', 'budgets', 'usd', 'cost', 'costs', 'credits', 'billing', 'cap', 'caps']; +export const SPEND_PAIRS: readonly (readonly [string, readonly string[]])[] = [...SPEND_BIND.map(w => [w, SPEND_OF] as const), ['cap', ['spend', 'usd', 'budget', 'ledger']]]; +export const PROSE_PATH = /\.(md|mdx|markdown|txt|rst|adoc|tmpl)$/i; /** Identifiers ending in _usd, Usd or cap_usd, any spend_cap spelling, and paid-provider call sites (the providers lib/pricing.ts prices). */ export const MONEY_LINE = [ /\b[A-Za-z0-9_]*_usd\b/i, /\b[a-z][A-Za-z0-9]*Usd\b/, /\bspend[_-]?cap\b/i, @@ -96,13 +105,14 @@ export const DOC_SECRET_LINE = [ ]; /** Whether a path or line's tokens name money: a metering or charge word (a cost word too, for a path), or a pair. */ -const namesMoney = (t: readonly string[], words: readonly string[]) => t.some(x => words.includes(x)) || MONEY_PAIRS.some(([w, beside]) => t.includes(w) && t.some(x => beside.includes(x))); +const pairs = (t: readonly string[], list: typeof MONEY_PAIRS) => list.some(([w, beside]) => t.includes(w) && t.some(x => beside.includes(x))); +const namesMoney = (t: readonly string[], words: readonly string[]) => t.some(x => words.includes(x)) || pairs(t, MONEY_PAIRS); /** A hash of the patterns that decide the risk classes, not of this file's bytes; lib/review-depth.ts folds it into DEPTH_RULES. */ export const RISK_RULES = createHash('sha256').update(JSON.stringify({ RISK_CLASSES, MONEY_PATH_TOKENS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME, SECURITY_PATH_TOKENS, DATA_PATH_TOKENS, EVAL_PATH_TOKENS, - MONEY_LINE_TOKENS, MONEY_PAIRS, - patterns: Object.entries({ DOC_SECRET_LINE, CONFIG_DATA_PATH: [CONFIG_DATA_PATH], STATS_SCRIPT: [STATS_SCRIPT], MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH], SECURITY_LINE, DATA_LINE, SVG_PATH: [SVG_PATH], SVG_ACTIVE }).map(([k, v]) => [k, v.map(r => '/' + r.source + '/' + r.flags)]), + MONEY_LINE_TOKENS, MONEY_PAIRS, SPEND_PAIRS, + patterns: Object.entries({ PROSE_PATH: [PROSE_PATH], DOC_SECRET_LINE, CONFIG_DATA_PATH: [CONFIG_DATA_PATH], STATS_SCRIPT: [STATS_SCRIPT], MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH], SECURITY_LINE, DATA_LINE, SVG_PATH: [SVG_PATH], SVG_ACTIVE }).map(([k, v]) => [k, v.map(r => '/' + r.source + '/' + r.flags)]), })).digest('hex').slice(0, 16); export const tokens = (p: string) => p.replace(/([a-z0-9])([A-Z])/g, '$1 $2').toLowerCase().split(/[^a-z0-9]+/).filter(Boolean); @@ -127,6 +137,7 @@ export function matchRisk(i: RiskInput): Map { if (STATS_SCRIPT.test(p)) continue; const t = tokens(p); const config = CONFIG_DATA_PATH.test(p); + const prose = PROSE_PATH.test(p); if (i.scopeAuth && t.some(x => AUTH_PATH_TOKENS.includes(x) || AUTH_PATH_PREFIXES.some(a => x.startsWith(a)))) hit('auth', 'auth path: ' + p); if (SECRET_PATH.test(p)) hit('secret', 'secret path: ' + p); if (namesMoney(t, MONEY_PATH_TOKENS)) hit('money', 'money path: ' + p); @@ -137,7 +148,7 @@ export function matchRisk(i: RiskInput): Map { for (const line of [...(i.added?.[p] ?? []), ...(i.removed?.[p] ?? [])]) { if (!why.has('secret') && SECRET_LINE.some(r => r.test(line))) hit('secret', 'secret line: ' + p); if (config) continue; - if (!why.has('money') && (MONEY_LINE.some(r => r.test(line)) || namesMoney(tokens(line), MONEY_LINE_TOKENS))) hit('money', 'money line: ' + p); + if (!why.has('money') && (MONEY_LINE.some(r => r.test(line)) || namesMoney(tokens(line), MONEY_LINE_TOKENS) || (!prose && pairs(tokens(line), SPEND_PAIRS)))) hit('money', 'money line: ' + p); if (!why.has('security') && SECURITY_LINE.some(r => r.test(line))) hit('security', 'security line: ' + p); if (!why.has('data') && DATA_LINE.some(r => r.test(line))) hit('data', 'data line: ' + p); } diff --git a/lib/review-targeted.ts b/lib/review-targeted.ts index 08af1cafa..346dbb803 100644 --- a/lib/review-targeted.ts +++ b/lib/review-targeted.ts @@ -21,7 +21,7 @@ import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { classifyPath } from './review-depth'; -import { AUTH_PATH_PREFIXES, AUTH_PATH_TOKENS, KEY_NAME, MONEY_LINE, MONEY_LINE_TOKENS, MONEY_PAIRS, MONEY_PATH_TOKENS, SECRET_LINE, SECRET_PATH, matchRisk } from './review-risk'; +import { AUTH_PATH_PREFIXES, AUTH_PATH_TOKENS, KEY_NAME, MONEY_LINE, MONEY_LINE_TOKENS, MONEY_PAIRS, MONEY_PATH_TOKENS, PROSE_PATH, SECRET_LINE, SECRET_PATH, SPEND_PAIRS, matchRisk } from './review-risk'; export const TARGETED_CLASSES = ['money', 'gate', 'auth', 'secret'] as const; export type TargetedClass = (typeof TARGETED_CLASSES)[number]; @@ -31,8 +31,8 @@ export const secretLine = (line: string) => SECRET_LINE.some(r => r.test(line)); /** A hash of the patterns that decide the classes, not of this file's bytes: a comment edit keeps it, a pattern edit changes it. */ export const TARGETED_RULES = createHash('sha256').update(JSON.stringify({ - TARGETED_CLASSES, MONEY_PATH_TOKENS, MONEY_LINE_TOKENS, MONEY_PAIRS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME, - patterns: Object.entries({ MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), + TARGETED_CLASSES, MONEY_PATH_TOKENS, MONEY_LINE_TOKENS, MONEY_PAIRS, SPEND_PAIRS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME, + patterns: Object.entries({ MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH], PROSE_PATH: [PROSE_PATH] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), })).digest('hex').slice(0, 16); export interface TargetedInput { diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 0258a6cd9..ed7f80470 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -26,6 +26,7 @@ import { reviewExclude, type Exclusion } from './review-exclude'; import { isUnavailable } from './ship-policy'; import { loadPolicyAt } from './ship-policy-at'; import { depthOfCommitInput, gateScanNeeded, platformFits, scanNumbers, type Depth } from './review-depth'; +import type { RiskClass } from './review-risk'; import { platformReviewAt } from './review-platform'; import { reviewPolicy, type ReviewPolicy } from './ship-policy-review'; import { TARGETED_CLASSES, classList, targetedClasses, type TargetedClass } from './review-targeted'; @@ -93,8 +94,11 @@ export function receiptDepthAtHead(repoRoot: string, base: string, binDir: strin return receiptReviewAtHead(repoRoot, base, binDir, head).depth; } -/** receiptDepthAtHead plus the exclusion the head's review read under: the depth classifies excluded paths as docs (GST-508). */ -export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string): { depth: ReceiptDepth; exclude: Exclusion } { +/** + * receiptDepthAtHead plus the exclusion the head's review read under: the depth classifies excluded paths as docs (GST-508). + * `declared` (write --declare-risk, GST-1129) joins the detected risk classes and only raises; the record then keeps both lists. + */ +export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string, declared: readonly RiskClass[] = []): { depth: ReceiptDepth; exclude: Exclusion } { const branch = base.replace(/^origin\//, ''); const baseSha = baseCommit(repoRoot, `origin/${branch}`); const baseRev = baseSha.ok ? baseSha.out : branch; @@ -113,8 +117,8 @@ export function receiptReviewAtHead(repoRoot: string, base: string, binDir: stri paths = declares && tagged !== null ? taggedPaths(g?.stdout ?? '') : null; } const exclude = exclusionAt(repoRoot, ref, sha, head, c, paths, 'gstack-ship-receipt'); - const d = depthOfCommitInput({ ...c, scope, gateTagged: tagged, ...riskPathsOf(review), ...(exclude.state === 'active' ? { excluded: exclude.excluded } : {}) }); - return { depth: { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk, mode: modeFor(d.depth, review) }, exclude }; + const d = depthOfCommitInput({ ...c, scope, gateTagged: tagged, ...riskPathsOf(review), declared, ...(exclude.state === 'active' ? { excluded: exclude.excluded } : {}) }); + return { depth: { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk, mode: modeFor(d.depth, review), ...(d.declared.length ? { declared: d.declared, detected: d.detected } : {}) }, exclude }; } /** diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index c76eb6f71..d78e77677 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -17,7 +17,9 @@ Finish the complete review and QA before applying any fix in Step 9.4. **Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan --base `, plus `--depth-floor ` on every plan run when the /ship prompt -gave `--depth-floor ` (it only raises); a repeat uses the plan run that picked it. +gave `--depth-floor ` (it only raises); add `--declare-risk ` when the diff touches a risk +class the plan's `REVIEW_DEPTH:` misses (spend it reserves or binds: `money`), and give Step 16's +receipt write the same flag; a repeat uses the plan run that picked it. A `scoped` repeat is a full pass on the range: its `fix-check` is decision 3.5's reviewer and prompt with `git diff $RANGE_BASE` as the hunks, inside this pass. Save its `REVIEW_PLAN:` line and fingerprint, `REVIEW_DEPTH:` (depth, diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 82da53d85..899aeb7c7 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -15,7 +15,9 @@ Finish the complete review and QA before applying any fix in Step 9.4. **Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan --base `, plus `--depth-floor ` on every plan run when the /ship prompt -gave `--depth-floor ` (it only raises); a repeat uses the plan run that picked it. +gave `--depth-floor ` (it only raises); add `--declare-risk ` when the diff touches a risk +class the plan's `REVIEW_DEPTH:` misses (spend it reserves or binds: `money`), and give Step 16's +receipt write the same flag; a repeat uses the plan run that picked it. A `scoped` repeat is a full pass on the range: its `fix-check` is decision 3.5's reviewer and prompt with `git diff $RANGE_BASE` as the hunks, inside this pass. Save its `REVIEW_PLAN:` line and fingerprint, `REVIEW_DEPTH:` (depth, diff --git a/test/review-declared-risk.test.ts b/test/review-declared-risk.test.ts new file mode 100644 index 000000000..d4145ddb3 --- /dev/null +++ b/test/review-declared-risk.test.ts @@ -0,0 +1,95 @@ +import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { classifyDiff, depthOfCommitInput } from '../lib/review-depth'; +import { matchRisk } from '../lib/review-risk'; +import { receiptReviewAtHead } from '../lib/ship-receipt-depth'; +import { ARTIFACT_SCHEMAS, checkSchema, type ValidationError } from '../lib/headless-artifacts'; + +const BIN = path.join(import.meta.dir, '..', 'bin'); +const money = (p: string, lines: string[]) => matchRisk({ codePaths: [p], added: { [p]: lines }, scopeAuth: false }).get('money'); + +// GST-1129: gstack-evals#170 (GEVA-221) reserves and binds paid spend in quick/laneb/r3/screen.ts. These are its run() hunk's added +// lines without the UBICLOUD_API_KEY line, which was the only thing that took the review to full depth. +const SCREEN = 'quick/laneb/r3/screen.ts'; +const GEVA221 = [ + ' const bound = ledger + ".bound";', + ' if (existsSync(bound)) throw new Error(boundTo());', + ' const runner = process.env.UBI_RUNNER ?? "";', + ' const ttl = process.env.UBI_TTL_HOURS || "3";', + ' bindCheck(ledger, m, configs);', + ' throw new Error(boundTo());', +]; + +describe('GST-1129 item 1: code that reserves or binds spend is money (GEVA-221)', () => { + test("GEVA-221's run() hunk, without the UBICLOUD_API_KEY line, plans full depth with money in the risk list", () => { + expect(money(SCREEN, GEVA221)).toBe('money line: ' + SCREEN); + const d = depthOfCommitInput({ paths: [SCREEN], gateTagged: 0, added: { [SCREEN]: GEVA221 }, perPath: [{ path: SCREEN, lines: GEVA221.length, binary: false }] }); + expect([d.depth, d.risk]).toEqual(['full', ['money']]); + }); + test.each([['reserveSession(ledger, item)'], ['const ok = bindCap(item, cap);'], ['lease.settle(budget)'], ['await reserveSpend(id)'], ['if (spendCap === null) return;'], ['const leased = leaseCredits(n);']])('money: %s', line => { + expect(money('src/run.ts', [line])).toBeDefined(); + }); + test.each([['appendEvent(m.ledger, { event: "rendered" });'], ['const bound = Math.min(a, b);'], ['reserve(slots);'], ['handler.bind(this);'], ['const cap = Math.max(0, n);']])('not money: %s', line => { + expect(money('src/run.ts', [line])).toBeUndefined(); + }); + test('prose never names money by these pairs: skill text says binds and ledger in passing (#3698, #3798)', () => { + expect(money('capy/skill/gstack/SKILL.md', ['Each pass binds its reviewers to the review ledger before Step 11.'])).toBeUndefined(); + expect(money('src/run.ts', ['Each pass binds its reviewers to the review ledger before Step 11.'])).toBeDefined(); + }); + test('code paths only (HARN-1): a data file and a stats script with the same words never name money', () => { + expect(money('runs/item.json', [' "bindCheck": "ledger", "reserve": "budget"'])).toBeUndefined(); + expect(money('docs/results/bind-check.py', ['bindCheck(ledger, m)'])).toBeUndefined(); + }); +}); + +describe('GST-1129 item 2: a declared risk class only raises the depth', () => { + const light = { paths: ['src/app.ts'], gateTagged: 0, perPath: [{ path: 'src/app.ts', lines: 10, binary: false }] }; + test('a class the matcher missed raises light to full, and the result keeps detected and declared apart', () => { + expect(depthOfCommitInput(light).depth).toBe('light'); + const d = depthOfCommitInput({ ...light, declared: ['money'] }); + expect([d.depth, d.risk, d.detected, d.declared]).toEqual(['full', ['money'], [], ['money']]); + }); + test('declaring nothing or another class never drops a detected one; a docs diff with a declared class plans full', () => { + const c = classifyDiff({ paths: ['lib/billing.ts'], gateTagged: 0, declared: [] }); + expect([c.risk, c.detected, c.declared]).toEqual([['money'], ['money'], []]); + const both = classifyDiff({ paths: ['lib/billing.ts'], gateTagged: 0, declared: ['auth'] }); + expect([both.risk, both.detected, both.declared]).toEqual([['money', 'auth'], ['money'], ['auth']]); + expect(depthOfCommitInput({ paths: ['docs/a.md'], gateTagged: 0, declared: ['security'] }).depth).toBe('full'); + }); + + function repo(): { dir: string; head: string; state: string } { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'declared-')); + const state = fs.mkdtempSync(path.join(os.tmpdir(), 'declared-state-')); + const git = (...a: string[]) => { const r = spawnSync('git', a, { cwd: dir, encoding: 'utf8', timeout: 30_000 }); if (r.status !== 0) throw new Error(r.stderr); return r.stdout.trim(); }; + git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@example.com'); git('config', 'user.name', 't'); git('config', 'commit.gpgsign', 'false'); + fs.writeFileSync(path.join(dir, 'README.md'), '# x\n'); git('add', '.'); git('commit', '-qm', 'base'); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + git('checkout', '-qb', 'feat'); + fs.mkdirSync(path.join(dir, 'src')); fs.writeFileSync(path.join(dir, 'src/app.ts'), 'export const a = 1;\n'); + git('add', '.'); git('commit', '-qm', 'change'); + return { dir, head: git('rev-parse', 'HEAD'), state }; + } + test('gstack-review-plan --declare-risk prints the declared and detected classes and plans full; an unknown class is a usage error', () => { + const r = repo(); + const plan = (...a: string[]) => spawnSync(path.join(BIN, 'gstack-review-plan'), ['--base', 'main', '--gate-tagged', '0', ...a], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + expect(plan().stdout).toMatch(/^REVIEW_DEPTH: light /m); + const out = plan('--declare-risk', 'money').stdout; + expect(out).toMatch(/^REVIEW_DEPTH: full .*reasons=risk:money,/m); + expect(out).toContain('RISK_DECLARED: classes=money detected=none'); + const bad = plan('--declare-risk', 'cash'); + expect([bad.status, bad.stdout]).toEqual([2, '']); + expect(bad.stderr).toContain('--declare-risk cash'); + }, 60_000); + test('the receipt records declared and detected classes beside the raised depth, and the schema accepts them', () => { + const r = repo(); + const d = receiptReviewAtHead(r.dir, 'main', BIN, r.head, ['money']).depth; + expect([d.depth, d.risk, d.declared, d.detected]).toEqual(['full', ['money'], ['money'], []]); + expect(receiptReviewAtHead(r.dir, 'main', BIN, r.head).depth).not.toHaveProperty('declared'); + const errors: ValidationError[] = []; + checkSchema({ schema_version: 1, head: r.head, base: 'main', review_depth: d }, ARTIFACT_SCHEMAS['ship-receipt'], '', errors); + expect(errors.filter(e => JSON.stringify(e).includes('review_depth'))).toEqual([]); + }, 60_000); +}); diff --git a/test/size-ledger/ship/gsta258-c-review-depth.json b/test/size-ledger/ship/gsta258-c-review-depth.json index d1908a260..e25dfda74 100644 --- a/test/size-ledger/ship/gsta258-c-review-depth.json +++ b/test/size-ledger/ship/gsta258-c-review-depth.json @@ -1,4 +1,4 @@ { - "bytes": 373, - "why": "GSTA-258 C: risk-scaled review depth (single/merged reader lines, NEXT: platform planned skip with its PLATFORM_REVIEW id, round 2 on risk class)" + "bytes": 560, + "why": "GSTA-258 C: risk-scaled review depth (single/merged reader lines, NEXT: platform planned skip with its PLATFORM_REVIEW id, round 2 on risk class) and GST-1129 --declare-risk" }