fix: PR opened by item done uses the agent's own summary, not a placeholder - #419
Conversation
…holder
push_and_open_pr hardcoded "Auto-opened on \`item done\` for {id}." as every
PR body, regardless of whether real information about the change was
available. agentflare work already parses the headless agent's own final
reply (parse_claude_reply) -- its prompt explicitly asks the agent to
"summarize what you changed and why" -- but that text only ever reached the
item as a comment, never the PR itself. Reviewers opened these PRs cold,
with a generic placeholder as the only description, no matter how good the
agent's own summary was.
- types.rs: ItemRequest gains an optional `summary` field, scoped to `done`
(mirrors the existing `push` field's scoping), documented in the item
tool's own description.
- worktree.rs: push_and_open_pr takes an optional summary and uses it as
the PR body when non-blank, falling back to the old placeholder
otherwise. Extracted the body-construction logic into a small pure
pr_body() so it's directly unit-testable without needing a real repo
remote/gh client (push_and_open_pr itself soft-fails without one).
- item.rs: item_done threads req.summary through to both push_and_open_pr
call sites.
- work.rs: agentflare work's own item_done call (the common path -- a
headless run that just replies with text and lets the wrapper handle
`done`) now passes the already-parsed reply_text as the summary. When the
agent instead calls `done` itself mid-session (as item #43's PR #417 did),
it can pass its own `summary` directly per the updated tool description.
Agentflare-Agent: claude-code
Agentflare-Branch: fix-pr-summary-uses-agent-reply
📝 WalkthroughWalkthroughThe done workflow now accepts agent-generated or caller-provided summaries. The worktree flow trims nonblank summaries for pull-request bodies and retains the existing placeholder when no usable summary is provided. ChangesSummary propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant item_done
participant push_and_open_pr
participant pr_body
participant PR creation
item_done->>push_and_open_pr: optional summary
push_and_open_pr->>pr_body: item and summary
pr_body-->>push_and_open_pr: trimmed summary or placeholder
push_and_open_pr->>PR creation: pull-request body
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/work.rs`:
- Around line 509-519: Update the completion flow around the wrapper’s
mcp.item_done call to reconcile an agent-initiated done before relying on the
second call: ensure reply_text is applied to an existing PR body when the
initial summary was omitted, or apply it before opening the PR. Correct the
nearby comment so it no longer claims the second call is a no-op while the item
remains in_review.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61f304a3-81c8-489c-ae66-7e2579f573b6
📒 Files selected for processing (5)
src/cli/work.rssrc/mcp_server.rssrc/mcp_server/item.rssrc/mcp_server/types.rssrc/worktree.rs
| // The agent may already have called `done` itself with its own | ||
| // `summary` (in which case this second call is a no-op — the | ||
| // claim is already released) -- but the common case is a | ||
| // headless run that just replies with text and lets this | ||
| // wrapper handle `done`, so pass the parsed reply through as | ||
| // the PR body rather than leaving it as the generic | ||
| // placeholder. | ||
| let done_resp = match mcp.item_done(ItemRequest { | ||
| action: "done".into(), | ||
| id: Some(item_id.into()), | ||
| summary: Some(reply_text.clone()), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Reconcile agent-initiated done calls before relying on this second call.
If the agent calls done and opens a PR, src/mcp_server/item.rs keeps the claim held while the item is in_review (Lines 627-636). The wrapper therefore calls item_done again instead of treating it as a no-op. src/worktree.rs then returns the existing PR (Lines 146-151) without applying reply_text to its body.
If the first done omitted summary, the generic placeholder remains. Update the existing PR body or change the completion flow so the parsed reply is applied before the PR is opened. Correct the no-op comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/work.rs` around lines 509 - 519, Update the completion flow around
the wrapper’s mcp.item_done call to reconcile an agent-initiated done before
relying on the second call: ensure reply_text is applied to an existing PR body
when the initial summary was omitted, or apply it before opening the PR. Correct
the nearby comment so it no longer claims the second call is a no-op while the
item remains in_review.
Summary
push_and_open_prhardcoded"Auto-opened on \item done` for {id}."as every PR body, regardless of whether real information about the change was available.agentflare workalready parses the headless agent's own final reply (parse_claude_reply`) — its prompt explicitly asks the agent to "summarize what you changed and why" — but that text only ever reached the item as a comment, never the PR itself. Reviewers opened these PRs cold, with a generic placeholder as the only description, no matter how good the agent's own summary was.Noticed on item #43 → PR #417: the item's own comment history had a real summary of the 8-task capacity-governor work, but the PR body was just the placeholder.
Changes
types.rs:ItemRequestgains an optionalsummaryfield, scoped todone(mirrors the existingpushfield's scoping), documented in theitemtool's own description.worktree.rs:push_and_open_prtakes an optional summary and uses it as the PR body when non-blank, falling back to the old placeholder otherwise. Extracted the body-construction logic into a small purepr_body()so it's directly unit-testable without needing a real repo remote/ghclient (push_and_open_pritself soft-fails without one).item.rs:item_donethreadsreq.summarythrough to bothpush_and_open_prcall sites.work.rs:agentflare work's ownitem_donecall (the common path — a headless run that just replies with text and lets the wrapper handledone) now passes the already-parsedreply_textas the summary. When the agent instead callsdoneitself mid-session (as item Replace ponytail platform detection with agent-detector process-tree approach #43's PR feat: capacity governor / backoff forworkautonomous job runner #417 did), it can pass its ownsummarydirectly per the updated tool description.Test plan
pr_body_uses_the_summary_when_given_one,pr_body_trims_the_summary,pr_body_falls_back_to_the_placeholder_when_summary_is_none,pr_body_falls_back_to_the_placeholder_when_summary_is_blankcargo test --bin agentflare -- worktree::tests:: cli::work::tests::— 25 passed, 0 failedcargo build --workspacecleancargo clippy --bin agentflare --all-targets— no new warningscargo fmt --checkcleanSummary by CodeRabbit
New Features
Bug Fixes