mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 10:40:32 +01:00
feat(ship): batched specialists per shard, audit skip at zero code paths, gate-scan summary for the parent (GSTA-167)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
1a558ded87
commit
bde74d6f33
17 files changed
+334
-101
No files matched your search
@@ -2,7 +2,8 @@
|
||||
* review-packet-collect — the parent's only read of a packet review
|
||||
* (docs/review-policy.md#reviewer-packets): every packet's output file, parsed and
|
||||
* validated as /ship Step 9.2 stages 1–3 require, merged by identity, and a record
|
||||
* under COLLECT_MAX_LINES lines. Missing or unusable output is incomplete coverage.
|
||||
* under COLLECT_MAX_LINES lines. Missing or unusable output is incomplete coverage, and so is
|
||||
* a batched packet whose COVERED line leaves out a checklist it inlined.
|
||||
*/
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
@@ -16,11 +17,12 @@ const LINE_MAX = 300;
|
||||
|
||||
export type Finding = Record<string, unknown> & { severity: 'CRITICAL' | 'INFORMATIONAL'; path: string; summary: string };
|
||||
export type ReviewerStatus = 'clean' | 'findings' | 'missing' | 'invalid' | 'pending';
|
||||
export interface ReviewerResult { packet: PacketEntry; status: ReviewerStatus; findings: Finding[]; records: number; invalid: number; recommendation?: string }
|
||||
export interface ReviewerResult { packet: PacketEntry; status: ReviewerStatus; findings: Finding[]; records: number; invalid: number; recommendation?: string; covered?: string[] }
|
||||
export interface Merged extends Finding { sources: string[] }
|
||||
|
||||
const str = (v: unknown) => typeof v === 'string' && v.trim().length > 0;
|
||||
const NO_FINDINGS = /^[\s`*]*NO[ _]FINDINGS[\s`*.]*$/i;
|
||||
const COVERED_LINE = /^[\s`*]*COVERED:\s*(.*?)[\s`*.]*$/i;
|
||||
const OUTSIDE_LINE = /^\s*(?:[-*]\s*)?\[(Critical|High|Medium|Low)\]\s+(\S+?)(?::(\d+))?\s+[—–-]\s+(.+)$/i;
|
||||
|
||||
/** A JSONL finding valid for its role, or null; the specialist roles need their whole required field set. */
|
||||
@@ -44,6 +46,8 @@ export function parseOutput(packet: PacketEntry, text: string | null): ReviewerR
|
||||
if (!line || line.startsWith('```')) continue;
|
||||
if (NO_FINDINGS.test(line)) { clean = true; continue; }
|
||||
if (/^Recommendation:/i.test(line)) { r.recommendation = line; continue; }
|
||||
const covered = COVERED_LINE.exec(line);
|
||||
if (covered) { r.covered = covered[1]!.split(',').map(c => c.trim()).filter(Boolean); continue; }
|
||||
let v: unknown;
|
||||
try { v = JSON.parse(line); } catch { r.invalid++; continue; }
|
||||
if (v && typeof v === 'object' && 'matrix' in v) { r.records++; continue; }
|
||||
@@ -51,7 +55,7 @@ export function parseOutput(packet: PacketEntry, text: string | null): ReviewerR
|
||||
if (!f) { r.invalid++; continue; }
|
||||
r.findings.push({ ...f, specialist: str(f.specialist) ? f.specialist : packet.reviewer });
|
||||
}
|
||||
r.status = r.findings.length ? 'findings' : clean || r.records ? 'clean' : 'invalid';
|
||||
r.status = r.findings.length ? 'findings' : clean || r.records || r.covered ? 'clean' : 'invalid';
|
||||
return r;
|
||||
}
|
||||
|
||||
@@ -106,6 +110,11 @@ export function mergeFindings(results: readonly ReviewerResult[]): Merged[] {
|
||||
const oneLine = (s: string) => (s.length > LINE_MAX ? s.slice(0, LINE_MAX - 1) + '…' : s);
|
||||
const where = (f: Finding) => `${f.path}${typeof f.line === 'number' ? `:${f.line}` : ''}`;
|
||||
|
||||
/** A batched specialist packet: one reader applying every checklist it names. */
|
||||
const isBatched = (p: PacketEntry) => p.role === 'specialist' && p.reviewer === 'batched';
|
||||
/** The checklists a batched packet inlined that its COVERED line does not name. */
|
||||
const uncovered = (r: ReviewerResult) => (isBatched(r.packet) && r.status !== 'missing' && r.status !== 'invalid' ? r.packet.checklists.filter(c => !(r.covered ?? []).includes(c)) : []);
|
||||
|
||||
export interface CollectResult { lines: string[]; complete: boolean; results: ReviewerResult[]; merged: Merged[] }
|
||||
|
||||
/** Read every packet's output in `dir`, write merged.jsonl and informational.md there, and return the compact record. */
|
||||
@@ -123,8 +132,13 @@ export function collect(dir: string): CollectResult | { error: string } {
|
||||
fs.writeFileSync(path.join(dir, 'informational.md'), `## Review notes (informational)\n\n${info.length ? info.map(note).join('\n') : 'None.'}\n`, { mode: 0o600 });
|
||||
const lines: string[] = [];
|
||||
const count = (r: ReviewerResult, s: string) => r.findings.filter(f => f.severity === s).length;
|
||||
const shown = results.length > 25 ? results.filter(r => r.status !== 'clean') : results;
|
||||
for (const r of shown) lines.push(`REVIEWER: ${r.packet.name} status=${r.status} crit=${count(r, 'CRITICAL')} info=${count(r, 'INFORMATIONAL')}${r.packet.fixtures === 'summary' ? ' fixtures=summary' : ''}${r.invalid ? ` invalid_lines=${r.invalid}` : ''}`);
|
||||
const shown = results.length > 25 ? results.filter(r => r.status !== 'clean' || uncovered(r).length) : results;
|
||||
const coverage = (r: ReviewerResult) => {
|
||||
if (!isBatched(r.packet)) return '';
|
||||
const missed = uncovered(r);
|
||||
return ` covered=${r.packet.checklists.filter(c => (r.covered ?? []).includes(c)).join(',') || 'none'}${missed.length ? ` uncovered=${missed.join(',')}` : ''}`;
|
||||
};
|
||||
for (const r of shown) lines.push(`REVIEWER: ${r.packet.name} status=${r.status} crit=${count(r, 'CRITICAL')} info=${count(r, 'INFORMATIONAL')}${coverage(r)}${r.packet.fixtures === 'summary' ? ' fixtures=summary' : ''}${r.invalid ? ` invalid_lines=${r.invalid}` : ''}`);
|
||||
for (const p of pending) lines.push(`REVIEWER: ${p.name} status=pending (build its packet with --findings ${path.join(dir, 'merged.jsonl')}, dispatch it, collect again)`);
|
||||
if (shown.length < results.length) lines.push(`REVIEWERS_CLEAN: ${results.length - shown.length} more, each status=clean`);
|
||||
for (const a of manifest.packets.filter(p => p.role === 'audit')) {
|
||||
@@ -135,7 +149,7 @@ export function collect(dir: string): CollectResult | { error: string } {
|
||||
lines.push(`AUDIT: ${a.name} status=${ok ? 'present' : last ? 'invalid' : 'missing'} out=${a.out}`);
|
||||
if (!ok) results.push({ packet: a, status: last ? 'invalid' : 'missing', findings: [], records: 0, invalid: 0 });
|
||||
}
|
||||
const incomplete = results.filter(r => r.status === 'missing' || r.status === 'invalid');
|
||||
const incomplete = results.filter(r => r.status === 'missing' || r.status === 'invalid' || uncovered(r).length);
|
||||
const tail = 6;
|
||||
const room = COLLECT_MAX_LINES - lines.length - tail;
|
||||
for (const m of crit.slice(0, Math.max(room, 0))) lines.push(oneLine(`CRITICAL: ${where(m)} [${m.sources.join('+')}]${typeof m.confidence === 'number' ? ` conf=${m.confidence}` : ''} ${String(m.summary).replace(/\s+/g, ' ')}${str(m.fix) ? ` fix: ${String(m.fix).replace(/\s+/g, ' ')}` : ''}`));
|
||||
@@ -144,6 +158,6 @@ export function collect(dir: string): CollectResult | { error: string } {
|
||||
for (const r of results.filter(x => x.recommendation).slice(0, 2)) lines.push(oneLine(`RECOMMENDATION: ${r.packet.name}: ${r.recommendation!.replace(/^Recommendation:\s*/i, '')}`));
|
||||
const invalid = results.reduce((n, r) => n + r.invalid, 0);
|
||||
lines.push(`COLLECT: reviewers=${reviewers.length} clean=${results.filter(r => r.status === 'clean').length} findings=${results.filter(r => r.status === 'findings').length} missing=${results.filter(r => r.status === 'missing').length} invalid=${results.filter(r => r.status === 'invalid').length} crit=${crit.length} info=${info.length} invalid_lines=${invalid} pending=${pending.length} merged=${path.join(dir, 'merged.jsonl')}`);
|
||||
if (incomplete.length) lines.push(resultLine('gstack-review-packet collect', 'REVIEW_COVERAGE_INCOMPLETE', oneLine(incomplete.map(r => `${r.packet.name}=${r.status}`).join(' ')).slice(0, 160)));
|
||||
if (incomplete.length) lines.push(resultLine('gstack-review-packet collect', 'REVIEW_COVERAGE_INCOMPLETE', oneLine(incomplete.map(r => `${r.packet.name}=${uncovered(r).length ? `uncovered:${uncovered(r).join(',')}` : r.status}`).join(' ')).slice(0, 160)));
|
||||
return { lines: lines.slice(0, COLLECT_MAX_LINES), complete: !incomplete.length, results, merged };
|
||||
}
|
||||
+35
-9
@@ -4,14 +4,15 @@
|
||||
* contract (lib/review-prompts.ts, the text the generated prose renders), its
|
||||
* checklists inlined, its DIFF_CMD diff at -U8 against the literal diff base with
|
||||
* a stat, at most 8 KB of the plan, a read budget, and the output path its result
|
||||
* goes to. A packet over PACKET_CAP_BYTES is sharded by path, never truncated.
|
||||
* goes to. A packet over PACKET_CAP_BYTES is sharded by path, never truncated. At every depth
|
||||
* a pass's specialist lines become one batched packet per shard (SPECIALIST_PACKING).
|
||||
* Selection is lib/review-depth.ts planReview's (through lib/review-plan-live.ts);
|
||||
* collect is lib/review-packet-collect.ts.
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import type { Dispatch, Role } from './review-depth';
|
||||
import { classifyPath, type Dispatch, type Role } from './review-depth';
|
||||
import * as P from './review-prompts';
|
||||
|
||||
export const PACKET_CAP_BYTES = 200_000;
|
||||
@@ -21,6 +22,31 @@ export const AUDITS = ['test-coverage', 'plan-completion'] as const;
|
||||
export type AuditName = (typeof AUDITS)[number];
|
||||
export type PacketRole = Role | 'audit';
|
||||
|
||||
export type Packing = 'batched' | 'parallel';
|
||||
export const PACKINGS: readonly Packing[] = ['batched', 'parallel'];
|
||||
/**
|
||||
* How build packs a pass's specialist DISPATCH lines: `batched` is one packet per shard inlining every
|
||||
* selected checklist (light's contract, each finding tagged with its checklist, a COVERED line collect
|
||||
* checks); `parallel` is one packet per specialist. The no-loss replay flips this one value.
|
||||
*/
|
||||
export const SPECIALIST_PACKING: Packing = 'batched';
|
||||
|
||||
/** Every per-specialist dispatch of a pass as one `specialist batched` dispatch in the first one's place: checklists in plan order, the diff all of them read (the excludes they share). */
|
||||
export function packSpecialists(dispatches: readonly Dispatch[]): Dispatch[] {
|
||||
const each = dispatches.filter(d => d.role === 'specialist' && !d.checklists);
|
||||
if (each.length < 2) return [...dispatches];
|
||||
const first = each[0]!;
|
||||
const batched: Dispatch = { role: 'specialist', name: 'batched', effort: first.effort, exclude: first.exclude.filter(x => each.every(d => d.exclude.includes(x))), checklists: each.map(d => d.name) };
|
||||
return dispatches.flatMap(d => (d === first ? [batched] : each.includes(d) ? [] : [d]));
|
||||
}
|
||||
|
||||
/** Data files: no code path for the coverage audit to trace. */
|
||||
const DATA_PATH = /\.(json|jsonl|ndjson|ya?ml|toml|csv|tsv|svg|png|jpe?g|gif|webp|ico|pdf)$/i;
|
||||
/** The changed paths the coverage audit traces: code-class paths that are not data. Docs, tests and data have none. */
|
||||
export function auditCodePaths(paths: readonly string[], root: string): string[] {
|
||||
return paths.filter(p => classifyPath(p, root) === 'code' && !DATA_PATH.test(p));
|
||||
}
|
||||
|
||||
export interface PacketEntry {
|
||||
role: PacketRole; name: string; reviewer: string; effort: string; file: string; out: string; bytes: number;
|
||||
checklists: string[]; fixtures: 'full' | 'summary'; shard: { index: number; total: number; oversize: boolean } | null;
|
||||
@@ -126,16 +152,16 @@ interface RoleText { title: string; instructions: string[]; contract: string; ch
|
||||
const doneLine = (name: string) => `DONE ${name} crit=<n> info=<n>`;
|
||||
|
||||
function roleText(d: Dispatch, ctx: BuildContext, testFw: string): RoleText {
|
||||
const jsonOut = (name: string) => `Write your result to the output path below with your file-write tool: \`NO FINDINGS\` alone, or the JSON lines and nothing else. Then reply with exactly one line, \`${doneLine(name)}\`, counting the CRITICAL and INFORMATIONAL findings you wrote. The parent reads only that file.`;
|
||||
const jsonOut = (name: string, covered?: readonly string[]) => `Write your result to the output path below with your file-write tool: \`NO FINDINGS\` alone, or the JSON lines and nothing else${covered ? `, then one last line \`COVERED: ${covered.join(',')}\` naming every checklist you applied (leave out one you did not apply: the parent counts it as missing coverage)` : ''}. Then reply with exactly one line, \`${doneLine(name)}\`, counting the CRITICAL and INFORMATIONAL findings you wrote. The parent reads only that file.`;
|
||||
const contract = P.SPECIALIST_CONTRACT.replace('{TEST_FW}', testFw);
|
||||
const base = { extra: [] as string[], diffLabel: 'Diff', summary: false, checklists: [] as string[] };
|
||||
switch (d.role) {
|
||||
case 'specialist': {
|
||||
if (d.checklists) return { ...base, title: 'batched specialist', checklists: d.checklists,
|
||||
instructions: [P.DEFENSIVE_REVIEW_FRAMING, `${P.SPECIALIST_INTRO} Apply every checklist below against the diff below; the red-team checklist goes last and looks for what the others missed.`],
|
||||
contract: `${contract}\n\nTag each finding with the checklist that found it: \`"specialist":"<checklist name>"\`.`, reply: jsonOut(d.name) };
|
||||
if (d.checklists) return { ...base, title: 'batched specialist', checklists: d.checklists, extra: d.checklists.includes('state-machines') ? [P.STATE_MATRIX_OUTPUT] : [],
|
||||
instructions: [P.DEFENSIVE_REVIEW_FRAMING, `${P.SPECIALIST_INTRO} Apply every checklist below against the diff below${d.checklists.includes('red-team') ? '; the red-team checklist goes last and looks for what the others missed' : ''}.`],
|
||||
contract: `${contract}\n\nTag each finding with the checklist that found it: \`"specialist":"<checklist name>"\`.`, reply: jsonOut(d.name, d.checklists) };
|
||||
const framed = d.name === 'security';
|
||||
const extra = d.name === 'state-machines' ? [`Your output file is also the matrix record file: write the matrix record(s) the checklist describes there with your findings; the parent runs \`gstack-state-matrix check\` on it.`] : [];
|
||||
const extra = d.name === 'state-machines' ? [P.STATE_MATRIX_OUTPUT] : [];
|
||||
return { ...base, title: `${d.name} specialist`, checklists: [d.name], summary: framed, extra,
|
||||
instructions: [...(framed ? [P.DEFENSIVE_REVIEW_FRAMING, P.FIXTURE_SUMMARY_PACKET] : []), `${P.SPECIALIST_INTRO} Apply the checklist below against the diff below.`],
|
||||
contract: `${contract}\n\nTag every finding \`"specialist":"${d.name}"\`.`, reply: jsonOut(d.name) };
|
||||
@@ -231,9 +257,9 @@ export function auditPrompt(skillRoot: string, audit: AuditName, base: string):
|
||||
|
||||
export function auditDraft(audit: AuditName, ctx: BuildContext, base: string, effort: string): PacketDraft {
|
||||
const { stack, testFw } = detectStack(ctx.root);
|
||||
const budget = 'The diff is below; do not rerun git diff. Read only files in the diff, their tests and the files the plan names, in named line ranges of at most 200 lines; no whole-repository search, no other packets, no transcript. Writes and test runs only as the audit instructions permit.';
|
||||
const budget = audit === 'test-coverage' ? P.AUDIT_READ_ONLY : 'The diff is below; do not rerun git diff. Read only files in the diff, their tests and the files the plan names, in named line ranges of at most 200 lines; no whole-repository search, no other packets, no transcript. Writes and test runs only as the audit instructions permit.';
|
||||
const plan = audit === 'plan-completion' && ctx.plan ? [`## Plan (whole, \`${ctx.plan.path}\`)\n\n${block(ctx.plan.text)}`, ''] : [];
|
||||
const head = [`Role: ${audit} audit (effort=${effort}).`, '', `## Instructions\n\n${auditPrompt(ctx.skillRoot, audit, base)}`, '',
|
||||
const head = [`Role: ${audit} audit (effort=${effort}).`, '', `## Instructions\n\n${auditPrompt(ctx.skillRoot, audit, base).replace('<allowed|audit-only>', 'audit-only')}`, '',
|
||||
`## Output\n\nWrite your whole final answer to the output path below, its LAST line the JSON object the instructions name. Then reply with exactly one line, \`DONE <NAME>\`.`, '',
|
||||
...(ctx.context ? [`## Parent context\n\n${block(ctx.context.trim())}`, ''] : []),
|
||||
...common({ ...ctx, plan: audit === 'plan-completion' ? undefined : ctx.plan }, stack, testFw, null, budget), ...plan];
|
||||
|
||||
@@ -76,6 +76,12 @@ export function renderFixCheckPrompt(i: FixCheckInput): string {
|
||||
/** What every reviewer packet allows beyond itself. */
|
||||
export const READ_BUDGET = 'Everything you need is in this packet. You may make at most 5 more reads, each a named line range of at most 200 lines of a file in this diff (code around a hunk, a caller the diff changed). No whole-repository search, no git commands, no other packets, no transcript, no web.';
|
||||
|
||||
/** The coverage audit packet's budget: an audit, never a writer; the parent writes and runs the tests it proposes. */
|
||||
export const AUDIT_READ_ONLY = 'This audit is read-only and takes three calls: read this packet, write the output file, reply with the DONE line. Make no edits, write no tests, run no tests, git or other commands and read no other file, whatever the audit instructions say about generating, running or reading: the parent writes and runs the tests. Judge coverage from the diff below and the tests it changes; put each test you would write, with its value card, in the diagram as a gap, mark a path whose tests this packet does not show as an unverified gap for the parent to confirm, and leave `tests_added` and `tests_extended` empty.';
|
||||
|
||||
/** A packet that applies the state-machines checklist writes its matrix records to its output file. */
|
||||
export const STATE_MATRIX_OUTPUT = 'Your output file is also the matrix record file: write the matrix record(s) the checklist describes there with your findings; the parent runs `gstack-state-matrix check` on it.';
|
||||
|
||||
/** The native adversarial pass writes JSONL like the specialists, plus a classification and its Recommendation line. */
|
||||
export const ADVERSARIAL_CONTRACT = 'Write each finding as one JSON object per line with the specialist fields (`"specialist":"adversarial"`) plus `"classification":"FIXABLE|INVESTIGATE"`, rated against the same floor (CRITICAL only for a test that would fail, a production break, or a security or data-loss defect); `NO FINDINGS` when nothing would fail. End with the one `Recommendation:` line either way.';
|
||||
|
||||
|
||||
Reference in new issue
Block a user