feat(done): make --report mandatory on every close#1701
Conversation
Why: when an agent closes a turn (or a team-lead marks a wish group done), the only audit-trail entry today is "Agent <uuid> killed" with no actor, no rationale, no summary. When auto-cleanup cascades on wish completion the parent/orchestrator sees N agents vanish with zero context. The "Run \`genie team done\` to clean up" message in notifyWaveCompletion is also misleading — the next line in doneCommand calls autoCleanupTeam() unconditionally, so the message implies cleanup is pending while the team is already disbanded. Change: add -r/--report <message> as a required option on both \`genie done [ref]\` and \`genie wish done <ref>\`. Validation lives in the action handlers (not Commander's requiredOption) so we emit a multi-line friendly hint with examples instead of Commander's generic missing-option error. The report flows through: - turnClose() reason for agent-session closes (already supported, was unused for outcome=done) - notifyWaveCompletion mailbox message so the orchestrator sees WHAT shipped, not just WHICH groups closed - console output of doneCommand for terminal observers Wave/wish-complete notification now also names auto-cleanup honestly: "Team will be auto-cleaned. Run \`genie team done\` to confirm or override." instead of implying nothing has happened. This does not change the auto-cleanup behavior itself — that's a separate over-reach (kills team members unrelated to the completed wish, including workspace primary agents) worth a follow-up that scopes killTeamMembers to the wish_slug of the completed wish. Tests: 14 done.test.ts cases pass (12 existing + 2 new for the mandatory-report path). Bundle builds clean.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request makes the --report option mandatory for the done and wish done commands, ensuring that every task completion includes a descriptive handoff note for audit trails and notifications. The implementation includes validation to ensure reports are non-empty and updates to the notification system to display these summaries. Feedback suggests improving the robustness of the doneAction function by providing a default value for the options parameter and centralizing the duplicated validation logic between modules to improve maintainability.
| export async function doneAction(ref: string | undefined, deps: DoneActionDeps = {}): Promise<void> { | ||
| export async function doneAction( | ||
| ref: string | undefined, | ||
| options: { report?: string }, |
There was a problem hiding this comment.
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 } = {},| const report = options.report?.trim(); | ||
| if (!report) { | ||
| console.error( | ||
| '❌ genie wish done requires --report "<one-line summary of what was completed>".\n' + | ||
| ' This is your handoff note — it lands in the audit trail and the wave/wish-complete\n' + | ||
| ' notification so the orchestrator can see WHY a close happened.\n' + | ||
| " Example: genie wish done my-wish#3 --report 'group 3 done — fixtures + smoke'", | ||
| ); | ||
| process.exit(2); | ||
| } |
There was a problem hiding this comment.
This validation logic and the detailed error message are nearly identical to the implementation in doneAction (within src/term-commands/done.ts). To improve maintainability, consider centralizing this logic. If exporting a helper from done.ts into wish.ts introduces a circular dependency cycle, ensure you use dynamic imports (require() or import()) to break the cycle at module load time.
References
- Use dynamic imports (require() or import()) to break circular dependency cycles that would otherwise occur at module load time.
The previous wording ("one-line summary of what you did") understated
what the report is for. The report is the orchestrator's primary view
into a closing turn — the only summary anyone reading later will see
without replaying the transcript. A one-liner is almost never enough.
- CLI hint now asks for a structured handoff: goal attempted, what
shipped, verified vs unverified, what's left or deferred, surprises.
- Help text and wish-done error mirror the same structure.
- Multi-line reports render as fenced "--- Handoff ---" blocks in
both console output and the wave/wish-complete mailbox so the
structure survives renderers and the indented "Report:" prefix
doesn't mangle line breaks.
- Help text explicitly tells the user to pass via heredoc for
multi-line reports — the natural path for a real session summary.
No API changes; tests still pass (14/14).
2 participants
Why
When an agent closes a turn — or a team-lead marks a wish group done — the only audit-trail entry today is
Agent <uuid> killedwith no actor, no rationale, no summary. WhenautoCleanupTeam()cascades on wish completion, the parent/orchestrator sees N agents vanish with zero context. I just spent ~30 minutes reconstructing a 3-agent disappearance from systemd-journal and pgserve queries because nothing in the runtime events explained why they were gone.There's also a UX bug worth flagging:
notifyWaveCompletionsends"Run \genie team done` to clean up."as the wave-complete message, but the very next line indoneCommandcallsautoCleanupTeam()` unconditionally. The message implies cleanup is pending; reality is the team has already been disbanded by the time the message lands. This PR makes the notification honest about what already happened.(The auto-cleanup over-reach itself — killing team members that weren't part of the completed wish, including the workspace primary agent — is a separate problem worth fixing in a follow-up: scope
killTeamMembersto agents whosewish_slugmatches the completed wish.)What
Add
-r, --report <message>as a required option on bothgenie done [ref]andgenie wish done <ref>.requiredOption) so we can emit a multi-line friendly hint with usage examples instead of Commander's genericerror: required optionline.The report flows through three surfaces:
turnClose()reasonfor agent-session closes (the field already existed, was just unused foroutcome=done).notifyWaveCompletionmailbox message so the orchestrator sees WHAT shipped, not just WHICH group closed.consoleoutput ofdoneCommandfor terminal observers.The wave/wish-complete notification also now names auto-cleanup honestly: "Team will be auto-cleaned. Run `genie team done` to confirm or override." instead of implying nothing has happened yet.
Files
src/genie.ts— register-r, --reportondone [ref]src/term-commands/done.ts— makereportmandatory; pass throughturnCloseandwishDonesrc/term-commands/state.ts—doneCommand(ref, report); threadreportintonotifyWaveCompletion; honest messagesrc/term-commands/wish.ts— register-r, --reportonwish done <ref>with same validation hintsrc/term-commands/done.test.ts— 12 existing tests updated to new signature, 2 new tests for the mandatory-report pathTest plan
bun test src/term-commands/done.test.ts— 14 pass, 0 failbunx biome check— clean (no fixes applied)bun run build— cleanbun dist/genie.js done --help— shows new optionbun dist/genie.js done some#1(no--report) — exits 2 with friendly hintbun dist/genie.js wish done some#1(no--report) — exits 2 with friendly hintOut of scope (follow-up)
autoCleanupTeam()over-reach: kills all team members on first wish-complete, not just agents tagged to that wish. Includes workspace primary agent and unrelated PR-review workers. Will file separately.