mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 06:54:50 +01:00
fix(ship): the start probe stops on an unreadable start directory, runs once per stage, and logStart is validated; brief wording stays main's (GST-776)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
a097560766
commit
f9ff08110d
5 files changed
+41
-26
No files matched your search
@@ -526,12 +526,13 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
|
||||
`COLLECT_WRONG_TREE`, a later commit in the same checkout `COLLECT_TREE_MOVED`; nothing
|
||||
is merged, the reader becomes `failed`, and `verify` reports `NOT_COLLECTED`. An
|
||||
outside voice's tree failure is recorded `unavailable` only when its reply holds no finding. The line catches a reader that
|
||||
worked elsewhere by accident; it is not proof of reading, since diff text can predict it. The brief asks for the bare
|
||||
line and gives the command only as the way to get it; a `printed:` label before the line is still read, and its tree
|
||||
and toplevel are still compared exactly (GST-776). Capy cannot start a subagent in a chosen directory, so before it
|
||||
takes a start token or sends a reader, the Capy pass program compares the checkout with the one shared readers start
|
||||
in (the thread's working directory) and stops when that is another checkout; a start directory outside any checkout,
|
||||
such as a multi-repo workspace root, holds no tree to review by mistake and dispatches. Step 11's voices take the `adversarial-review` start
|
||||
worked elsewhere by accident; it is not proof of reading, since diff text can predict it. A `printed:` label
|
||||
before the line (plain, bold or inside its backticks) is still read, and its tree and toplevel are still compared
|
||||
exactly (GST-776). Capy cannot start a subagent in a chosen directory, so before a stage takes a start token or sends
|
||||
its first reader, the Capy pass program compares the checkout with the one shared readers start in (the thread's
|
||||
working directory) and stops when that is another checkout or cannot be read: run /ship in the thread's own checkout.
|
||||
A start directory outside any checkout, such as a multi-repo workspace root, holds no tree to review by mistake and
|
||||
dispatches. Step 11's voices take the `adversarial-review` start
|
||||
token by default, and a `logStart` list without it stops before anything runs.
|
||||
|
||||
## Where this is enforced
|
||||
|
||||
+10
-8
@@ -27,6 +27,7 @@ const finish = (tail) => {
|
||||
};
|
||||
const stop = (why) => finish(['STOP: ' + why]);
|
||||
const logStart = input.stage === 'voices' ? (input.logStart ?? ['adversarial-review']) : (input.logStart || []);
|
||||
if (!Array.isArray(logStart) || !logStart.every((s) => typeof s === 'string' && s.trim())) return stop('input.logStart must be a list of gstack-review-log --start arguments, got ' + JSON.stringify(input.logStart) + '; nothing ran');
|
||||
if (input.stage === 'voices' && !(logStart.length && logStart.every((s) => s.split(' ')[0] === 'adversarial-review'))) return stop('Step 11 (stage "voices") takes the adversarial-review start token, which Step 11.5 binds, so its logStart is ["adversarial-review"] (with any --reservation in input.reservation) or omitted, never ' + JSON.stringify(input.logStart) + '; nothing ran');
|
||||
|
||||
const passCmd = [bin, 'pass', '--base', q(input.base), '--out', q(input.out), '--pass', q(input.pass)];
|
||||
@@ -42,14 +43,6 @@ const pr = await sh(passCmd.join(' '));
|
||||
if (pr.code !== 0) return stop('gstack-review-dispatch pass exited ' + pr.code + ': ' + pr.out.slice(-1500));
|
||||
const passOut = pr.out.trimEnd().split('\n');
|
||||
if (passOut[passOut.length - 1].startsWith('PASS: none ')) return finish(passOut);
|
||||
if (input.cwd) {
|
||||
const top = 'd=$(git rev-parse --show-toplevel 2>/dev/null) && cd "$d" && pwd -P';
|
||||
const ship = await sh(top);
|
||||
const start = await tools.capy.bash({ command: top, description: 'Ship review pass ' + input.pass + ' reader start checkout', timeout: 300 });
|
||||
const want = ship.out.trim(), got = (start.output || '').trim();
|
||||
if (ship.code !== 0) return stop('cannot read the checkout ' + input.cwd + ' names: ' + ship.out.slice(-300));
|
||||
if (start.exit_code === 0 && want !== got) return stop('readers on the shared machine start in ' + got + ', not in ' + want + ', the checkout this pass ships, and Capy cannot start a subagent in another directory; no start token was taken and no reader was dispatched. Run /ship in ' + got + ' (the thread\'s working directory) on this branch');
|
||||
}
|
||||
const resumed = passOut.some((l) => l.startsWith('MANIFEST: ') && l.endsWith(' resumed'));
|
||||
lines.push(...passOut.filter((l) => !resumed || l.startsWith('MANIFEST: ')));
|
||||
const mf = input.out.replace(/\/+$/, '') + '/pass-manifest.json';
|
||||
@@ -92,6 +85,15 @@ const active = input.stage === 'voices' ? ['voices'] : ['first', 'specialists',
|
||||
try {
|
||||
let m = await load();
|
||||
if (input.stage === 'voices' && !m.readers.some((r) => r.stage === 'voices')) return finish(['VOICES: none (the pass plan lists no Step 11 voice)', 'NEXT: judge']);
|
||||
if (input.cwd && !m.readers.some((r) => active.includes(r.stage) && !['planned', 'rendered'].includes(r.state))) {
|
||||
const top = 'if d=$(git rev-parse --show-toplevel 2>&1); then cd "$d" && pwd -P; else case "$d" in *"not a git repository"*) echo NOT_A_CHECKOUT ;; *) echo "$d"; exit 1 ;; esac; fi';
|
||||
const ship = await sh(top);
|
||||
const start = await tools.capy.bash({ command: top, description: 'Ship review pass ' + input.pass + ' reader start checkout', timeout: 300 });
|
||||
const want = ship.out.trim(), got = (start.output || '').trim();
|
||||
if (ship.code !== 0 || want === 'NOT_A_CHECKOUT') return stop('cannot read the checkout input.cwd names (' + input.cwd + '): ' + want.slice(-300));
|
||||
if (start.exit_code !== 0) return stop('cannot read the directory readers on the shared machine start in: ' + got.slice(-300) + '; no start token was taken and no reader was dispatched');
|
||||
if (got !== 'NOT_A_CHECKOUT' && got !== want) return stop('readers on the shared machine start in ' + got + ', not in ' + want + ', the checkout this pass ships, and Capy cannot start a subagent in another directory; no start token was taken and no reader was dispatched. Run /ship in ' + got + " (the thread's working directory) on this branch, with a fresh --out");
|
||||
}
|
||||
for (const s of logStart) {
|
||||
const key = (input.stage === 'voices' ? 'voices:' : 'pass:') + s;
|
||||
if ((m.log_starts || {})[key]) continue;
|
||||
|
||||
+4
-2
@@ -142,7 +142,7 @@ function renderRole(role: BriefRole, slots: Slots, t: SectionTexts): string {
|
||||
*/
|
||||
export function workspaceParagraph(dir: string): string {
|
||||
const q = `'${dir.replace(/'/g, `'\\''`)}'`;
|
||||
return `Workspace: the checkout under review is ${dir}. Your shell and file tools may start in another checkout, and every command runs in a fresh shell: begin each command with \`cd ${q} && \`, and give every file read, search, edit or write an absolute path under ${dir}. Start your final reply with the bare line \`REVIEWED_TREE: <tree> <toplevel>\`, even where this brief limits your output to findings: nothing before it, not the command and not a label. Get the line by running \`cd ${q} && echo "REVIEWED_TREE: $(git rev-parse 'HEAD^{tree}') $(git rev-parse --show-toplevel)"\` and copy what it prints. A reply without that line, or from another checkout, does not count.`;
|
||||
return `Workspace: the checkout under review is ${dir}. Your shell and file tools may start in another checkout, and every command runs in a fresh shell: begin each command with \`cd ${q} && \`, and give every file read, search, edit or write an absolute path under ${dir}. Make the first line of your final reply the line \`cd ${q} && echo "REVIEWED_TREE: $(git rev-parse 'HEAD^{tree}') $(git rev-parse --show-toplevel)"\` prints, even where this brief limits your output to findings; a reply without it, or from another checkout, does not count.`;
|
||||
}
|
||||
export const withWorkspace = (text: string, dir: string) => `${text}\n\n${workspaceParagraph(dir)}`;
|
||||
/** The checkout a brief names: the toplevel of the directory the bins run in, where its diff and merge base come from. */
|
||||
@@ -156,8 +156,10 @@ export function checkoutRoot(): string {
|
||||
* backtick wrapper, a bold label and backticks around the sha or the whole path are removed; the path's own characters are kept.
|
||||
*/
|
||||
function parseTreeLine(line: string): string | null {
|
||||
let t = line.trim().replace(/^(?:[-*>]\s+|\d+[.)]\s+|#+\s+)/, '').replace(/^[Pp]rinted:\s+/, '');
|
||||
const label = /^(?:\*\*)?[Pp]rinted:(?:\*\*)?\s+/;
|
||||
let t = line.trim().replace(/^(?:[-*>]\s+|\d+[.)]\s+|#+\s+)/, '').replace(label, '');
|
||||
for (const w of ['**', '`']) if (t.length > 2 * w.length && t.startsWith(w) && t.endsWith(w)) t = t.slice(w.length, -w.length).trim();
|
||||
t = t.replace(label, '');
|
||||
const m = /^(?:\*\*REVIEWED_TREE:\*\*|REVIEWED_TREE:)\s+`?([0-9a-f]{40,64})`?\s+(.+)$/.exec(t);
|
||||
if (!m) return null;
|
||||
const p = m[2]!.length > 2 && m[2]!.startsWith('`') && m[2]!.endsWith('`') ? m[2]!.slice(1, -1) : m[2]!;
|
||||
|
||||
@@ -585,22 +585,18 @@ describe('the REVIEWED_TREE line as GBRA-218 readers wrote it (GST-776)', () =>
|
||||
const top = fs.realpathSync(spawnSync('git', ['rev-parse', '--show-toplevel'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim());
|
||||
const tree = spawnSync('git', ['rev-parse', 'HEAD^{tree}'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
|
||||
const command = `cd '${top}' && echo "REVIEWED_TREE: $(git rev-parse 'HEAD^{tree}') $(git rev-parse --show-toplevel)"`;
|
||||
// Value: protects=a reader that reviewed the right checkout is not failed for how it wrote its fingerprint; fails_when=the brief asks for "the line <command> prints" (two Sonnet 5.5 low readers then wrote `printed: REVIEWED_TREE: …` and were failed as reported="none"), or a `printed:` prefix lets a wrong tree through;
|
||||
// why_new=GBRA-218 pass restarts on gstack 1.92.25.0; seam=lib/review-brief.ts workspaceParagraph and checkReviewedTree via gstack-review-dispatch collect; no_claim=that every model now writes the bare line
|
||||
test('the brief asks for the bare line first and gives the command only as the way to get it', () => {
|
||||
const p = workspaceParagraph(top);
|
||||
expect(p).not.toContain('the line `cd');
|
||||
const bare = p.indexOf('`REVIEWED_TREE: <tree> <toplevel>`');
|
||||
expect(bare).toBeGreaterThan(0);
|
||||
expect(p.indexOf(command)).toBeGreaterThan(bare);
|
||||
});
|
||||
|
||||
// Value: protects=a reader that reviewed the right checkout is not failed for how it wrote its fingerprint; fails_when=a reply that echoes the command and then writes `printed: REVIEWED_TREE: …` (two Sonnet 5.5 low readers) is failed as reported="none", or a `printed:` label lets a wrong tree, toplevel or other label through;
|
||||
// why_new=GBRA-218 pass restarts on gstack 1.92.25.0; seam=lib/review-brief.ts checkReviewedTree via gstack-review-dispatch collect; no_claim=the brief's wording, which stays main's until a no-loss replay
|
||||
test('the echoed command then a printed: line is read; the same shape with a wrong tree or checkout is still refused', () => {
|
||||
const reply = (fingerprint: string) => `${command}\nprinted: REVIEWED_TREE: ${fingerprint}\nNO FINDINGS\n`;
|
||||
expect(checkReviewedTree(reply(`${tree} ${top}`), `${tree} ${top}`)).toMatchObject({ ok: true });
|
||||
expect(checkReviewedTree(reply(`${'b'.repeat(40)} ${top}`), `${tree} ${top}`)).toMatchObject({ ok: false, moved: true, reported: `${'b'.repeat(40)} ${top}` });
|
||||
expect(checkReviewedTree(reply(`${tree} /elsewhere`), `${tree} ${top}`)).toMatchObject({ ok: false, moved: false, reported: `${tree} /elsewhere` });
|
||||
expect(checkReviewedTree(`Note: REVIEWED_TREE: ${tree} ${top}\nNO FINDINGS`, `${tree} ${top}`).reported).toBe('none');
|
||||
for (const line of ['`printed: REVIEWED_TREE: ' + tree + ' ' + top + '`', `**Printed:** REVIEWED_TREE: ${tree} ${top}`]) {
|
||||
expect(checkReviewedTree(line, `${tree} ${top}`).ok, line).toBe(true);
|
||||
expect(checkReviewedTree(line.replace(tree, 'c'.repeat(40)), `${tree} ${top}`).ok, line).toBe(false);
|
||||
}
|
||||
|
||||
const out = tmp();
|
||||
const ledger = path.join(out, 'review-ledger.jsonl');
|
||||
|
||||
@@ -594,6 +594,20 @@ describe('readers on a shared machine review the checkout the pass ships (GST-48
|
||||
expect(started.creates.length).toBeGreaterThan(0);
|
||||
const outside = host(path.dirname(wt), env);
|
||||
expect(await runPass(outside, { base: 'main', out: path.join(wt, '.state/run/p3'), pass: 'w0p3', cwd: wt, planFlags: ['--depth-floor', 'full'] })).toContain('NEXT: judge (Phase 4)');
|
||||
const broken = tmp('rp-broken-');
|
||||
fs.writeFileSync(path.join(broken, '.git'), 'garbage\n');
|
||||
const unreadable = host(broken, env);
|
||||
expect(await runPass(unreadable, { base: 'main', out: path.join(wt, '.state/run/p4'), pass: 'w0p4', cwd: wt, planFlags: ['--depth-floor', 'full'] })).toContain('STOP: cannot read the directory readers on the shared machine start in: fatal: invalid gitfile format');
|
||||
expect(unreadable.creates.length).toBe(0);
|
||||
const slow = host(wt, env, { pollsToFinish: 30 });
|
||||
let probes = 0;
|
||||
const bash = slow.tools.capy.bash;
|
||||
slow.tools.capy.bash = async (a) => { if (a.command.startsWith('if d=$(git rev-parse')) probes++; return bash(a); };
|
||||
const input = { base: 'main', out: path.join(wt, '.state/run/p5'), pass: 'w0p5', cwd: wt, planFlags: ['--depth-floor', 'full'] };
|
||||
expect(await runPass(slow, input)).toContain('NEXT: poll');
|
||||
expect(await runPass(slow, input)).toContain('NEXT: poll');
|
||||
expect(probes).toBe(1);
|
||||
expect(await runPass(host(wt, env), { ...input, logStart: [7] })).toStartWith('STOP: input.logStart must be a list');
|
||||
}, 240_000);
|
||||
|
||||
// Value: protects=a reader that never left the machine's default checkout cannot hand /ship a review of another tree; fails_when=collect merges a reply whose REVIEWED_TREE names the primary checkout while the pass ships a worktree, or a reply with no such line;
|
||||
|
||||
Reference in new issue
Block a user