mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-04 15:57:33 +01:00
fix(autoplan): export refuses artifact paths outside the run before writing; a deferred close keeps earlier undecided findings; reopening CEO grants Eng its rerun (review of #3234)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
68e15d9692
commit
a1f0d08ccd
5 files changed
+82
-14
No files matched your search
@@ -134,7 +134,8 @@ export const pendingDeltas = (state: RunState) => (state.capped_deltas ?? []).fi
|
||||
|
||||
/**
|
||||
* The owner's answers to cap items (deferred closes, deferred blockers, capped deltas) once the plan proceeds:
|
||||
* `b` grants that phase one more round, journaled as a budget change, and adds it (and Eng) to the reopen set;
|
||||
* `b` grants that phase one more round, journaled as a budget change, and adds it to the reopen set; Eng, which
|
||||
* reruns last, gets its own grant unless it already has one;
|
||||
* `a` accepts. Capped deltas record which.
|
||||
*/
|
||||
export function applyCapChoices(state: RunState, choices: Array<{ item: GateItem; chosen: string }>, reopen: Set<string>, by: string | undefined): string[] {
|
||||
@@ -150,8 +151,13 @@ export function applyCapChoices(state: RunState, choices: Array<{ item: GateItem
|
||||
(state.round_grants ??= {})[cap.phase] = (state.round_grants[cap.phase] ?? 0) + 1;
|
||||
journal(state.out, { event: 'budget_changed', by: by ?? 'gate', round_grant: { phase: cap.phase, cap: roundCap(state, cap.phase) }, item: item.id });
|
||||
reopen.add(cap.phase);
|
||||
if (state.phase_order.includes('eng')) reopen.add('eng');
|
||||
lines.push(`ROUND_GRANTED: ${cap.phase} cap=${roundCap(state, cap.phase)} (${item.id}b by ${by ?? 'gate'})`);
|
||||
if (cap.phase === 'eng' || !state.phase_order.includes('eng') || granted.has('eng')) continue;
|
||||
granted.add('eng');
|
||||
state.round_grants.eng = (state.round_grants.eng ?? 0) + 1;
|
||||
journal(state.out, { event: 'budget_changed', by: by ?? 'gate', round_grant: { phase: 'eng', cap: roundCap(state, 'eng') }, item: item.id, reason: `eng reruns after ${cap.phase}` });
|
||||
reopen.add('eng');
|
||||
lines.push(`ROUND_GRANTED: eng cap=${roundCap(state, 'eng')} (reruns after ${cap.phase}; ${item.id}b by ${by ?? 'gate'})`);
|
||||
}
|
||||
return lines;
|
||||
}
|
||||
@@ -74,7 +74,7 @@ type OpenRow = FindingRow & { urgent?: boolean; user_challenge?: boolean; settle
|
||||
* Open voices become skipped. Their dispatched or execution-unknown attempts are settled `unknown` at their
|
||||
* reservation (a deferred review may still have run and been billed); only reservations with no journaled dispatch
|
||||
* are released. The consensus file is written from the bound voices so export, validate and resume work. Open
|
||||
* findings (this round's bound rows, else the rows of the round last reviewed, minus answered ones) carry into
|
||||
* findings (this round's bound rows plus the unanswered rows of the round last reviewed) carry into
|
||||
* tasks.jsonl and the gate; Critical, urgent and user-challenge findings become owner approval items.
|
||||
*/
|
||||
export function closeDeferred(state: RunState, phase: Phase, o: { reason: string; by: string; cause: 'cap' | 'owner' }): { lines: string[]; blockers: string[] } {
|
||||
@@ -95,15 +95,15 @@ export function closeDeferred(state: RunState, phase: Phase, o: { reason: string
|
||||
const consensus = consensusRecord(phase, ps.native?.status === 'completed' ? { rows: native, model: ps.native.model } : null,
|
||||
ps.outside?.status === 'completed' ? { rows: outside, model: ps.outside.model, provider: ps.outside.provider } : null, reconcile(native, outside, []));
|
||||
const answered = settledIds(state);
|
||||
let rows: OpenRow[] = [...native, ...outside];
|
||||
const current: OpenRow[] = [...native, ...outside];
|
||||
const ids = new Set(current.map(r => r.id));
|
||||
const earlier: OpenRow[] = reviewRows(state, previousReview(state, phase)).filter(r => !answered.has(r.id) && !ids.has(r.id));
|
||||
const rows = [...current, ...earlier];
|
||||
let carried: string | undefined;
|
||||
if (!rows.length) {
|
||||
rows = reviewRows(state, previousReview(state, phase)).filter(r => !answered.has(r.id));
|
||||
if (rows.length) {
|
||||
carried = `${phase}-deferred.findings.jsonl`;
|
||||
fs.writeFileSync(path.join(state.out, carried), rows.map(r => JSON.stringify({ ...r, disposition: 'deferred' })).join('\n') + '\n');
|
||||
state.artifacts[carried] = { path: carried, sha256: sha256File(path.join(state.out, carried)) };
|
||||
}
|
||||
if (earlier.length) {
|
||||
carried = `${phase}-deferred.findings.jsonl`;
|
||||
fs.writeFileSync(path.join(state.out, carried), earlier.map(r => JSON.stringify({ ...r, disposition: 'deferred' })).join('\n') + '\n');
|
||||
state.artifacts[carried] = { path: carried, sha256: sha256File(path.join(state.out, carried)) };
|
||||
}
|
||||
const open = rows.filter(r => !r.settled_by && !answered.has(r.id));
|
||||
const counts = Object.fromEntries(['Critical', 'High', 'Medium', 'Low', 'Informational'].map(s => [s, open.filter(r => r.severity === s).length]));
|
||||
|
||||
@@ -147,10 +147,11 @@ export function exportPublic(state: RunState, dir: string, env: NodeJS.ProcessEn
|
||||
try {
|
||||
const files: Provenance['files'] = {};
|
||||
for (const [name, entry] of Object.entries(state.artifacts)) {
|
||||
const src = path.join(out, entry.path);
|
||||
const src = path.resolve(out, entry.path);
|
||||
const dest = path.resolve(staging, entry.path);
|
||||
if (!src.startsWith(out + path.sep) || !dest.startsWith(staging + path.sep)) throw new RunError('ARTIFACT_PATH_ESCAPE', `artifact ${name} path ${JSON.stringify(entry.path)} leaves the run directory; nothing was written`, 3);
|
||||
if (!fs.existsSync(src) || sha256File(src) !== entry.sha256) throw new RunError('ARTIFACT_STALE', `${entry.path} does not match run.json; run gstack-autoplan export --out ${out} first`, 3);
|
||||
const text = fs.readFileSync(src, 'utf8');
|
||||
const dest = path.join(staging, entry.path);
|
||||
fs.mkdirSync(path.dirname(dest), { recursive: true });
|
||||
fs.writeFileSync(dest, name === 'plan.md' ? rewriteProse(text, roots) : entry.path.endsWith('.json') ? JSON.stringify(rewriteJson(JSON.parse(text), roots), null, 2) + '\n' : rewriteRoots(text, roots));
|
||||
files[name] = { private_sha256: entry.sha256, public_sha256: sha256File(dest) };
|
||||
@@ -160,6 +161,8 @@ export function exportPublic(state: RunState, dir: string, env: NodeJS.ProcessEn
|
||||
manifest.provenance = { private_run: state.run, private_manifest_sha256: sha256File(privateRun), exported_at: new Date().toISOString(), roots: roots.map(r => r.label), files };
|
||||
fs.writeFileSync(path.join(staging, FILES.run), JSON.stringify(manifest, null, 2) + '\n');
|
||||
const check = validatePublic(staging, privateRun, env);
|
||||
const escapes = check.errors.filter(e => e.code === 'ARTIFACT_PATH_ESCAPE');
|
||||
if (escapes.length) throw new RunError('ARTIFACT_PATH_ESCAPE', escapes.map(e => `${e.path}: ${e.message}`).join('; '), 3);
|
||||
const leaks = check.errors.filter(e => e.code === 'ARTIFACT_MACHINE_PATH');
|
||||
if (leaks.length) throw new RunError('ARTIFACT_MACHINE_PATH', leaks.slice(0, 5).map(e => `${e.path}: ${e.message}`).join('; ') + (leaks.length > 5 ? ` (+${leaks.length - 5} more)` : ''), 3);
|
||||
fs.rmSync(target, { recursive: true, force: true });
|
||||
|
||||
@@ -127,7 +127,8 @@ describe('close --deferred', () => {
|
||||
expect(reply.status, reply.stderr).toBe(0);
|
||||
expect(reply.stdout).toMatch(/^ROUND_GRANTED: ceo cap=4 \(uc\d+b by human\)$/m);
|
||||
expect(reply.stdout).toMatch(/^REOPENED: ceo,eng/m);
|
||||
expect(readRunJson(s.out).round_grants).toEqual({ ceo: 1 });
|
||||
expect(reply.stdout).toMatch(/^ROUND_GRANTED: eng cap=4 \(reruns after ceo; uc\d+b by human\)$/m);
|
||||
expect(readRunJson(s.out).round_grants).toEqual({ ceo: 1, eng: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -177,6 +178,45 @@ describe('the round cap', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('deferred close keeps earlier open findings; a reopened phase reruns Eng', () => {
|
||||
test('a deferred confirm round with a new finding still carries the earlier undecided Critical to the gate and tasks', () => {
|
||||
const s = start();
|
||||
phaseVia(r, s.out, 'ceo', { native: [{ severity: 'Critical', title: 'Retries can double-charge', fix: 'Add a durable idempotency key' }], outside: [] });
|
||||
phaseVia(r, s.out, 'eng', NONE);
|
||||
expect(answer(s.out, readRunJson(s.out).gate_rev, 'p1d').status).toBe(0);
|
||||
expect(r.ap(['next', '--out', s.out]).status).toBe(0);
|
||||
const rr = readRunJson(s.out);
|
||||
const review = r.review(path.join(s.out, 'ceo-native-prompt.md'), { FIXTURE_FINDINGS: JSON.stringify([{ severity: 'Medium', title: 'Jitter source is unspecified' }]) });
|
||||
expect(review.status, review.stderr).toBe(0);
|
||||
const sub = r.ap(['submit', '--out', s.out, '--phase', 'ceo', '--voice', 'native', '--result', rr.phases.ceo.native.attempts.at(-1).result, '--model', 'anthropic/claude-opus-5-5', '--attempt', rr.phases.ceo.native.attempt]);
|
||||
expect(sub.status, sub.stderr).toBe(0);
|
||||
const closed = r.ap(['close', '--out', s.out, '--phase', 'ceo', '--deferred', 'owner pauses remaining reviews', '--by', 'human']);
|
||||
expect(closed.status, closed.stderr).toBe(0);
|
||||
expect(closed.stdout).toMatch(/^PHASE_DEFERRED: ceo round=2\/\S+ by=human open=2 \(Critical=1 Medium=1\) blockers=\S+-ceo-native-1 /m);
|
||||
phaseVia(r, s.out, 'eng', NONE);
|
||||
expect(gate(s.out).items.filter((i: any) => i.cap?.kind === 'blocker').map((i: any) => i.title)).toEqual([expect.stringContaining('Retries can double-charge')]);
|
||||
expect(readJsonl(path.join(s.out, 'tasks.jsonl')).map((t: any) => t.priority).sort()).toEqual(['Critical', 'Medium']);
|
||||
});
|
||||
|
||||
test('reopening a capped CEO round also grants Eng its round, so Eng reruns last instead of deferring', () => {
|
||||
const s = start(['--max-rounds', '1']);
|
||||
phaseVia(r, s.out, 'ceo', NONE);
|
||||
phaseVia(r, s.out, 'eng', NONE);
|
||||
expect(r.ap(['revise', '--out', s.out, '--plan', v2File(s.dir)]).status).toBe(0);
|
||||
const items = gate(s.out).items;
|
||||
const ceo = items.find((i: any) => i.cap?.kind === 'delta' && i.phase === 'ceo');
|
||||
const eng = items.find((i: any) => i.cap?.kind === 'delta' && i.phase === 'eng');
|
||||
const replied = answer(s.out, readRunJson(s.out).gate_rev, `${ceo.id}b ${eng.id}a p1a`);
|
||||
expect(replied.status, replied.stderr).toBe(0);
|
||||
expect(replied.stdout).toMatch(/^ROUND_GRANTED: eng cap=2 \(reruns after ceo; /m);
|
||||
phaseVia(r, s.out, 'ceo', NONE);
|
||||
const next = r.ap(['next', '--out', s.out]);
|
||||
expect(next.status, next.stderr).toBe(0);
|
||||
expect(next.stdout).toMatch(/^PHASE: eng /m);
|
||||
expect(next.stdout).not.toContain('PHASE_DEFERRED: eng');
|
||||
});
|
||||
});
|
||||
|
||||
describe('confirm rounds', () => {
|
||||
test('a reopened phase reviews the delta; a re-raised settled challenge is recorded settled unless it cites a changed line', () => {
|
||||
const s = start();
|
||||
|
||||
@@ -107,6 +107,25 @@ describe('export --public', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('export --public path containment', () => {
|
||||
test('an artifact path that leaves the run directory refuses before anything is written, outside or inside --public', () => {
|
||||
const { out, dir } = gatedRun();
|
||||
const text = 'synthetic artifact text\n';
|
||||
fs.writeFileSync(path.join(dir, 'sentinel.txt'), text);
|
||||
const victim = path.join(r.host.base, `victim${n}.txt`);
|
||||
fs.writeFileSync(victim, 'unrelated owner file\n');
|
||||
const state = readRunJson(out);
|
||||
state.artifacts['escape.txt'] = { path: `../../${path.basename(victim)}`, sha256: crypto.createHash('sha256').update(text).digest('hex') };
|
||||
fs.writeFileSync(path.join(out, 'run.json'), JSON.stringify(state) + '\n');
|
||||
const pub = path.join(dir, 'public');
|
||||
const x = r.ap(['export', '--out', out, '--public', pub]);
|
||||
expect(x.status, x.stdout + x.stderr).toBe(3);
|
||||
expect(x.stderr).toContain('ARTIFACT_PATH_ESCAPE');
|
||||
expect(fs.readFileSync(victim, 'utf8')).toBe('unrelated owner file\n');
|
||||
expect(fs.existsSync(pub)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('rewriteRoots', () => {
|
||||
test('a worktree path becomes repo-relative; a local root nested in the worktree still reads as local notes', () => {
|
||||
const roots = [{ label: 'run', root: '/w/repo/.context/run' }, { label: 'home', root: '/home/u' }, { label: 'worktree', root: '/w/repo' }];
|
||||
|
||||
Reference in new issue
Block a user