mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix(owner-brief): the Captain brief resolves rec through parseReply, so carried decisions match gstack-gate render (#3546)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
61e201ec08
commit
6ee32c2239
3 files changed
+92
-8
No files matched your search
@@ -1,5 +1,15 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
**The Captain brief now says what `rec` does when a gate carries earlier decisions.** At a gate after the owner already answered some items, such as a round-cap gate, `gstack-owner-brief render --captain` listed each item's recommendation ("keep the plan as written", `rec resolves to d2a`) while `gstack-gate render` and `gstack-autoplan answer --reply rec` keep the carried answer (`d2b`). A Captain relaying the brief told the owner the opposite of what their one-word reply would record (#3546). The brief now takes its choices from the same resolver `gstack-gate render` uses.
|
||||
|
||||
### Fixed
|
||||
- **Carried choices in the Captain brief**: each choice under "What we need from you", the change-something example and the relaying agent's `rec resolves to` line use the `rec` resolution from `parseReply`, so a carried item shows the owner's earlier answer marked `(your answer at gate_rev N, carried forward)`, and the header says how many earlier decisions `rec` keeps.
|
||||
|
||||
### Tests
|
||||
- `test/captain-brief.test.ts` drives a fixture autoplan run to a round-cap gate with three carried decisions, renders `gstack-gate render` and `gstack-owner-brief render --captain` for it, and asserts the same `rec` resolution and choice labels; the negative control strips the carried decisions and expects both to fall back to the recommendations.
|
||||
|
||||
## [1.91.295.0] - 2026-10-28
|
||||
|
||||
**Deleting or renaming a test no longer fails pregate with PREGATE_NO_LANE.** A PR that removed a test file, or renamed one, failed `lanes` because the removed path had no test lane of its own, so cleanup PRs needed a hand-written `dependencies` entry to pass (#3529). Pregate now accounts for a removed test the way it accounts for an edit. A renamed test runs under its new name, a deleted test runs the remaining tests of the files it exercised, and either one runs the tests of any file that still names it, such as a curation list. Every waived test is printed on its own line, so the reason is visible in the gate output. Removing any other file still needs a lane, as before.
|
||||
|
||||
+10
-7
@@ -14,7 +14,7 @@ import { createHash } from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { gateCost, gateCostSentence } from './autoplan-gate-cost';
|
||||
import { recHash, type GateItem } from './gate-list';
|
||||
import { parseReply, recHash, type GateItem } from './gate-list';
|
||||
import { renderUrgentBlock } from './headless-artifacts';
|
||||
import { findSection, firstTitle } from './markdown-sections';
|
||||
import type { BriefInputs } from './owner-brief';
|
||||
@@ -75,7 +75,10 @@ export function renderCaptainBrief(inputs: BriefInputs, o: { outDir: string }):
|
||||
const missing = sections.filter(s => !s.found || !s.found.body.trim()).map(s => s.title);
|
||||
const urgent = inputs.findings.some(r => r.urgent === true || r.outside_plan === true) ? [renderUrgentBlock(inputs.findings).trimEnd(), ''] : [];
|
||||
const items = list.items.filter(i => i.id !== 'p1');
|
||||
const recText = (i: GateItem) => plainOption(i.options.find(x => x.key === i.recommended)?.text ?? i.recommended);
|
||||
const rec = parseReply('rec', list).choices;
|
||||
const recKey = (i: GateItem) => rec.find(c => c.id === i.id)?.chosen ?? i.recommended;
|
||||
const recOption = (i: GateItem) => i.options.find(x => x.key === recKey(i));
|
||||
const carried = items.filter(i => i.carried?.chosen === recKey(i));
|
||||
|
||||
let header: string[];
|
||||
if (!pending) {
|
||||
@@ -85,7 +88,7 @@ export function renderCaptainBrief(inputs: BriefInputs, o: { outDir: string }):
|
||||
} else {
|
||||
header = [
|
||||
`**Decision needed:** approve the plan "${title}". Reply **rec** to take every recommendation (recommended), or say in words what to change.`,
|
||||
`${items.length ? `${items.length} choice${items.length === 1 ? '' : 's'} below; rec takes the recommended answer for each.` : 'No open choices besides the approval itself.'} Nothing is built until you answer.`,
|
||||
`${items.length ? `${items.length} choice${items.length === 1 ? '' : 's'} below; rec ${carried.length ? `keeps the ${carried.length} decided at an earlier gate and takes the recommended answer for the rest` : 'takes the recommended answer for each'}.` : 'No open choices besides the approval itself.'} Nothing is built until you answer.`,
|
||||
];
|
||||
}
|
||||
|
||||
@@ -94,15 +97,15 @@ export function renderCaptainBrief(inputs: BriefInputs, o: { outDir: string }):
|
||||
if (!pending) need.push('Nothing. This gate is answered.', '');
|
||||
else {
|
||||
need.push(`One decision: approve the plan.`, '');
|
||||
need.push(`- **rec (recommended):** ${p1 ? plainOption(p1.options.find(x => x.key === p1.recommended)?.text ?? 'approve') : 'approve'}${items.length ? ', taking these recommendations:' : '.'}`);
|
||||
items.forEach((i, n) => need.push(` ${n + 1}. ${stripRef(i.title)}: ${recText(i)}${i.options.find(x => x.key === i.recommended)?.changes_plan ? ' (changes the plan)' : ''}.`));
|
||||
need.push(`- **rec (recommended):** ${p1 ? plainOption(recOption(p1)?.text ?? 'approve') : 'approve'}${items.length ? ', taking these recommendations:' : '.'}`);
|
||||
items.forEach((i, n) => need.push(` ${n + 1}. ${stripRef(i.title)}: ${plainOption(recOption(i)?.text ?? recKey(i))}${carried.includes(i) ? ` (your answer at gate_rev ${i.carried!.gate_rev}, carried forward)` : recOption(i)?.changes_plan ? ' (changes the plan)' : ''}.`));
|
||||
const example = items.find(i => i.options.length > 1);
|
||||
const alt = example?.options.find(x => x.key !== example.recommended);
|
||||
const alt = example?.options.find(x => x.key !== recKey(example));
|
||||
need.push(`- **Change something:** say which choice and what you want instead, in words${example && alt ? ` (for example "${clip(stripRef(example.title), 60)}: ${plainOption(alt.text)}")` : ''}. A change that alters the plan reopens its review before anything is built.`);
|
||||
need.push('- **Reject:** nothing is built.', '');
|
||||
if (!missing.length) need.push(`- **do it:** rec at full scope: every recommendation above, the follow-ons the plan names (docs, trains, cleanups) included, carried through to done with no further ask.`, '', 'Reply: **rec**, or **do it**', '');
|
||||
}
|
||||
const tokens = list.items.map(i => `${i.id}${i.recommended}`).join(' ');
|
||||
const tokens = rec.map(c => `${c.id}${c.chosen}`).join(' ');
|
||||
const trailer = `_For the relaying agent, not the person:_ run ${list.run}, gate_rev=${list.gate_rev}, rec_hash=${hash}; rec resolves to ${tokens}; plan ${plan.file ?? '(missing)'} (sha256 ${plan.sha}). Answer with ` +
|
||||
`\`gstack-autoplan answer --out ${o.outDir} --gate-rev ${list.gate_rev} --reply rec --rec-hash ${hash} --by agent:<id> --owner-reply "<their exact words>"\`, then \`gstack-approvals import --autoplan ${o.outDir} --by <name>\`.`;
|
||||
// The price sits above the trailer: the relaying agent sends everything above that line, so the person approving sees it.
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* every choice rec takes shown in words, gate ids only on the agent line, the
|
||||
* rec_hash binding, and the incomplete and answered states.
|
||||
*/
|
||||
import { afterEach, describe, expect, test } from 'bun:test';
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, test } from 'bun:test';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
@@ -13,6 +13,8 @@ import { capyRelayLine, renderCaptainBrief } from '../lib/captain-brief';
|
||||
import { parseReply, recHash, type GateList } from '../lib/gate-list';
|
||||
import { findSection, headings } from '../lib/markdown-sections';
|
||||
import { loadBriefInputs } from '../lib/owner-brief';
|
||||
import { capyRunner, phaseVia, readRunJson, writePlan } from './helpers/autoplan-capy';
|
||||
import { SMALL_PLAN } from './helpers/autoplan-run-fixture';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const dirs: string[] = [];
|
||||
@@ -151,3 +153,72 @@ describe('gstack-owner-brief render --captain', () => {
|
||||
expect(capyRelayLine('/r', {})).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('carried decisions at a cap gate (#3546)', () => {
|
||||
const r = capyRunner();
|
||||
afterAll(() => r.cleanup());
|
||||
const SECTIONS = '\n## Before → after\n\nBefore: retries hammer. After: they back off.\n\n## Why it matters\n\nOutages.\n\n## What caused this work\n\nIncident 7.\n\n## Big decisions\n\n1. Exponential backoff.\n';
|
||||
const PLAN = SMALL_PLAN + SECTIONS;
|
||||
const WORDS: Record<string, string> = { 'keep your direction': 'keep the plan as written', 'accept the change': "make the reviewer's change" };
|
||||
|
||||
function cappedRun(): string {
|
||||
const dir = path.join(r.host.base, 'carried');
|
||||
const plan = writePlan(dir, PLAN);
|
||||
const out = path.join(dir, 'run');
|
||||
expect(r.ap(['next', '--out', out, '--plan', plan, '--no-ui', '--max-rounds', '1']).status).toBe(0);
|
||||
const challenge = (title: string) => ({ severity: 'Medium', title, fix: `change: ${title}`, user_challenge: true, changes: 'scope' });
|
||||
phaseVia(r, out, 'ceo', { native: [challenge('Use a queue table'), challenge('Add metrics')], outside: [challenge('Drop jitter')] });
|
||||
phaseVia(r, out, 'eng', { native: [], outside: [] });
|
||||
const ds = JSON.parse(fs.readFileSync(path.join(out, 'gate.json'), 'utf8')).items.filter((i: any) => i.id.startsWith('d'));
|
||||
expect(ds.map((i: any) => i.recommended)).toEqual(['a', 'a', 'a']);
|
||||
expect(r.ap(['answer', '--by', 'human', '--out', out, '--gate-rev', '1', '--reply', `${ds.map((i: any) => `${i.id}b`).join(' ')} p1b`]).status).toBe(0);
|
||||
const v2 = path.join(dir, 'plan-v2.md');
|
||||
fs.writeFileSync(v2, PLAN.replace('cap 30 s', 'cap 60 s, dead-letter after 8 attempts'));
|
||||
const rev = r.ap(['revise', '--out', out, '--plan', v2, '--incorporates', readRunJson(out).amendments.map((a: any) => a.decision).join(',')]);
|
||||
expect(rev.stdout).toMatch(/^REVISED: plan=[0-9a-f]{12} reopened=none capped=ceo,eng$/m);
|
||||
return out;
|
||||
}
|
||||
const cli = (bin: string, args: string[]) => spawnSync('bun', [path.join(ROOT, 'bin', bin), ...args], { encoding: 'utf8', timeout: 60_000 }).stdout ?? '';
|
||||
function renders(out: string) {
|
||||
const gate = JSON.parse(fs.readFileSync(path.join(out, 'gate.json'), 'utf8')) as GateList;
|
||||
const gateText = cli('gstack-gate', ['render', path.join(out, 'gate.json'), '--all']);
|
||||
const briefText = cli('gstack-owner-brief', ['render', '--captain', '--out', out]);
|
||||
const gateRec = JSON.parse(/\`rec\` resolves to: (\{.*\})$/m.exec(gateText)![1]!).choices as Record<string, string>;
|
||||
const briefRec = Object.fromEntries(/rec resolves to ([a-z0-9 ]+);/.exec(briefText)![1]!.split(' ').map(t => [t.slice(0, -1), t.slice(-1)]));
|
||||
return { gate, gateText, briefText, gateRec, briefRec };
|
||||
}
|
||||
let out = '';
|
||||
beforeAll(() => { out = cappedRun(); });
|
||||
|
||||
test('the Captain brief resolves rec to the carried choices, as gstack-gate render does, with the same choice labels', () => {
|
||||
const { gate, gateText, briefText, gateRec, briefRec } = renders(out);
|
||||
expect(gate.gate_rev).toBe(2);
|
||||
expect(gate.items.some(i => i.cap?.kind === 'delta')).toBe(true);
|
||||
const carried = gate.items.filter(i => i.carried);
|
||||
expect(carried.map(i => [i.recommended, i.carried!.chosen])).toEqual([['a', 'b'], ['a', 'b'], ['a', 'b']]);
|
||||
expect(briefRec).toEqual(gateRec);
|
||||
expect(Object.fromEntries(carried.map(i => [i.id, gateRec[i.id]]))).toEqual({ d1: 'b', d2: 'b', d3: 'b' });
|
||||
gate.items.filter(i => i.id !== 'p1').forEach((item, n) => {
|
||||
const text = item.options.find(o => o.key === gateRec[item.id])!.text;
|
||||
expect(gateText).toContain(` ${item.id}${gateRec[item.id]}) ${text}`);
|
||||
expect(briefText).toContain(` ${n + 1}. ${item.title.replace(/\s*\[[^\]]*\]\s*$/, '')}: ${WORDS[text] ?? text}`);
|
||||
});
|
||||
expect(briefText).toContain("Use a queue table: make the reviewer's change (your answer at gate_rev 1, carried forward).");
|
||||
expect(briefText).toContain('rec keeps the 3 decided at an earlier gate and takes the recommended answer for the rest.');
|
||||
});
|
||||
|
||||
test('negative control: without carried decisions both resolve rec to the recommendations', () => {
|
||||
const copy = path.join(r.host.base, 'uncarried');
|
||||
fs.cpSync(out, copy, { recursive: true });
|
||||
const gate = JSON.parse(fs.readFileSync(path.join(copy, 'gate.json'), 'utf8')) as GateList;
|
||||
expect(Object.fromEntries(gate.items.map(i => [i.id, i.recommended]))).not.toEqual(renders(out).gateRec);
|
||||
for (const item of gate.items) delete item.carried;
|
||||
fs.writeFileSync(path.join(copy, 'gate.json'), JSON.stringify(gate));
|
||||
const { briefText, gateRec, briefRec } = renders(copy);
|
||||
expect(briefRec).toEqual(gateRec);
|
||||
expect(gateRec).toEqual(Object.fromEntries(gate.items.map(i => [i.id, i.recommended])));
|
||||
expect(briefText).toContain('Use a queue table: keep the plan as written (a reviewer suggested changing it).');
|
||||
expect(briefText).not.toContain('carried forward');
|
||||
expect(briefText).toContain('rec takes the recommended answer for each.');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user