Skip to content

Don't pass a failed outgoing turn's error on as the handoff recap - #959

Open
zcy22606 wants to merge 1 commit into
hardbeat920:mainfrom
zcy22606:fix/failed-handoff-recap
Open

zcy22606 wants to merge 1 commit into
hardbeat920:mainfrom
zcy22606:fix/failed-handoff-recap

Conversation

@zcy22606

@zcy22606 zcy22606 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What changed

requestOutgoingHandoff returns an empty recap when the outgoing turn fails: it reported session.error or usage.limited, or sendHarnessTurn threw. The caller (chooseHandoffBrief) then uses the deterministic recap built from the transcript, which is what the existing catch comment already intended.

Why

When a session switches providers, the outgoing agent is asked to write the recap. If that turn fails, providers often stream the error as assistant text before failing, e.g. Claude Code with "Your organization has disabled Claude subscription access for Claude Code". requestOutgoingHandoff kept whatever message.delta text it had collected, and since such a message is longer than MIN_AGENT_BRIEF, chooseHandoffBrief used it as the brief. The incoming agent then received the outgoing provider's error message instead of a summary of the conversation. The same applied to partial text from a turn that threw midway.

UI

No layout changes. After a failed outgoing recap, the handoff card shows the transcript-based recap instead of the provider's error text.

Validation

  • New handoffTurn.test.ts: a streamed recap is returned; an error streamed as assistant text followed by session.error yields ""; partial text from a turn that throws yields "". The last two fail on main.
  • npm run check:web: vitest (5287 passed, 13 skipped) and tsc --noEmit. No Rust changes.

Checklist

  • I ran npm run check (check:web; no Rust files changed)
  • This PR is small and focused
  • I did not mix unrelated changes

Summary by CodeRabbit

  • Bug Fixes
    • Outgoing handoffs now use a reliable fallback when an error or usage limit interrupts the recap, instead of returning incomplete or error-related text.
    • Successful handoffs continue to include the completed recap.

When switching providers, the outgoing agent is asked for a recap. If
that turn fails (the account lost access, a usage limit, the CLI
exiting), the provider often streams its error as assistant text first,
and that text became the brief the incoming agent received.

Treat a turn that reported session.error or usage.limited, or threw, as
failed and return nothing, so the caller falls back to the deterministic
recap built from the transcript.
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 79a93fe6-170e-49a0-9039-4a985a504d40
📥 Commits

Reviewing files that changed from the base of the PR and between 226ec5b and a78ed03.

📒 Files selected for processing (2)
  • src/features/sessions/model/handoffTurn.test.ts
  • src/features/sessions/model/handoffTurn.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

requestOutgoingHandoff now tracks turn failures and returns an empty string on failure. Tests cover streamed recap text, failure events, and thrown errors.

Changes: Outgoing handoff

Layer / File(s) Summary
Track failures and select the return value
src/features/sessions/model/handoffTurn.ts, src/features/sessions/model/handoffTurn.test.ts
The handoff turn marks session.error, usage.limited, and thrown errors as failures. It returns an empty string on failure and the trimmed brief otherwise. Tests cover successful and failed turns.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a78ed

No demonstrated issue remains that would prevent merging after normal checks.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preventing failed outgoing-turn errors from being used as the handoff recap.
Description check Passed The description includes all required sections, explains the change and reason, documents UI impact, records validation, and completes the checklist.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant