mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
review: targeted read-only pass on money, gate, auth and secret diffs (GST-257 part 1, report-only)
lib/review-targeted.ts classifies a diff's added and deleted lines into money, gate, auth and secret (token-bounded, docs and tests excluded, fails closed to all four). gstack-review-plan prints TARGETED and dispatches one read-only targeted reader at full effort when a class hits; review/specialists/targeted.md is its brief. The receipt records review_targeted bound to product_patch_id, a carry keeps it, and gstack-ship-queue ready / train build print a report-only REVIEW_TARGETED line that refuses nothing. No-loss evidence: the dispatch-superset test in test/review-targeted.test.ts runs gstack-review-plan on 18 fixture diffs and requires every reviewer main dispatched at8be7f21, unchanged, plus only the targeted reader. Brief identity: gstack-review-dispatch render on this tree and origin/main2356792gives 13 of 13 existing reviewer briefs byte-identical (handoffs/GSTA-160/gst257-briefcmp/SUMMARY.txt). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
a77fae2837
commit
88cf2540cb
22 files changed
+723
-50
No files matched your search
@@ -111,6 +111,8 @@ export interface ShipReceipt {
|
||||
review_depth?: { depth: '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[] };
|
||||
/** 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. */
|
||||
review_targeted?: { classes: string[]; rules: string; brief_sha: string; product_patch_id: string; status: 'done' | 'failed' | 'not-run'; findings: number; raised_by: string };
|
||||
}
|
||||
|
||||
export const RESULT_STATUSES: readonly ResultStatus[] = ['complete', 'gate_pending', 'amend_pending', 'incomplete', 'interrupted', 'refused'];
|
||||
@@ -234,6 +236,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,
|
||||
review_depth: { type: 'object', required: ['depth', 'class', 'runtime_lines', 'rules', 'base', 'gate_scan'], properties: { depth: { enum: ['light', 'standard', 'full'] }, 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 } },
|
||||
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_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 } } } },
|
||||
issues: { type: 'object', properties: { fixes: { type: 'array', items: { type: 'integer' } }, refs: { type: 'array', items: { type: 'integer' } }, flakes: { type: 'array', items: { type: 'object', required: ['issue', 'run'], properties: { issue: { type: 'integer' }, run: str } } }, related: { type: 'array', items: { type: 'object', required: ['pr', 'author', 'closes'], properties: { pr: { type: 'integer' }, repo: str, author: str, closes: { type: 'array', items: { type: 'integer' } }, supersedes: { type: 'boolean' }, credit: { type: 'boolean' } } } } } },
|
||||
},
|
||||
|
||||
+11
-1
@@ -12,13 +12,15 @@
|
||||
* adversarial-native diff_base
|
||||
* outside-voice diff_base, diff (the branch diff, which the bin reads from git)
|
||||
* fix-check fix_list, hunks, tests, callers (scripts/resolvers/fix-check.ts)
|
||||
* targeted classes, brief (review/specialists/targeted.md's text), checklist (its path), diff_base, stack, test_fw, learnings
|
||||
*/
|
||||
import { createHash } from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check';
|
||||
import { parseClassList, targetedBrief } from './review-targeted';
|
||||
|
||||
export const BRIEF_ROLES = ['specialist', 'specialist-batched', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check'] as const;
|
||||
export const BRIEF_ROLES = ['specialist', 'specialist-batched', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted'] as const;
|
||||
export type BriefRole = typeof BRIEF_ROLES[number];
|
||||
export type Slots = Record<string, string | undefined>;
|
||||
|
||||
@@ -119,6 +121,14 @@ function renderRole(role: BriefRole, slots: Slots, t: SectionTexts): string {
|
||||
const body = fill(t.outside, [['DIFF_BASE=$(git merge-base origin/<base> HEAD)', `DIFF_BASE=${diffBase}`]], role);
|
||||
return [body, '', `## Branch diff (git diff ${diffBase})`, FENCE + 'diff', (slots.diff ?? '').trim(), FENCE].join('\n');
|
||||
}
|
||||
case 'targeted': {
|
||||
const classes = parseClassList(need(slots, 'classes'));
|
||||
if (!classes?.length) throw new BriefError(`slot classes must list money, gate, auth or secret, got ${JSON.stringify(slots.classes)}`);
|
||||
let brief: string;
|
||||
try { brief = targetedBrief(need(slots, 'brief'), classes); } catch (e) { throw new BriefError((e as Error).message); }
|
||||
const body = fill(t.specialist, [['Read the checklist at {checklist path}', `Read the targeted brief above (${need(slots, 'checklist')}: ${classes.join(', ')})`]], role);
|
||||
return `${brief}\n\n${common(body, '')}`;
|
||||
}
|
||||
case 'fix-check':
|
||||
return renderFixCheckPrompt({ fixList: need(slots, 'fix_list'), touchedHunks: need(slots, 'hunks'), testResults: need(slots, 'tests'), callers: slots.callers ?? '' });
|
||||
}
|
||||
|
||||
@@ -22,6 +22,8 @@ export interface Collected extends ClassInput {
|
||||
stampOnly: string[];
|
||||
/** Added lines per path, for scanNumbers. */
|
||||
added: Record<string, string[]>;
|
||||
/** Deleted lines per path, for the targeted classes (lib/review-targeted.ts). */
|
||||
removed: Record<string, string[]>;
|
||||
diffLines: number;
|
||||
mergeBase: string | null;
|
||||
untracked: string[];
|
||||
@@ -52,11 +54,11 @@ export function loadRegistryAt(root: string, sha: string): RegistryInfo {
|
||||
}
|
||||
|
||||
/** Numstat, deletions and the -U0 patch for `range` (`[mergeBase, head]` or `[mergeBase]` for the working tree). */
|
||||
function readDiff(root: string, range: string[], errors: DepthError[]): Pick<Collected, 'perPath' | 'deleted' | 'stampOnly' | 'added'> {
|
||||
function readDiff(root: string, range: string[], errors: DepthError[]): Pick<Collected, 'perPath' | 'deleted' | 'stampOnly' | 'added' | 'removed'> {
|
||||
const numstat = git(root, ['diff', '--numstat', '-z', '--no-renames', ...range]);
|
||||
const status = git(root, ['diff', '--name-status', '-z', '--no-renames', '--diff-filter=D', ...range]);
|
||||
const patch = git(root, ['diff', '-U0', '--no-color', '--no-ext-diff', '--no-renames', ...range]);
|
||||
const empty = { perPath: [], deleted: [], stampOnly: [], added: {} };
|
||||
const empty = { perPath: [], deleted: [], stampOnly: [], added: {}, removed: {} };
|
||||
for (const [r, what] of [[numstat, '--numstat'], [status, '--name-status'], [patch, '-U0']] as const) {
|
||||
if (!r.ok) { errors.push({ code: 'numstat_failed', detail: oneLine(r.err || 'git diff failed'), input: `git diff ${what} ${range.join(' ')}`, repair: 'check that both commits exist locally (git fetch origin <base>) and rerun' }); return empty; }
|
||||
}
|
||||
@@ -69,6 +71,7 @@ function readDiff(root: string, range: string[], errors: DepthError[]): Pick<Col
|
||||
const statusParts = status.out.split('\0');
|
||||
const deleted = statusParts.flatMap((s, k) => (s === 'D' && statusParts[k + 1] ? [statusParts[k + 1]!] : [])).filter(p => !scratch(p));
|
||||
const added: Record<string, string[]> = {};
|
||||
const removed: Record<string, string[]> = {};
|
||||
const changed: Record<string, string[]> = {};
|
||||
let cur = '';
|
||||
let minus = '';
|
||||
@@ -83,10 +86,11 @@ function readDiff(root: string, range: string[], errors: DepthError[]): Pick<Col
|
||||
}
|
||||
if (!cur || scratch(cur) || line.startsWith('@@')) continue;
|
||||
if (line[0] === '+') (added[cur] ??= []).push(line.slice(1));
|
||||
if (line[0] === '-') (removed[cur] ??= []).push(line.slice(1));
|
||||
if (line[0] === '+' || line[0] === '-') (changed[cur] ??= []).push(line);
|
||||
}
|
||||
const stampOnly = perPath.map(e => e.path).filter(p => p.endsWith('package.json') && !!changed[p]?.length && changed[p]!.every(l => /^[+-]\s*"version"\s*:\s*"[^"]*"\s*,?\s*$/.test(l)));
|
||||
return { perPath, deleted, stampOnly, added };
|
||||
return { perPath, deleted, stampOnly, added, removed };
|
||||
}
|
||||
|
||||
/** More data candidates than this keep every data file code: the consumer scan is not run. */
|
||||
@@ -154,7 +158,7 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C
|
||||
const mb = git(root, ['merge-base', baseSha, headSha]);
|
||||
if (!mb.ok) {
|
||||
errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || 'no merge base'), input: `git merge-base ${baseSha} ${headSha}`, repair: 'check that both commits exist locally (git fetch origin <base>) and rerun' });
|
||||
return result({ perPath: [], deleted: [], stampOnly: [], added: {} }, { root: [], registry, errors, mergeBase: null, untracked: [], consumed: 'all' });
|
||||
return result({ perPath: [], deleted: [], stampOnly: [], added: {}, removed: {} }, { root: [], registry, errors, mergeBase: null, untracked: [], consumed: 'all' });
|
||||
}
|
||||
const mergeBase = mb.out.trim();
|
||||
const diff = readDiff(root, [mergeBase, headSha], errors);
|
||||
@@ -172,7 +176,7 @@ export function collectLive(root: string, base: string): Collected {
|
||||
const mb = git(root, ['merge-base', ref, 'HEAD']);
|
||||
if (!mb.ok) {
|
||||
errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || `no merge base with ${ref}`), input: `git merge-base ${ref} HEAD`, repair: `fetch the base first (git fetch origin ${base})` });
|
||||
return result({ perPath: [], deleted: [], stampOnly: [], added: {} }, { root, registry, errors, mergeBase: null, untracked: [], consumed: 'all' });
|
||||
return result({ perPath: [], deleted: [], stampOnly: [], added: {}, removed: {} }, { root, registry, errors, mergeBase: null, untracked: [], consumed: 'all' });
|
||||
}
|
||||
const mergeBase = mb.out.trim();
|
||||
const diff = readDiff(root, [mergeBase], errors);
|
||||
|
||||
+9
-3
@@ -18,7 +18,7 @@ export type DiffClass = 'docs' | 'tests' | 'code' | 'gate';
|
||||
export type PathClass = 'docs' | 'tests' | 'code';
|
||||
export type Depth = 'light' | 'standard' | 'full';
|
||||
export type Effort = 'light' | 'medium' | 'full' | 'host';
|
||||
export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check';
|
||||
export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check' | 'targeted';
|
||||
export type Step = 'full' | 'scoped' | 'fix-check' | 'done' | 'stop';
|
||||
|
||||
export const SPECIALISTS = ['testing', 'maintainability', 'security', 'performance', 'data-migration', 'api-contract', 'design', 'simplification', 'state-machines'] as const;
|
||||
@@ -38,7 +38,7 @@ export const BATCH_SCOPED: readonly Specialist[] = ['data-migration', 'api-contr
|
||||
* them; the Red Team, the native adversarial pass and the fix-check at full (the lane's reasoning); the outside voice at
|
||||
* its own provider's. Light's batched specialist carries the Red Team prompt, so it stays full.
|
||||
*/
|
||||
export const ROLE_EFFORT = { specialist: 'medium', 'red-team': 'full', 'adversarial-native': 'full', 'fix-check': 'full', 'outside-voice': 'host' } as const satisfies Record<Role, Effort>;
|
||||
export const ROLE_EFFORT = { specialist: 'medium', 'red-team': 'full', 'adversarial-native': 'full', 'fix-check': 'full', 'outside-voice': 'host', targeted: 'full' } as const satisfies Record<Role, Effort>;
|
||||
/** Why light with runtime above 0 keeps the native adversarial pass as its own dispatch. */
|
||||
export const LIGHT_ADVERSARIAL_REASON = 'backtest inconclusive (GSTA-147 D5)';
|
||||
const CLASS_RANK: Record<DiffClass, number> = { docs: 0, tests: 1, code: 2, gate: 3 };
|
||||
@@ -53,7 +53,7 @@ const RELEASE_METADATA = [/^CHANGELOG\.md$/, /^VERSION$/, /^agents-digest\//];
|
||||
/** Gate paths, the review-depth machinery included: an edit to how much review runs gets the most review. */
|
||||
const GATE_PATH = [
|
||||
/(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/,
|
||||
/^lib\/review-(depth|collect|scope)\.ts$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/,
|
||||
/^lib\/review-(depth|collect|scope|targeted)\.ts$/, /^review\/specialists\/targeted\.md$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/,
|
||||
];
|
||||
|
||||
/** Data files: JSON, CSV, TSV and raster images. YAML, TOML and SVG are never data here: deploy configs, API specs and executable SVG stay code. */
|
||||
@@ -291,6 +291,8 @@ export interface PlanInput extends ClassInput {
|
||||
floor?: Depth;
|
||||
/** false under --no-tiers or review_effort_tiers off: specialists run at the host's default effort. */
|
||||
tiers?: boolean;
|
||||
/** The targeted classes (lib/review-targeted.ts) and whether the targeted reader already read this product patch. */
|
||||
targeted?: { classes: readonly string[]; ran?: boolean };
|
||||
}
|
||||
/** One reviewer. `exclude` holds git pathspec argv items the reviewer's diff leaves out; `checklists` names what the batched specialist reads. */
|
||||
export interface Dispatch { role: Role; name: string; effort: Effort; exclude: string[]; checklists?: string[]; reason?: string }
|
||||
@@ -432,6 +434,10 @@ export function planReview(i: PlanInput): ReviewPlan {
|
||||
add('fix-check', 'fix-check', 'full');
|
||||
if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host');
|
||||
}
|
||||
if (i.targeted?.classes.length && ['full', 'scoped', 'fix-check'].includes(next.step)) {
|
||||
if (i.targeted.ran) skipped.push('targeted (already read this product patch)');
|
||||
else add('targeted', 'targeted', ROLE_EFFORT.targeted);
|
||||
}
|
||||
return { ...depth, stepDepth, depthReasons, reasons: c.reasons, next, numbers: i.numbers, dispatches, skipped };
|
||||
}
|
||||
|
||||
|
||||
+4
-4
@@ -35,10 +35,10 @@ export interface ScopeFinding {
|
||||
export interface ScopedFinding extends ScopeFinding { locus: Locus; kind: Kind; blocking: boolean; scope: ScopeAction; raised_by?: string }
|
||||
|
||||
/** Who can raise a finding: the core checklist, QA probes, a specialist by name and the review-depth roles; anything else is `unknown`. */
|
||||
const RAISERS: readonly string[] = ['core', 'qa', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', ...SPECIALISTS];
|
||||
const RAISERS: readonly string[] = ['core', 'qa', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted', ...SPECIALISTS];
|
||||
const RAISER_ALIASES: Record<string, string> = { adversarial: 'adversarial-native' };
|
||||
/** Reviewers that read after pass 1 finished (Step 11's voices, a fix-check): only a blocking finding of theirs is ever `fix`. */
|
||||
const AFTER_PASS_RAISERS: readonly string[] = ['adversarial-native', 'outside-voice', 'fix-check'];
|
||||
/** Reviewers that read after pass 1 finished (Step 11's voices and the targeted reader, a fix-check): only a blocking finding of theirs is ever `fix`. */
|
||||
const AFTER_PASS_RAISERS: readonly string[] = ['adversarial-native', 'outside-voice', 'fix-check', 'targeted'];
|
||||
|
||||
/** The reviewer that raised a finding, for attributing passes later (GSTA-152 D6): its `raised_by` when known, else its `specialist` tag, else `unknown`. */
|
||||
export function raisedBy(finding: ScopeFinding): string {
|
||||
@@ -297,7 +297,7 @@ export interface ReviewLoopSummary {
|
||||
passes: number; blocking_found: number; deferred: number; noted: number; fixed: number; scope: ReviewScopePolicy | 'unknown';
|
||||
/** One-reviewer re-checks of a fix (docs/review-policy.md#transitions); never passes. */
|
||||
fix_checks?: number;
|
||||
/** Reviewer dispatches of this invocation by role (specialist, red-team, adversarial-native, outside-voice, fix-check). */
|
||||
/** Reviewer dispatches of this invocation by role (specialist, red-team, adversarial-native, outside-voice, fix-check, targeted). */
|
||||
dispatches?: Record<string, number>;
|
||||
/** Prompt plus diff bytes the dispatches read, / 4: a relative estimate, never a bill. */
|
||||
est_input_tokens?: number;
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
/**
|
||||
* review-targeted — the targeted review pass on money, gate, auth and secret code
|
||||
* (docs/review-policy.md#targeted-pass-on-money-gate-auth-and-secret-code; GST-257).
|
||||
* A pure function from a collected diff (lib/review-collect.ts: changed paths with
|
||||
* their added and deleted lines), gstack-diff-scope's flags and the gate scan to the
|
||||
* classes the diff touches. bin/gstack-review-plan prints them and dispatches one
|
||||
* `targeted` reader with the matching blocks of review/specialists/targeted.md;
|
||||
* gstack-ship-receipt records the pass and gstack-ship-queue reports it.
|
||||
*
|
||||
* Matching reads added and deleted lines, so removing a cap, a check or a redaction
|
||||
* counts. Path and identifier signals are token-bounded: `costume` never names cost.
|
||||
* Docs and tests paths never trigger. Any input the classifier cannot read fails
|
||||
* closed to all four classes, as a depth error fails the depth closed to full.
|
||||
*
|
||||
* The gate class is the gate scan's tagged hunks (a relaxed threshold, a skipped test, a
|
||||
* loosened check), not GATE_PATH: over the last 100 non-release commits on main, GATE_PATH
|
||||
* alone hit 43 (the precision gate allows 25), and those paths already get full depth.
|
||||
*/
|
||||
import { createHash } from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { classifyPath } from './review-depth';
|
||||
|
||||
export const TARGETED_CLASSES = ['money', 'gate', 'auth', 'secret'] as const;
|
||||
export type TargetedClass = (typeof TARGETED_CLASSES)[number];
|
||||
|
||||
const MONEY_PATH_TOKENS = ['cost', 'costs', 'spend', 'spending', 'budget', 'budgets', 'ledger', 'billing', 'price', 'prices', 'pricing', 'fee', 'fees', 'invoice', 'invoices'];
|
||||
/** Identifiers ending in _usd, Usd or cap_usd, any spend_cap spelling, and paid-provider call sites (the providers lib/pricing.ts prices). */
|
||||
const MONEY_LINE = [
|
||||
/\b[A-Za-z0-9_]*_usd\b/i, /\b[a-z][A-Za-z0-9]*Usd\b/, /\bspend[_-]?cap\b/i,
|
||||
/\bapi\.(?:anthropic|openai|voyageai)\.com\b/, /\bgenerativelanguage\.googleapis\.com\b/, /\bopenrouter\.ai\b/,
|
||||
/\bnew\s+(?:Anthropic|OpenAI|AnthropicBedrock|AnthropicVertex)\s*\(/, /\.(?:messages|chat\.completions|responses|embeddings)\.create\s*\(/,
|
||||
];
|
||||
const AUTH_PATH_TOKENS = ['auth', 'authn', 'authz', 'oauth', 'oauth2', 'jwt', 'session', 'sessions', 'permission', 'permissions', 'role', 'roles'];
|
||||
const AUTH_PATH_PREFIXES = ['authenticat', 'authoriz'];
|
||||
const SECRET_PATH = /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i;
|
||||
/** An environment name whose underscore-separated segments include a key word: OPENAI_API_KEY, GH_TOKEN; never MAX_TOKENS. */
|
||||
const KEY_NAME = String.raw`(?:[A-Z0-9]+_)*(?:API_?KEY|SECRET|TOKEN|PASSWORD|PASSWD|PRIVATE_KEY|ACCESS_KEY|CREDENTIALS?)(?:_[A-Z0-9]+)*\b`;
|
||||
const SECRET_LINE = [
|
||||
new RegExp(String.raw`\b(?:process\.env|Bun\.env|import\.meta\.env)(?:\.|\[\s*['"\x60])${KEY_NAME}`),
|
||||
new RegExp(String.raw`\b(?:os\.environ(?:\.get\(|\[)|os\.getenv\(|ENV(?:\.fetch\(|\[))\s*['"]${KEY_NAME}`),
|
||||
new RegExp(String.raw`\$\{?${KEY_NAME}`),
|
||||
/\b(?:keychain|keytar|find-generic-password|find-internet-password|secret-tool|git-credential|credential\.helper)\b/i, /\bgh auth token\b/, /(?:^|[\s'"/])\.netrc\b/,
|
||||
];
|
||||
|
||||
/** 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, 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}`)]),
|
||||
})).digest('hex').slice(0, 16);
|
||||
|
||||
export interface TargetedInput {
|
||||
paths: readonly string[];
|
||||
/** Added and deleted lines per path (lib/review-collect.ts). */
|
||||
added?: Record<string, readonly string[]>;
|
||||
removed?: Record<string, readonly string[]>;
|
||||
/** gstack-diff-scope output (SCOPE_* keys); missing or SCOPE_ERROR fails closed. */
|
||||
scope?: Record<string, string>;
|
||||
/** Gate hunks the gate scan tagged; null when it did not run, which fails closed once a code path changed. */
|
||||
gateTagged: number | null;
|
||||
root?: string | readonly string[];
|
||||
/** Collector failures (a diff git could not list). */
|
||||
errors?: readonly { code: string; input: string }[];
|
||||
}
|
||||
export interface TargetedResult { classes: TargetedClass[]; reasons: string[]; errors: string[]; rules: string }
|
||||
|
||||
const tokens = (p: string) => p.replace(/([a-z0-9])([A-Z])/g, '$1 $2').toLowerCase().split(/[^a-z0-9]+/).filter(Boolean);
|
||||
|
||||
/** The classes a diff touches, in TARGETED_CLASSES order, with one reason per class; any input error returns all four. */
|
||||
export function targetedClasses(i: TargetedInput): TargetedResult {
|
||||
const errors = (i.errors ?? []).map(e => `${e.code} (${e.input})`);
|
||||
const s = i.scope;
|
||||
if (!s) errors.push('scope unavailable (gstack-diff-scope did not run)');
|
||||
else if (s.SCOPE_ERROR) errors.push(`scope error: ${s.SCOPE_ERROR}`);
|
||||
const code = i.paths.filter(p => classifyPath(p, i.root) === 'code');
|
||||
if (code.length && i.gateTagged === null) errors.push('gate scan unavailable');
|
||||
if (errors.length) return { classes: [...TARGETED_CLASSES], reasons: errors.map(e => `fail-closed: ${e}`), errors, rules: TARGETED_RULES };
|
||||
const why = new Map<TargetedClass, string>();
|
||||
const hit = (c: TargetedClass, reason: string) => { if (!why.has(c)) why.set(c, reason); };
|
||||
if ((i.gateTagged ?? 0) > 0) hit('gate', `gate scan tagged ${i.gateTagged} hunk(s)`);
|
||||
for (const p of code) {
|
||||
const t = tokens(p);
|
||||
if (s!.SCOPE_AUTH === 'true' && 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 (t.some(x => MONEY_PATH_TOKENS.includes(x))) hit('money', `money path: ${p}`);
|
||||
for (const line of [...(i.added?.[p] ?? []), ...(i.removed?.[p] ?? [])]) {
|
||||
if (!why.has('money') && MONEY_LINE.some(r => r.test(line))) hit('money', `money line: ${p}`);
|
||||
if (!why.has('secret') && SECRET_LINE.some(r => r.test(line))) hit('secret', `secret line: ${p}`);
|
||||
}
|
||||
}
|
||||
const classes = TARGETED_CLASSES.filter(c => why.has(c));
|
||||
return { classes, reasons: classes.map(c => why.get(c)!), errors: [], rules: TARGETED_RULES };
|
||||
}
|
||||
|
||||
/** The static brief every targeted reader gets its blocks from; its sha256 is the `brief` the plan prints and the receipt records. */
|
||||
export const TARGETED_BRIEF_FILE = path.join(import.meta.dir, '..', 'review', 'specialists', 'targeted.md');
|
||||
export function targetedBriefSha(file = TARGETED_BRIEF_FILE): string {
|
||||
try { return createHash('sha256').update(fs.readFileSync(file)).digest('hex'); } catch { return 'missing'; }
|
||||
}
|
||||
|
||||
/** A class list as printed and recorded: comma-joined, or `none`. */
|
||||
export const classList = (classes: readonly string[]) => (classes.length ? classes.join(',') : 'none');
|
||||
|
||||
/** Parse a printed class list (`money,gate` or `none`); an unknown name gives null. */
|
||||
export function parseClassList(text: string): TargetedClass[] | null {
|
||||
const names = text.trim() === 'none' ? [] : text.split(',').map(s => s.trim()).filter(Boolean);
|
||||
if (names.some(n => !(TARGETED_CLASSES as readonly string[]).includes(n))) return null;
|
||||
return TARGETED_CLASSES.filter(c => names.includes(c));
|
||||
}
|
||||
|
||||
/** The brief a targeted reader gets: review/specialists/targeted.md's preamble plus the `## <class>` block of each class that hit. */
|
||||
export function targetedBrief(text: string, classes: readonly TargetedClass[]): string {
|
||||
const parts = text.replace(/\r\n/g, '\n').split(/\n(?=## )/);
|
||||
const blocks = new Map(parts.slice(1).map(b => [/^## (\w+)/.exec(b)?.[1] ?? '', b.trimEnd()]));
|
||||
const missing = classes.filter(c => !blocks.has(c));
|
||||
if (!classes.length || missing.length) throw new Error(`targeted brief: ${classes.length ? `no block for ${missing.join(', ')}` : 'no class to brief'}`);
|
||||
return [parts[0]!.trimEnd(), ...classes.map(c => blocks.get(c)!)].join('\n\n');
|
||||
}
|
||||
+1
-1
@@ -29,7 +29,7 @@ import { SMALL_DIFF_LINES, type Role } from './review-depth';
|
||||
|
||||
export const SHIP_STEPS = ['0.9', '1', '2', '3', '4', '5', '6', '6.5', '7', '8', '9', '10', '11', '11.5', '12', '13', '14', '14.5', '15', '16', '17', '18', '19', '20', '21', '22'] as const;
|
||||
/** Reviewer roles plus `docs`, the Step 14.5 documentation audit: each is one reader priced and reconciled the same way. */
|
||||
export const DISPATCH_ROLES = ['specialist', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'docs'] as const satisfies readonly (Role | 'docs')[];
|
||||
export const DISPATCH_ROLES = ['specialist', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted', 'docs'] as const satisfies readonly (Role | 'docs')[];
|
||||
type MissingRole = Exclude<Role, typeof DISPATCH_ROLES[number]>;
|
||||
const ROLES_COMPLETE: [MissingRole] extends [never] ? true : never = true;
|
||||
void ROLES_COMPLETE;
|
||||
|
||||
@@ -365,6 +365,7 @@ function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>, dry
|
||||
}
|
||||
const shipped = bindShippedReceipt(ctx.repoRoot, `refs/remotes/${env.remote}/${ctx.base}`, info.body, p.pr, [info.headRefOid]);
|
||||
if (!shipped.ok) return outRefused({ code: shipped.code, message: shipped.message }, [shipped.line, ...(shipped.depthLine ? [shipped.depthLine] : [])], `#${p.pr}'s lane runs /ship, then ${Q} ready --pr ${p.pr} --screen|--no-screen; then ${Q} train build --prs <pins>`, 'none');
|
||||
lines.push(`${shipped.targetedLine} pr=${p.pr}`);
|
||||
const ready = loadReady(env.repo.keys, p.pr, info.headRefOid);
|
||||
if (!ready) return outRefused({ code: 'QUEUE_NOT_READY', message: `no READY receipt for #${p.pr} at ${info.headRefOid.slice(0, 12)} (local state or ${'refs/notes/gstack-trains'})` }, [], `the lane runs ${Q} ready --pr ${p.pr} --screen|--no-screen`, 'none');
|
||||
let screen = ready.screen;
|
||||
|
||||
@@ -271,6 +271,7 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate
|
||||
gateLine(policy, attempt),
|
||||
receipt.line,
|
||||
...(receipt.depthLine ? [receipt.depthLine] : []),
|
||||
...(receipt.targetedLine ? [receipt.targetedLine] : []),
|
||||
];
|
||||
const gate = policy.policy.full_e2e ? gateEvidence(ctx, o.pr, head, attempt, o.gateExit, o.gateLog) : null;
|
||||
if (gate) lines.push(gate.line);
|
||||
|
||||
+57
-13
@@ -11,7 +11,10 @@
|
||||
* `review_loop_exception`: the owner's acceptance of a stop after a
|
||||
* fix-check edit under frozen scope (#3559), shaped like the docs exception.
|
||||
* The queue recomputes the depth the head needs from git objects
|
||||
* (lib/review-collect.ts) and refuses a receipt below it. Kept beside
|
||||
* (lib/review-collect.ts) and refuses a receipt below it.
|
||||
* `review_targeted`: the targeted pass on money, gate, auth and secret code
|
||||
* (lib/review-targeted.ts), which the queue reports beside the depth and does
|
||||
* not refuse on yet (GST-257 PR 1). Kept beside
|
||||
* lib/ship-receipt.ts so that module stays under the size ratchet.
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process';
|
||||
@@ -19,6 +22,7 @@ import * as path from 'node:path';
|
||||
import type { ShipReceipt } from './headless-artifacts';
|
||||
import { collectCommit } from './review-collect';
|
||||
import { depthOfCommitInput, gateScanNeeded, type Depth } from './review-depth';
|
||||
import { TARGETED_CLASSES, classList, targetedClasses, type TargetedClass } from './review-targeted';
|
||||
import { runScriptSync } from './script-invocation';
|
||||
import { bindReceipt, type ReceiptBindingCode } from './ship-receipt';
|
||||
import { docsExceptionLine, parseReceiptDocs } from './ship-receipt-docs';
|
||||
@@ -122,8 +126,46 @@ export function exceptionProblems(body: string, r: ShipReceipt, isAncestor?: (co
|
||||
return out;
|
||||
}
|
||||
|
||||
export type ReviewTargeted = NonNullable<ShipReceipt['review_targeted']>;
|
||||
|
||||
/** `write --review-targeted` input (gstack-review-dispatch verify --targeted-out), bound to the receipt's product patch-id here. */
|
||||
export function parseReviewTargeted(value: unknown, productPatchId: string): { ok: true; targeted: ReviewTargeted } | { ok: false; message: string } {
|
||||
const bad = (message: string) => ({ ok: false as const, message });
|
||||
if (!value || typeof value !== 'object' || Array.isArray(value)) return bad('review_targeted must be { classes, rules, brief_sha, status, findings, raised_by }');
|
||||
const v = value as Record<string, unknown>;
|
||||
for (const key of Object.keys(v)) if (!['classes', 'rules', 'brief_sha', 'status', 'findings', 'raised_by'].includes(key)) return bad(`review_targeted: unknown key ${JSON.stringify(key)}${key === 'product_patch_id' ? ' (write binds the pass to the receipt\'s product_patch_id)' : ''}`);
|
||||
if (!Array.isArray(v.classes) || !v.classes.every(c => (TARGETED_CLASSES as readonly unknown[]).includes(c))) return bad(`review_targeted.classes must list ${TARGETED_CLASSES.join(', ')}`);
|
||||
if (!text(v.rules) || !text(v.raised_by)) return bad('review_targeted.rules and raised_by must be non-empty one-line strings');
|
||||
if (typeof v.brief_sha !== 'string' || !/^[0-9a-f]{64}$/.test(v.brief_sha)) return bad('review_targeted.brief_sha must be the sha256 of review/specialists/targeted.md');
|
||||
if (!['done', 'failed', 'not-run'].includes(v.status as string)) return bad('review_targeted.status must be done, failed or not-run');
|
||||
if (typeof v.findings !== 'number' || !Number.isInteger(v.findings) || v.findings < 0) return bad('review_targeted.findings must be an integer >= 0');
|
||||
const classes = TARGETED_CLASSES.filter(c => (v.classes as string[]).includes(c));
|
||||
return { ok: true, targeted: { classes, rules: v.rules as string, brief_sha: v.brief_sha, product_patch_id: productPatchId, status: v.status as ReviewTargeted['status'], findings: v.findings, raised_by: (v.raised_by as string).trim() } };
|
||||
}
|
||||
|
||||
/** The queue's report-only line: the classes the head needs against the classes a done targeted pass covered; never a refusal (GST-257 PR 1). */
|
||||
export function receiptTargetedLine(required: readonly TargetedClass[], r: ShipReceipt): string {
|
||||
const rt = r.review_targeted;
|
||||
const covered: readonly string[] = rt?.status === 'done' ? rt.classes : [];
|
||||
const ok = required.every(c => covered.includes(c));
|
||||
return `REVIEW_TARGETED: required=${classList(required)} receipt=${rt ? classList(covered) : 'legacy'} verdict=${ok ? 'ok' : 'missing'}`;
|
||||
}
|
||||
|
||||
export interface DepthVerdict { ok: boolean; line: string; message?: string }
|
||||
|
||||
/** The head's committed range against the base, read once for the depth verdict and the targeted line. */
|
||||
interface HeadInputs { baseSha: { ok: boolean; out: string }; c: ReturnType<typeof collectCommit>; scope: Record<string, string> }
|
||||
function headInputs(root: string, baseRef: string, head: string): HeadInputs {
|
||||
const baseSha = git(root, ['rev-parse', '--verify', '--quiet', `${baseRef}^{commit}`]);
|
||||
const baseRev = baseSha.ok ? baseSha.out : baseRef;
|
||||
return { baseSha, c: collectCommit(root, baseRev, head), scope: scopeOfRange(root, BIN_DIR, baseRev, head) };
|
||||
}
|
||||
/** The receipt's recorded gate scan when it names this head; otherwise null (unavailable). */
|
||||
const boundScan = (r: ShipReceipt, head: string): number | null => {
|
||||
const g = r.review_depth?.gate_scan;
|
||||
return g && g.tagged !== null && /^[0-9a-f]{40,64}$/.test(g.head) && g.head === head.toLowerCase() ? g.tagged : null;
|
||||
};
|
||||
|
||||
/**
|
||||
* The depth the bound head needs against `baseRef`, recomputed from git
|
||||
* objects; the recorded gate scan counts only when it names the receipt's
|
||||
@@ -131,14 +173,13 @@ export interface DepthVerdict { ok: boolean; line: string; message?: string }
|
||||
* with no review_depth and no gstack_version predates depth receipts and is
|
||||
* accepted as legacy; one from a writer that records depth but lacks it is not.
|
||||
*/
|
||||
export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceipt, head: string): DepthVerdict {
|
||||
export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceipt, head: string, at?: HeadInputs): DepthVerdict {
|
||||
const rd = r.review_depth;
|
||||
if (!rd && !r.gstack_version) return { ok: true, line: 'REVIEW_DEPTH: legacy receipt (written before receipts recorded review_depth); accepted' };
|
||||
const baseSha = git(root, ['rev-parse', '--verify', '--quiet', `${baseRef}^{commit}`]);
|
||||
const baseRev = baseSha.ok ? baseSha.out : baseRef;
|
||||
const c = collectCommit(root, baseRev, head);
|
||||
const scanBound = !!rd && rd.gate_scan.tagged !== null && /^[0-9a-f]{40,64}$/.test(rd.gate_scan.head) && rd.gate_scan.head === head.toLowerCase();
|
||||
const need = depthOfCommitInput({ ...c, scope: scopeOfRange(root, BIN_DIR, baseRev, head), gateTagged: scanBound ? rd!.gate_scan.tagged : null });
|
||||
const { baseSha, c, scope } = at ?? headInputs(root, baseRef, head);
|
||||
const tagged = boundScan(r, head);
|
||||
const scanBound = tagged !== null;
|
||||
const need = depthOfCommitInput({ ...c, scope, gateTagged: tagged });
|
||||
const have = rd?.depth;
|
||||
const inputs = `base@${short(baseSha.ok ? baseSha.out : null)},registry@${short(c.registry.sha)},gate_scan@${scanBound ? short(rd!.gate_scan.head) : 'missing'}`;
|
||||
const line = `REVIEW_DEPTH: receipt=${have ?? 'none'} required=${need.depth} runtime_lines=${need.runtimeLines} class=${need.class} reasons=${need.reasons.join(',')} inputs=${inputs}`;
|
||||
@@ -148,17 +189,20 @@ export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceip
|
||||
return { ok: false, line, message: `the receipt ${why}, but head ${head.slice(0, 12)} needs ${need.depth} (${need.reasons.join(', ')}${errs ? `; ${errs}` : ''}); the receipt is unchanged; run /ship on ${head.slice(0, 12)}` };
|
||||
}
|
||||
|
||||
/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept. */
|
||||
/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept; the targeted line is reported, never refused on. */
|
||||
export function bindShippedReceipt(root: string, baseRef: string, body: string, pr: string | undefined, accepted: readonly string[]):
|
||||
| { ok: true; receipt: ShipReceipt; head: string; line: string; depthLine: string }
|
||||
| { ok: false; code: ReceiptBindingCode | 'QUEUE_RECEIPT_DEPTH_RAISED' | 'QUEUE_RECEIPT_EXCEPTION_INVALID'; message: string; line: string; depthLine?: string } {
|
||||
| { ok: true; receipt: ShipReceipt; head: string; line: string; depthLine: string; targetedLine: string }
|
||||
| { ok: false; code: ReceiptBindingCode | 'QUEUE_RECEIPT_DEPTH_RAISED' | 'QUEUE_RECEIPT_EXCEPTION_INVALID'; message: string; line: string; depthLine?: string; targetedLine?: string } {
|
||||
const b = bindReceipt(body, pr, accepted);
|
||||
if (!b.ok) return b;
|
||||
const ancestor = (commit: string) => b.receipt.carry?.ancestry === 'rewritten' || git(root, ['merge-base', '--is-ancestor', commit, b.head]).ok;
|
||||
const problems = exceptionProblems(body, b.receipt, ancestor);
|
||||
if (problems.length) return { ok: false, code: 'QUEUE_RECEIPT_EXCEPTION_INVALID', message: `the receipt's owner exception does not stand: ${problems.join('; ')}`, line: b.line };
|
||||
const d = receiptDepthVerdict(root, baseRef, b.receipt, b.head);
|
||||
if (!d.ok) return { ok: false, code: 'QUEUE_RECEIPT_DEPTH_RAISED', message: d.message!, line: b.line, depthLine: d.line };
|
||||
return { ...b, depthLine: d.line };
|
||||
const at = headInputs(root, baseRef, b.head);
|
||||
const d = receiptDepthVerdict(root, baseRef, b.receipt, b.head, at);
|
||||
const gateTagged = boundScan(b.receipt, b.head) ?? (gateScanNeeded(at.c.paths, at.c.root) ? null : 0);
|
||||
const targetedLine = receiptTargetedLine(targetedClasses({ ...at.c, scope: at.scope, gateTagged }).classes, b.receipt);
|
||||
if (!d.ok) return { ok: false, code: 'QUEUE_RECEIPT_DEPTH_RAISED', message: d.message!, line: b.line, depthLine: d.line, targetedLine };
|
||||
return { ...b, depthLine: d.line, targetedLine };
|
||||
}
|
||||
|
||||
+1
-1
@@ -19,7 +19,7 @@ const KEY_ORDER: Array<keyof ShipReceipt | 'docs'> = [
|
||||
'schema_version', 'pr', 'head', 'version', 'base', 'gated_tree', 'predecessor', 'tree', 'gate_reuse', 'gate', 'ci', 'spend_usd',
|
||||
'session_kind', 'artifacts_consumed', 'run', 'thread', 'policy', 'queue_mode', 'preregistration', 'history',
|
||||
'head_tree', 'product_patch_id', 'gstack_version', 'review_depth', 'pregate', 'requires_remote', 'issues', 'approval', 'docs', 'review_loop',
|
||||
'review_loop_exception', 'carry',
|
||||
'review_loop_exception', 'review_targeted', 'carry',
|
||||
];
|
||||
|
||||
/** Keys in the fixed order so identifiers lead; undefined values are dropped. */
|
||||
|
||||
Reference in new issue
Block a user