Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/genie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,13 @@ registerApprovalCommands(program);
program
.command('done [ref]')
.description('Close the current turn (inside an agent session) or mark a wish group done (team-lead, <slug>#<group>)')
.action(async (ref: string | undefined) => {
.option(
'-r, --report <message>',
'Full session handoff — what shipped, what is verified, what is left, surprises, decisions. REQUIRED. As long as it needs to be (multi-line OK; pass via heredoc).',
)
.action(async (ref: string | undefined, options: { report?: string }) => {
const { doneAction } = await import('./term-commands/done.js');
await doneAction(ref);
await doneAction(ref, options);
});

program
Expand Down
126 changes: 88 additions & 38 deletions src/term-commands/done.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* `genie done` command dispatch — agent-session path vs wish-group path
* vs the Group 6 permanent-agent rejection guard.
* vs the Group 6 permanent-agent rejection guard, plus the mandatory
* --report nudge that gives every close a one-line audit trail.
*
* Uses injected deps (no DB) to isolate routing behavior.
*/
Expand All @@ -9,6 +10,8 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
import type { TurnCloseResult } from '../lib/turn-close.js';
import { PermanentAgentDoneRejected, doneAction } from './done.js';

const REPORT = { report: 'shipped foo + bar; smoke green' };

describe('doneAction dispatch', () => {
const originalAgent = process.env.GENIE_AGENT_NAME;

Expand All @@ -20,11 +23,11 @@ describe('doneAction dispatch', () => {
process.env.GENIE_AGENT_NAME = originalAgent;
});

test('agent session + no ref → calls turnClose(outcome=done)', async () => {
test('agent session + no ref → calls turnClose(outcome=done) with report as reason', async () => {
process.env.GENIE_AGENT_NAME = 'engineer-g2';
const calls: Array<{ outcome: string }> = [];
const turnCloseFn = async (opts: { outcome: string }): Promise<TurnCloseResult> => {
calls.push({ outcome: opts.outcome });
const calls: Array<{ outcome: string; reason?: string }> = [];
const turnCloseFn = async (opts: { outcome: string; reason?: string }): Promise<TurnCloseResult> => {
calls.push({ outcome: opts.outcome, reason: opts.reason });
return { noop: false, executorId: 'exec-1', outcome: 'done', closedAt: new Date().toISOString() };
};
let wishCalls = 0;
Expand All @@ -33,28 +36,28 @@ describe('doneAction dispatch', () => {
};
const lookupCallingAgent = async () => ({ id: 'agent-task', kind: 'task' as const });

await doneAction(undefined, { turnCloseFn: turnCloseFn as never, wishDone, lookupCallingAgent });
await doneAction(undefined, REPORT, { turnCloseFn: turnCloseFn as never, wishDone, lookupCallingAgent });

expect(calls).toEqual([{ outcome: 'done' }]);
expect(calls).toEqual([{ outcome: 'done', reason: REPORT.report }]);
expect(wishCalls).toBe(0);
});

test('positional ref → delegates to wish-group doneCommand', async () => {
test('positional ref → delegates to wish-group doneCommand with report', async () => {
process.env.GENIE_AGENT_NAME = 'engineer-g2';
let turnCalls = 0;
const turnCloseFn = async (): Promise<TurnCloseResult> => {
turnCalls++;
return { noop: false, executorId: 'x', outcome: 'done', closedAt: null };
};
const wishRefs: string[] = [];
const wishDone = async (ref: string) => {
wishRefs.push(ref);
const wishCalls: Array<{ ref: string; report: string }> = [];
const wishDone = async (ref: string, report: string) => {
wishCalls.push({ ref, report });
};

await doneAction('my-wish#2', { turnCloseFn: turnCloseFn as never, wishDone });
await doneAction('my-wish#2', REPORT, { turnCloseFn: turnCloseFn as never, wishDone });

expect(turnCalls).toBe(0);
expect(wishRefs).toEqual(['my-wish#2']);
expect(wishCalls).toEqual([{ ref: 'my-wish#2', report: REPORT.report }]);
});

test('positional ref without GENIE_AGENT_NAME → still delegates to wish path', async () => {
Expand All @@ -63,15 +66,15 @@ describe('doneAction dispatch', () => {
turnCalls++;
return { noop: false, executorId: 'x', outcome: 'done', closedAt: null };
};
const wishRefs: string[] = [];
const wishDone = async (ref: string) => {
wishRefs.push(ref);
const wishCalls: Array<{ ref: string; report: string }> = [];
const wishDone = async (ref: string, report: string) => {
wishCalls.push({ ref, report });
};

await doneAction('other-wish#1', { turnCloseFn: turnCloseFn as never, wishDone });
await doneAction('other-wish#1', REPORT, { turnCloseFn: turnCloseFn as never, wishDone });

expect(turnCalls).toBe(0);
expect(wishRefs).toEqual(['other-wish#1']);
expect(wishCalls).toEqual([{ ref: 'other-wish#1', report: REPORT.report }]);
});

test('no ref + no GENIE_AGENT_NAME → exits with error', async () => {
Expand All @@ -83,7 +86,7 @@ describe('doneAction dispatch', () => {
}) as never;

try {
await expect(doneAction(undefined)).rejects.toThrow(/process.exit stub/);
await expect(doneAction(undefined, REPORT)).rejects.toThrow(/process.exit stub/);
expect(exitCode).toBe(2);
} finally {
process.exit = originalExit;
Expand All @@ -101,7 +104,61 @@ describe('doneAction dispatch', () => {
const lookupCallingAgent = async () => ({ id: 'agent-task', kind: 'task' as const });

// Should complete without throwing
await doneAction(undefined, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
await doneAction(undefined, REPORT, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
});

// ==========================================================================
// Mandatory --report nudge
// ==========================================================================

describe('mandatory --report', () => {
test('missing report → exits 2 with friendly hint, never calls turnClose or wishDone', async () => {
process.env.GENIE_AGENT_NAME = 'engineer-g2';
const originalExit = process.exit;
let exitCode: number | undefined;
process.exit = ((code?: number) => {
exitCode = code;
throw new Error('process.exit stub');
}) as never;

let turnCalls = 0;
const turnCloseFn = async (): Promise<TurnCloseResult> => {
turnCalls++;
return { noop: false, executorId: 'x', outcome: 'done', closedAt: null };
};
let wishCalls = 0;
const wishDone = async () => {
wishCalls++;
};

try {
await expect(doneAction(undefined, {}, { turnCloseFn: turnCloseFn as never, wishDone })).rejects.toThrow(
/process.exit stub/,
);
expect(exitCode).toBe(2);
expect(turnCalls).toBe(0);
expect(wishCalls).toBe(0);
} finally {
process.exit = originalExit;
}
});

test('whitespace-only report → treated as missing', async () => {
process.env.GENIE_AGENT_NAME = 'engineer-g2';
const originalExit = process.exit;
let exitCode: number | undefined;
process.exit = ((code?: number) => {
exitCode = code;
throw new Error('process.exit stub');
}) as never;

try {
await expect(doneAction(undefined, { report: ' \n ' })).rejects.toThrow(/process.exit stub/);
expect(exitCode).toBe(2);
} finally {
process.exit = originalExit;
}
});
});

// ==========================================================================
Expand All @@ -126,13 +183,10 @@ describe('doneAction dispatch', () => {
const lookupCallingAgent = async () => ({ id: 'team-lead-uuid', kind: 'permanent' as const });

try {
await expect(doneAction(undefined, { turnCloseFn: turnCloseFn as never, lookupCallingAgent })).rejects.toThrow(
/process.exit stub/,
);
await expect(
doneAction(undefined, REPORT, { turnCloseFn: turnCloseFn as never, lookupCallingAgent }),
).rejects.toThrow(/process.exit stub/);
expect(exitCode).toBe(4);
// turnClose must NOT be reached on rejection — the side effects
// (state='done', current_executor_id=NULL) are exactly what the
// permanent-agent guard prevents.
expect(turnCalls).toBe(0);
} finally {
process.exit = originalExit;
Expand All @@ -148,7 +202,7 @@ describe('doneAction dispatch', () => {
};
const lookupCallingAgent = async () => ({ id: 'engineer-g6-uuid', kind: 'task' as const });

await doneAction(undefined, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
await doneAction(undefined, REPORT, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
expect(turnCalls).toBe(1);
});

Expand All @@ -161,9 +215,7 @@ describe('doneAction dispatch', () => {
};
const lookupCallingAgent = async () => null;

await doneAction(undefined, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
// We could not prove permanence, so the existing turnClose path runs;
// its own ghost-executor recovery / error reporting takes over.
await doneAction(undefined, REPORT, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
expect(turnCalls).toBe(1);
});

Expand All @@ -176,9 +228,7 @@ describe('doneAction dispatch', () => {
};
const lookupCallingAgent = async () => ({ id: 'agent-degraded', kind: null });

await doneAction(undefined, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
// kind=null defaults to "let it through" — same posture as the
// shouldResume chokepoint, which treats unknown kind as task-bound.
await doneAction(undefined, REPORT, { turnCloseFn: turnCloseFn as never, lookupCallingAgent });
expect(turnCalls).toBe(1);
});

Expand All @@ -204,14 +254,14 @@ describe('doneAction dispatch', () => {
lookupCalls++;
return { id: 'team-lead-uuid', kind: 'permanent' as const };
};
const wishRefs: string[] = [];
const wishDone = async (ref: string) => {
wishRefs.push(ref);
const wishCalls: Array<{ ref: string; report: string }> = [];
const wishDone = async (ref: string, report: string) => {
wishCalls.push({ ref, report });
};

await doneAction('my-wish#3', { wishDone, lookupCallingAgent });
await doneAction('my-wish#3', REPORT, { wishDone, lookupCallingAgent });
expect(lookupCalls).toBe(0);
expect(wishRefs).toEqual(['my-wish#3']);
expect(wishCalls).toEqual([{ ref: 'my-wish#3', report: REPORT.report }]);
});
});
});
61 changes: 53 additions & 8 deletions src/term-commands/done.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,45 @@ export interface CallingAgentLookup {

interface DoneActionDeps {
/** Wish-group-done fallback. Injected for tests. */
wishDone?: (ref: string) => Promise<void>;
wishDone?: (ref: string, report: string) => Promise<void>;
/** Turn-close path. Injected for tests. */
turnCloseFn?: typeof turnClose;
/** Lookup the calling agent's id + kind. Injected for tests. */
lookupCallingAgent?: () => Promise<CallingAgentLookup | null>;
}

/** User-facing nudge when --report is missing. Mandatory so every close
* leaves a full handoff trail in events + mailbox notifications, instead
* of the silent "agent vanished" mystery. The report is the orchestrator's
* primary view into what happened — make it count. */
const REPORT_MISSING_HINT = [
'❌ genie done requires --report "<session handoff>".',
'',
' This is your handoff to the orchestrator. It lands in the audit trail and the',
' wave/wish-complete notification, and is the ONLY summary anyone reading later',
' will see without replaying your transcript. Write it like you are briefing the',
' next person on call.',
'',
' Cover:',
' • What you attempted (the actual goal of this turn / group)',
' • What shipped — files changed, PRs opened, migrations run, services touched',
' • What is verified vs unverified (tests passed? smoke run? CI green?)',
' • What is left, blocked, or deferred — and why',
' • Surprises or decisions a future agent needs to know (data losses, infra',
' quirks, hooks fired, anything non-obvious)',
'',
' Length: as long as it needs to be. A one-liner is almost never enough.',
' Multi-line is fine — pass via heredoc or a file:',
" genie done --report \"$(cat <<'EOF'",
' Goal: wire dev-local auth bridge for hv tenant.',
' Shipped: PR #143 (fixtures), PR #144 (smoke). Both green on CI.',
" Verified: 'make smoke' passed locally; tenant-A login round-trip OK.",
' Left: CSRF rotation deferred to followup (issue #1245).',
' Notes: had to bump core@1.260507.5 — desktop shell rebuild required.',
' EOF',
' )"',
].join('\n');

/**
* Default lookup: resolve the calling agent's id + kind via GENIE_EXECUTOR_ID.
*
Expand Down Expand Up @@ -99,34 +131,47 @@ async function rejectIfPermanent(deps: DoneActionDeps): Promise<void> {
}
}

async function runAgentSessionPath(deps: DoneActionDeps): Promise<void> {
async function runAgentSessionPath(deps: DoneActionDeps, report: string): Promise<void> {
await rejectIfPermanent(deps);
const fn = deps.turnCloseFn ?? turnClose;
const result = await fn({ outcome: 'done' });
const result = await fn({ outcome: 'done', reason: report });
if (result.noop) {
console.log(`ℹ️ Executor ${result.executorId} already closed — no-op.`);
} else {
console.log(`✅ Turn closed: outcome=done, executor=${result.executorId}`);
console.log('--- Handoff ---');
console.log(report.trimEnd());
console.log('--- End handoff ---');
}
}

export async function doneAction(ref: string | undefined, deps: DoneActionDeps = {}): Promise<void> {
export async function doneAction(
ref: string | undefined,
options: { report?: string },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding a default value to the options parameter makes the function more robust, especially since it's an exported function that might be called from other contexts (like tests or future commands) where the options object might be omitted.

options: { report?: string } = {},

deps: DoneActionDeps = {},
): Promise<void> {
const agentName = process.env.GENIE_AGENT_NAME;
const report = options.report?.trim();

if (!report) {
console.error(REPORT_MISSING_HINT);
process.exit(2);
}

try {
if (!ref && agentName) {
await runAgentSessionPath(deps);
await runAgentSessionPath(deps, report);
return;
}

if (ref) {
const fallback =
deps.wishDone ??
(async (r: string) => {
(async (r: string, rpt: string) => {
const { doneCommand } = await import('./state.js');
await doneCommand(r);
await doneCommand(r, rpt);
});
await fallback(ref);
await fallback(ref, report);
return;
}

Expand Down
Loading
Loading