mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 06:54:50 +01:00
fix: CEO-only finish leaves owner-only cap items undecided, refuses marker phase flags and a plan at the report path, and replaces ceo-review.md atomically
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
42dbaed9e0
commit
ba50ed9b1d
6 files changed
+89
-23
No files matched your search
+1
-1
@@ -188,7 +188,7 @@ function doNext(out: string, rest: string[], json: boolean): number {
|
||||
if (fresh) {
|
||||
const plan = flag(rest, '--plan');
|
||||
if (!plan) throw new RunError('RUN_NOT_INITIALIZED', `${out} has no run.json; pass --plan <file> on the first next`, 3);
|
||||
const ceoOnly = checkCeoOnlyFlags(rest, path.resolve(plan), undefined);
|
||||
const ceoOnly = checkCeoOnlyFlags(rest, path.resolve(plan), undefined, out);
|
||||
const ui = rest.includes('--ui') ? true : rest.includes('--no-ui') ? false : undefined;
|
||||
const maxRounds = num(rest, '--max-rounds');
|
||||
if (maxRounds !== undefined && (!Number.isInteger(maxRounds) || maxRounds < 1)) throw new RunError('ARTIFACT_SCHEMA', '--max-rounds needs an integer >= 1', 1);
|
||||
|
||||
@@ -3106,13 +3106,14 @@ the last `next` printed (`ATTEMPT: ... phase=<p> voice=<v>`).
|
||||
### `CEO_ONLY_CONFLICT`
|
||||
|
||||
**Meaning.** `gstack-autoplan next --ceo-only` was combined with `--ui`,
|
||||
`--developer-tool` or `--agent-primary`, each of which adds a phase a
|
||||
CEO-only run never opens, or the plan's `gstack-autoplan-scope` marker names
|
||||
`--ceo-only`. The mode is a command-line choice for one run, so a plan can't
|
||||
pin it.
|
||||
`--developer-tool` or `--agent-primary` (on the command line or in the plan's
|
||||
`gstack-autoplan-scope` marker), each of which adds a phase a CEO-only run
|
||||
never opens; or the marker names `--ceo-only` (the mode is a command-line
|
||||
choice for one run, so a plan can't pin it); or the plan sits at
|
||||
`<out>/ceo-review.md`, the report the run would write over it.
|
||||
|
||||
**Fix.** Drop the conflicting flag (or the marker token), or run the full
|
||||
review without `--ceo-only`.
|
||||
**Fix.** Drop the conflicting flag (or the marker token), move the plan out of
|
||||
the report path, or run the full review without `--ceo-only`.
|
||||
|
||||
<a id="ceo-only-after-init"></a>
|
||||
### `CEO_ONLY_AFTER_INIT`
|
||||
|
||||
+5
-3
@@ -597,12 +597,14 @@ voices as usual. When CEO closes, the run prints `REVIEWED:` and ends
|
||||
`GSTACK_RESULT: skill=autoplan status=reviewed run=<dir>`, with
|
||||
`<dir>/ceo-review.md` in the same sections, hashed in `run.json`. Each item a
|
||||
gate would have asked becomes a `decisions.jsonl` row of kind `auto` and status
|
||||
`auto_kept` (the plan's direction kept, nobody answered), and tasks are blocked
|
||||
on no decision. The driver never edits the plan, writes no `gate.json` and never
|
||||
`auto_kept` (nobody answered: the plan's direction is kept, and a round-cap item
|
||||
only the owner can accept, such as a Critical left open, stays undecided with
|
||||
no chosen option), and tasks are blocked on no decision. The driver never edits the plan, writes no `gate.json` and never
|
||||
approves, so `gstack-artifact validate` accepts the run as it stands.
|
||||
|
||||
- The flag goes on the first `next` only. It refuses beside `--ui`,
|
||||
`--developer-tool` or `--agent-primary`, and in a plan's scope marker
|
||||
`--developer-tool` or `--agent-primary` on the command line or in the plan's
|
||||
scope marker, when the marker names it, and on a plan at the report path
|
||||
(`CEO_ONLY_CONFLICT`); a directory that holds a full run refuses it
|
||||
(`CEO_ONLY_AFTER_INIT`).
|
||||
- `answer`, `recommend` and `revise` refuse a CEO-only run, and so does
|
||||
|
||||
+19
-11
@@ -7,18 +7,21 @@
|
||||
* only the CEO phase. `finishRun` is the one finish path next, phase close,
|
||||
* resume and close share: a full run writes the gate (`finishGate`); a
|
||||
* CEO-only run writes `ceo-review.md`, records each would-be gate item as a
|
||||
* decision row of kind `auto` and status `auto_kept` (the plan's direction
|
||||
* kept, nobody answered), and ends `reviewed`. It never writes gate.json,
|
||||
* decision row of kind `auto` and status `auto_kept` (nobody answered: the
|
||||
* plan's direction is kept, and an item only the owner can accept stays
|
||||
* undecided), and ends `reviewed`. It never writes gate.json,
|
||||
* never calls approve() and never edits the plan, so the commands that need a
|
||||
* gate or an approvable run refuse it (CEO_ONLY_NO_GATE); `budget` and
|
||||
* `close --deferred` still recover a run stopped by its spend cap.
|
||||
*/
|
||||
import * as crypto from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { buildGate, finishGate } from './autoplan-gate';
|
||||
import { carryLines } from './autoplan-carry';
|
||||
import { aggregateFindings, exportRun } from './autoplan-export';
|
||||
import { decisionRows, keepDirection, type GateItem } from './gate-list';
|
||||
import { MARKER_RE } from './autoplan-scope';
|
||||
import { RunError, journal, voiceRows, writeDecisions, type RunState } from './autoplan-state';
|
||||
import type { DecisionRow, FindingRow } from './headless-artifacts';
|
||||
import { sha256File } from './headless-artifacts';
|
||||
@@ -26,18 +29,20 @@ import { sha256File } from './headless-artifacts';
|
||||
export const CEO_ONLY_FLAG = '--ceo-only';
|
||||
export const CEO_ONLY_CONFLICTS = ['--ui', '--developer-tool', '--agent-primary'] as const;
|
||||
export const CEO_ONLY_REPORT = 'ceo-review.md';
|
||||
const MARKER_RE = /^<!--\s*gstack-autoplan-scope:(.*?)-->\s*$/;
|
||||
|
||||
/**
|
||||
* The first next's checks: true when this next asks for a CEO-only run. A marker naming the flag refuses whether or not
|
||||
* the flag was passed; the flag refuses beside a phase-adding flag or on a directory that holds a full run.
|
||||
* the flag was passed; the flag refuses beside a phase-adding flag (argument or marker), on a plan at the report path,
|
||||
* or on a directory that holds a full run.
|
||||
*/
|
||||
export function checkCeoOnlyFlags(args: readonly string[], plan: string | undefined, existing: RunState | undefined): boolean {
|
||||
export function checkCeoOnlyFlags(args: readonly string[], plan: string | undefined, existing: RunState | undefined, out?: string): boolean {
|
||||
const marker = plan && fs.existsSync(plan) ? MARKER_RE.exec(fs.readFileSync(plan, 'utf8').split('\n', 1)[0]!.replace(/^\uFEFF/, '').trimEnd()) : null;
|
||||
if (marker?.[1]!.trim().split(/\s+/).includes(CEO_ONLY_FLAG)) throw new RunError('CEO_ONLY_CONFLICT', `the scope marker in ${plan} names ${CEO_ONLY_FLAG}`, 3);
|
||||
const tokens = marker?.[1]!.trim().split(/\s+/) ?? [];
|
||||
if (tokens.includes(CEO_ONLY_FLAG)) throw new RunError('CEO_ONLY_CONFLICT', `the scope marker in ${plan} names ${CEO_ONLY_FLAG}`, 3);
|
||||
if (!args.includes(CEO_ONLY_FLAG)) return false;
|
||||
const conflicts = CEO_ONLY_CONFLICTS.filter(f => args.includes(f));
|
||||
const conflicts = CEO_ONLY_CONFLICTS.filter(f => args.includes(f) || tokens.includes(f));
|
||||
if (conflicts.length) throw new RunError('CEO_ONLY_CONFLICT', `${CEO_ONLY_FLAG} with ${conflicts.join(', ')}`, 3);
|
||||
if (plan && out && path.resolve(plan) === path.resolve(out, CEO_ONLY_REPORT)) throw new RunError('CEO_ONLY_CONFLICT', `the plan ${plan} is the report path`, 3);
|
||||
if (existing && !existing.ceo_only) throw new RunError('CEO_ONLY_AFTER_INIT', `${existing.out} holds full run ${existing.run} (status ${existing.status})`, 3);
|
||||
return true;
|
||||
}
|
||||
@@ -52,13 +57,16 @@ export function finishRun(state: RunState): string[] {
|
||||
if (!state.ceo_only) { finishGate(state); return carryLines(state); }
|
||||
const list = buildGate(state, 1);
|
||||
const items = list.items.filter(i => i.id !== 'p1');
|
||||
const rows: DecisionRow[] = decisionRows({ ...list, items }).map((row, i) => ({ ...row, kind: 'auto', status: 'auto_kept', chosen: keepDirection(items[i]!), default_if_unanswered: keepDirection(items[i]!), source: 'ceo-only' }));
|
||||
const kept = (item: GateItem) => (item.cap && item.kind === 'approval' ? null : keepDirection(item));
|
||||
const rows: DecisionRow[] = decisionRows({ ...list, items }).map((row, i) => ({ ...row, kind: 'auto', status: 'auto_kept', chosen: kept(items[i]!), default_if_unanswered: kept(items[i]!), source: 'ceo-only' }));
|
||||
writeDecisions(state.out, rows);
|
||||
state.status = 'reviewed';
|
||||
exportRun(state);
|
||||
const findings = aggregateFindings(state);
|
||||
const report = path.join(state.out, CEO_ONLY_REPORT);
|
||||
fs.writeFileSync(report, renderReport(state, items, rows, findings));
|
||||
const tmp = `${report}.${crypto.randomBytes(4).toString('hex')}.tmp`;
|
||||
fs.writeFileSync(tmp, renderReport(state, items, rows, findings), { flag: 'wx' });
|
||||
fs.renameSync(tmp, report);
|
||||
state.artifacts[CEO_ONLY_REPORT] = { path: CEO_ONLY_REPORT, sha256: sha256File(report) };
|
||||
journal(state.out, { event: 'ceo_only_reviewed', decisions: rows.length, findings: findings.length, report: CEO_ONLY_REPORT });
|
||||
return [`REVIEWED: ceo-only run=${state.run} decisions=${rows.length} (auto_kept) findings=${findings.length} report=${report} (not approved; no gate)`];
|
||||
@@ -82,10 +90,10 @@ function renderReport(state: RunState, items: GateItem[], rows: DecisionRow[], f
|
||||
...none(challenges.map(f => `- [${f.severity}] ${f.title} (${f.id}, ${f.voice}): proposal: ${f.resolution ?? 'unstated'}; the plan's direction is kept`)), '',
|
||||
'## Scope added', '', 'None. The driver never edits the plan; scope proposals are listed under Findings.', '',
|
||||
'## Conservative choices', '',
|
||||
...none(rows.map((r, i) => `- ${r.label}: ${r.title}: kept ${r.chosen} (${items[i]!.options.find(o => o.key === r.chosen)?.text ?? r.chosen}); the reviewers recommended ${r.recommended}`)), '',
|
||||
...none(rows.map((r, i) => `- ${r.label}: ${r.title}: ${r.chosen === null ? 'left open for the owner (not accepted)' : `kept ${r.chosen} (${items[i]!.options.find(o => o.key === r.chosen)?.text ?? r.chosen})`}; the reviewers recommended ${r.recommended}`)), '',
|
||||
'## Findings', '',
|
||||
...none(findings.map(f => `- [${f.severity}] ${f.title} (${f.id}; ${where(f)}): fix: ${f.resolution ?? 'unstated'}; disposition: ${f.disposition ?? 'open'}${(f as { reconciliation?: string }).reconciliation ? `; voices: ${(f as { reconciliation?: string }).reconciliation}` : ''}`)), '',
|
||||
'## Decision Audit Trail', '',
|
||||
...none(rows.map(r => `- ${r.id}: ${r.kind} ${r.status}, chosen ${r.chosen}, recommended ${r.recommended}${r.recommended_basis ? ` (${r.recommended_basis})` : ''}`)), '',
|
||||
...none(rows.map(r => `- ${r.id}: ${r.kind} ${r.status}, chosen ${r.chosen ?? 'none'}, recommended ${r.recommended}${r.recommended_basis ? ` (${r.recommended_basis})` : ''}`)), '',
|
||||
].join('\n');
|
||||
}
|
||||
@@ -65,7 +65,7 @@ export function detectDocsOnly(implementationPlan: string, root: string): DocsOn
|
||||
|
||||
export interface ScopeFlags { ui?: boolean; developerTool?: boolean; agentPrimary?: boolean; dx?: boolean; docsOnly?: boolean }
|
||||
export const SCOPE_FLAG_TOKENS = ['--ui', '--no-ui', '--no-dx', '--developer-tool', '--agent-primary', '--docs-only'] as const;
|
||||
const MARKER_RE = /^<!--\s*gstack-autoplan-scope:(.*?)-->\s*$/;
|
||||
export const MARKER_RE = /^<!--\s*gstack-autoplan-scope:(.*?)-->\s*$/;
|
||||
|
||||
export function scopeTokens(f: ScopeFlags): string[] {
|
||||
return [...(f.ui === true ? ['--ui'] : f.ui === false ? ['--no-ui'] : []), ...(f.dx === false ? ['--no-dx'] : []),
|
||||
|
||||
@@ -12,7 +12,7 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { importAutoplan } from '../lib/approval-record';
|
||||
import { ROOT, SMALL_PLAN, autoplan, env, journal, readRun, runPhase, startSmallRun } from './helpers/autoplan-run-fixture';
|
||||
import { ROOT, SMALL_PLAN, attemptsOf, autoplan, env, journal, readRun, resultOf, review, runPhase, startSmallRun } from './helpers/autoplan-run-fixture';
|
||||
|
||||
const TMP = fs.mkdtempSync(path.join(os.tmpdir(), 'ap-ceo-only-'));
|
||||
afterAll(() => fs.rmSync(TMP, { recursive: true, force: true }));
|
||||
@@ -175,6 +175,18 @@ describe('--ceo-only is a command-line flag for one run', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('a scope marker that adds a phase conflicts with the flag', () => {
|
||||
for (const token of ['--ui', '--developer-tool', '--agent-primary']) {
|
||||
const marked = plan(`marker${token}`, `<!-- gstack-autoplan-scope: ${token} -->
|
||||
${SMALL_PLAN}`);
|
||||
const out = path.join(dir(`marker${token}`), 'run');
|
||||
const r = autoplan(['next', '--out', out, '--plan', marked, '--ceo-only']);
|
||||
expect(r.status, token).toBe(3);
|
||||
expect(r.stderr).toContain('(CEO_ONLY_CONFLICT)');
|
||||
expect(fs.existsSync(path.join(out, 'run.json'))).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test('a directory that holds a full run refuses it after init', () => {
|
||||
const out = startSmallRun(dir('full'));
|
||||
const r = autoplan(['next', '--out', out, '--ceo-only']);
|
||||
@@ -184,3 +196,46 @@ describe('--ceo-only is a command-line flag for one run', () => {
|
||||
expect(readRun(out).phase_order).toContain('eng');
|
||||
});
|
||||
});
|
||||
|
||||
describe('the CEO-only finish records no answer nobody gave and writes only its own report', () => {
|
||||
test('a Critical left open by close --deferred stays undecided: no chosen option, listed as needing the owner', () => {
|
||||
const out = startSmallRun(dir('blocker'), ['--ceo-only']);
|
||||
const r = review(path.join(out, 'ceo-native-prompt.md'), { FIXTURE_FINDINGS: JSON.stringify([{ severity: 'Critical', title: 'Retries can double-charge', fix: 'idempotency key' }]) });
|
||||
expect(r.status, r.stderr).toBe(0);
|
||||
const ids = attemptsOf(out, 'ceo');
|
||||
const bound = autoplan(['submit', '--out', out, '--phase', 'ceo', '--voice', 'native', '--result', resultOf(out, 'ceo', 'native'), '--model', 'anthropic/claude-opus-5-5', '--attempt', ids.native, '--runner', 'host-subagent']);
|
||||
expect(bound.status, bound.stderr).toBe(0);
|
||||
const close = autoplan(['close', '--out', out, '--phase', 'ceo', '--deferred', 'owner stop', '--by', 'human']);
|
||||
expect(close.status, close.stderr).toBe(0);
|
||||
expect(close.stdout).toContain('REVIEWED: ceo-only');
|
||||
const blocker = rows(path.join(out, 'decisions.jsonl')).find(d => d.title.includes('Retries can double-charge'));
|
||||
expect(blocker).toMatchObject({ kind: 'auto', status: 'auto_kept', chosen: null, default_if_unanswered: null });
|
||||
const report = fs.readFileSync(path.join(out, 'ceo-review.md'), 'utf8');
|
||||
expect(report).not.toContain('you accept the risk');
|
||||
expect(report).toMatch(/Retries can double-charge.*left open for the owner/);
|
||||
});
|
||||
|
||||
test('a plan at the report path refuses before anything is written', () => {
|
||||
const out = path.join(dir('collide'), 'run');
|
||||
fs.mkdirSync(out, { recursive: true });
|
||||
const plan = path.join(out, 'ceo-review.md');
|
||||
fs.writeFileSync(plan, SMALL_PLAN);
|
||||
const r = autoplan(['next', '--out', out, '--plan', plan, '--no-ui', '--ceo-only']);
|
||||
expect(r.status).toBe(3);
|
||||
expect(r.stderr).toContain('(CEO_ONLY_CONFLICT)');
|
||||
expect(fs.existsSync(path.join(out, 'run.json'))).toBe(false);
|
||||
expect(fs.readFileSync(plan, 'utf8')).toBe(SMALL_PLAN);
|
||||
});
|
||||
|
||||
test('a symlink at the report path is replaced, never written through', () => {
|
||||
const out = startSmallRun(dir('symlink'), ['--ceo-only']);
|
||||
const victim = path.join(dir('symlink'), 'victim.txt');
|
||||
fs.writeFileSync(victim, 'keep me');
|
||||
fs.symlinkSync(victim, path.join(out, 'ceo-review.md'));
|
||||
runPhase(out, 'ceo');
|
||||
expect(readRun(out).status).toBe('reviewed');
|
||||
expect(fs.readFileSync(victim, 'utf8')).toBe('keep me');
|
||||
expect(fs.lstatSync(path.join(out, 'ceo-review.md')).isSymbolicLink()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user