mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix: adversarial-review fixes (result refuses an unpushed HEAD, resolves --out and reports unwritable records; ready names a failed merge-ref fetch; unattended /ship never runs gh pr ready)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
1490d9d438
commit
4afaa98f54
7 files changed
+26
-14
No files matched your search
+13
-6
@@ -225,24 +225,31 @@ function cmdResult(rest: string[], json: boolean): number {
|
||||
source = path.resolve(bodyFile);
|
||||
} else if (pr) {
|
||||
const repo = flag(rest, '--repo');
|
||||
const r = gh(['pr', 'view', pr, ...(repo ? ['--repo', repo] : []), '--json', 'body']);
|
||||
const r = gh(['pr', 'view', pr, ...(repo ? ['--repo', repo] : []), '--json', 'body,headRefOid']);
|
||||
if (!r.ok) return incomplete(`gh pr view failed: ${r.err.trim()}`);
|
||||
try { body = JSON.parse(r.out).body ?? ''; } catch { return incomplete('gh returned no JSON body'); }
|
||||
let pushed: unknown;
|
||||
try { const v = JSON.parse(r.out); body = v.body ?? ''; pushed = v.headRefOid; } catch { return incomplete('gh returned no JSON body'); }
|
||||
if (typeof pushed === 'string' && /^[0-9a-f]{12,64}$/i.test(pushed) && !pushed.toLowerCase().startsWith(head.toLowerCase())) {
|
||||
return refused('QUEUE_RECEIPT_STALE', `HEAD ${head.slice(0, 12)} is not PR #${pr}'s pushed head ${pushed.slice(0, 12)}; push it, then rerun gstack-ship-receipt result`);
|
||||
}
|
||||
source = `PR #${pr}${repo ? ` (${repo})` : ''}`;
|
||||
} else { process.stderr.write(`gstack-ship-receipt result: --pr <n> or --body <file> is required\n${HELP}\n`); return EXIT.usage; }
|
||||
const bound = bindReceipt(body, pr, [head]);
|
||||
if (!bound.ok) return refused(bound.code, bound.message);
|
||||
const stamp = new Date().toISOString().replace(/[:.]/g, '-');
|
||||
const dir = flag(rest, '--out') ?? path.join(resolveStateRoot(), 'ship', 'runs', `${stamp}-${head.slice(0, 12)}`);
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
const out = flag(rest, '--out');
|
||||
const dir = out ? path.resolve(out) : path.join(resolveStateRoot(), 'ship', 'runs', `${stamp}-${head.slice(0, 12)}`);
|
||||
const receiptPath = path.join(dir, 'ship-receipt.md');
|
||||
fs.writeFileSync(receiptPath, renderReceiptBlock(bound.receipt));
|
||||
try {
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.writeFileSync(receiptPath, renderReceiptBlock(bound.receipt));
|
||||
} catch (e) { return incomplete(`cannot write the run record under ${dir}: ${e instanceof Error ? e.message : String(e)}`); }
|
||||
const run = {
|
||||
schema_version: 1, skill: 'ship', status: 'complete', head, pr: bound.receipt.pr ?? (pr ? Number(pr) : null), base: bound.receipt.base, version: bound.receipt.version ?? null,
|
||||
queue_mode: bound.receipt.queue_mode ?? null, session_kind: bound.receipt.session_kind ?? process.env.GSTACK_SESSION_KIND ?? null,
|
||||
receipt: receiptPath, receipt_source: source, branch: repoRoot ? git(['rev-parse', '--abbrev-ref', 'HEAD'], repoRoot) ?? null : null, created_at: new Date().toISOString(),
|
||||
};
|
||||
fs.writeFileSync(path.join(dir, 'ship-run.json'), JSON.stringify(run, null, 2) + '\n');
|
||||
try { fs.writeFileSync(path.join(dir, 'ship-run.json'), JSON.stringify(run, null, 2) + '\n'); } catch (e) { return incomplete(`cannot write ${path.join(dir, 'ship-run.json')}: ${e instanceof Error ? e.message : String(e)}`); }
|
||||
process.stderr.write(bound.line + '\n');
|
||||
process.stdout.write(json ? JSON.stringify({ ...run, run: dir }) + '\n' : `GSTACK_RESULT: skill=ship status=complete run=${dir}\n`);
|
||||
return EXIT.ok;
|
||||
|
||||
@@ -648,7 +648,7 @@ if [ "$_SESSION_KIND" = "unattended" ]; then
|
||||
case "$SKILL_NAME" in
|
||||
autoplan) _UA_RECORD="This skill writes a run directory through gstack-autoplan (plan.md, review-record.md, tasks.jsonl, decisions.jsonl, findings.jsonl, timing.json, run.json, gate.json): append each recommended choice to decisions.jsonl with kind \"auto\" and each pending one with kind \"approval\" and status \"pending\". The final approval gate is written as a gate list (gstack-gate render) and this run ENDS there with \`GSTACK_RESULT: skill=autoplan status=gate_pending run=<dir>\`." ;;
|
||||
eval-plan) _UA_RECORD="This run ENDS at the Step 4 spend approval, left pending, with \`GSTACK_RESULT: skill=eval-plan status=gate_pending run=<outdir>\`, where <outdir> is the pilot's --out directory. No helper writes a gate.json for this skill yet, so the parent reads the pilot report and spend ledger in <outdir>." ;;
|
||||
ship) _UA_RECORD="This skill ends with \`gstack-ship-receipt result\` (Step 22), which writes <state root>/ship/runs/<utc>-<head12>/ship-run.json (a /ship record, not a run manifest) and prints \`GSTACK_RESULT: skill=ship status=complete run=<dir>\`; a run that stops earlier prints \`GSTACK_RESULT: skill=ship status=refused run=-\` itself. The one publication allowed here: when an open PR for this branch exists and HEAD is its pushed head, update that PR's body with the receipt through the pr-body section's existing-PR path (never pr-create, never a push); otherwise the handoff section writes the receipt to .gstack/tmp/ship-receipt.md and the parent reads it there." ;;
|
||||
ship) _UA_RECORD="This skill ends with \`gstack-ship-receipt result\` (Step 22), which writes <state root>/ship/runs/<utc>-<head12>/ship-run.json (a /ship record, not a run manifest) and prints \`GSTACK_RESULT: skill=ship status=complete run=<dir>\`; a run that stops earlier prints \`GSTACK_RESULT: skill=ship status=refused run=-\` itself. The one publication allowed here: when an open PR for this branch exists and HEAD is its pushed head, update that PR's body and title with the receipt through the pr-body section's existing-PR path (never pr-create, never a push, and never \`gh pr ready\`: the draft-state items there concern an early PR this run never opened); otherwise the handoff section writes the receipt to .gstack/tmp/ship-receipt.md and the parent reads it there." ;;
|
||||
review) _UA_RECORD="$_UA_NONE The parent reads the review-log rows (\`gstack-review-read\`)." ;;
|
||||
qa) _UA_RECORD="$_UA_NONE The parent reads the \`qa-report-*.md\` file." ;;
|
||||
*) _UA_RECORD="$_UA_NONE The parent reads this skill's own record (docs/unattended.md)." ;;
|
||||
|
||||
+5
-2
@@ -100,8 +100,11 @@ Not every workflow writes this line or a run directory today:
|
||||
open PR for the branch exists and HEAD is its pushed head, Step 18 takes the
|
||||
pr-body section's existing-PR path and posts the body (receipt first) and
|
||||
title through `gstack-post`. That is the one publication the unattended
|
||||
rules allow; it never runs `pr-create`, never pushes a commit, and a branch
|
||||
with no open PR, or whose PR head is not HEAD, takes the handoff section.
|
||||
rules allow; it never runs `pr-create`, never pushes a commit, never runs
|
||||
`gh pr ready` (the existing-PR path's draft-state items concern an early PR
|
||||
an unattended run never opens), and a branch with no open PR, or whose PR
|
||||
head is not HEAD, takes the handoff section. `result --pr <n>` also refuses
|
||||
`QUEUE_RECEIPT_STALE` when HEAD is not the PR's pushed head.
|
||||
When the run committed anything (a CHANGELOG entry, generated tests), the
|
||||
parent pushes those commits itself, then reruns the receipt write, the body
|
||||
post and `result` at the pushed head: a receipt for an unpushed head is
|
||||
|
||||
@@ -154,8 +154,9 @@ export function directiveFor(kind: SessionKind): string {
|
||||
'a destructive or irreversible option, or an approval gate is appended with kind "approval" and status ' +
|
||||
'"pending" — never chosen, never approved — and the skill ends at its next gate. Only /autoplan and ' +
|
||||
'/eval-plan print `GSTACK_RESULT: skill=<name> status=gate_pending run=<dir>` there, and /ship prints its ' +
|
||||
'own line at Step 22 (`gstack-ship-receipt result`); every other skill prints no GSTACK_RESULT line, so do ' +
|
||||
'not invent one (Unattended session block; docs/unattended.md names each skill\'s record).'
|
||||
'own line at Step 22 (`gstack-ship-receipt result`); every other skill prints no GSTACK_RESULT line of its own ' +
|
||||
'(only a refused install check prints `status=refused run=-` for any skill), so do not invent one ' +
|
||||
'(Unattended session block; docs/unattended.md names each skill\'s record).'
|
||||
);
|
||||
case 'headless':
|
||||
// #2733 review (multi-specialist): a spawned-marked subagent under a
|
||||
|
||||
@@ -118,7 +118,8 @@ export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise<Tr
|
||||
const again = `resolve the conflict with ${ctx.base} (or wait for GitHub to recompute refs/pull/${o.pr}/merge), push, then ${Q} ready --pr ${o.pr} --screen; a screen dispatched on a stale merge ref plans no cases`;
|
||||
if (!/^[0-9a-f]{40,64}$/.test(mergeSha)) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `${env.remote} has no refs/pull/${o.pr}/merge (the PR conflicts with ${ctx.base}, or GitHub has not computed it yet)` }, lines, again, 'none');
|
||||
const fetched = git(ctx.repoRoot, ['fetch', '-q', env.remote, `refs/pull/${o.pr}/merge`]);
|
||||
const mergeParent = fetched.ok ? git(ctx.repoRoot, ['rev-parse', '--verify', '--quiet', `${mergeSha}^2`]).out : '';
|
||||
if (!fetched.ok) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `could not fetch refs/pull/${o.pr}/merge from ${env.remote}: ${(fetched.err || fetched.out).trim().slice(0, 200) || 'git fetch failed'}` }, lines, `check the remote, then ${Q} ready --pr ${o.pr} --screen`, 'none');
|
||||
const mergeParent = git(ctx.repoRoot, ['rev-parse', '--verify', '--quiet', `${mergeSha}^2`]).out;
|
||||
if (mergeParent !== head) return outRefused({ code: 'QUEUE_SCREEN_STALE_REF', message: `${env.remote}'s refs/pull/${o.pr}/merge (${mergeSha.slice(0, 12)}) merges ${mergeParent ? mergeParent.slice(0, 12) : 'no PR head'}, not the PR head ${head.slice(0, 12)}: GitHub has not recomputed it, which is what a conflicting PR looks like (CHANGELOG-only conflicts included)` }, lines, again, 'none');
|
||||
const d = await dispatchScreen(env, o, mergeSha, baseSha);
|
||||
if ('exit' in d) return { ...d, lines: [...lines, ...d.lines] };
|
||||
|
||||
+1
-1
@@ -103,10 +103,10 @@ export function missingIssueLines(body: string, issues: NonNullable<ShipReceipt[
|
||||
return expected.filter(e => !have.some(l => l === e || l.startsWith(`${e} `)));
|
||||
}
|
||||
|
||||
/** The first lines of the completion message to the coordinator: identifiers only. */
|
||||
/** Receipt fields come from the PR body, so anything printed from one stays on its own line, control characters included. */
|
||||
const oneLine = (s: string) => s.replace(/[\x00-\x1f\x7f]+/g, ' ');
|
||||
|
||||
/** The first lines of the completion message to the coordinator: identifiers only. */
|
||||
export function receiptSummaryLine(r: ShipReceipt): string {
|
||||
const parts = [`pr=${r.pr ?? '-'}`, `head=${r.head.slice(0, 12)}`, `version=${r.version ?? '-'}`, `base=${r.base}`];
|
||||
if (r.gated_tree) parts.push(`gated_tree=${r.gated_tree.slice(0, 12)}`);
|
||||
|
||||
@@ -427,7 +427,7 @@ describe('gstack-skill-start behavior', () => {
|
||||
expect(ship.match(line)).toHaveLength(1);
|
||||
expect((ship.match(/^GSTACK_INSTRUCTION_BEGIN: (\S+)/gm) ?? []).map((h) => h.replace(/^GSTACK_INSTRUCTION_BEGIN: /, ''))).toEqual(['unattended-session']);
|
||||
expect(ship).toContain('`GSTACK_RESULT: skill=ship status=complete run=<dir>`');
|
||||
expect(ship).toContain("update that PR's body with the receipt through the pr-body section's existing-PR path (never pr-create, never a push)");
|
||||
expect(ship).toContain("update that PR's body and title with the receipt through the pr-body section's existing-PR path (never pr-create, never a push, and never `gh pr ready`");
|
||||
expect(ship).toContain('the handoff section writes the receipt to .gstack/tmp/ship-receipt.md');
|
||||
for (const skill of ['review', 'qa', 'autoplan']) {
|
||||
expect(runStart(['--skill', skill], { GSTACK_SESSION_KIND: 'unattended' })).not.toContain('UNATTENDED_SHIP');
|
||||
|
||||
Reference in new issue
Block a user