Skip to content

fix(codebuddy): gate the quirk handling on the whole family, route linked sub-agent chunks - #946

Open
Liang-HZ wants to merge 3 commits into
spacering-net:mainfrom
Liang-HZ:feat/codebuddy-family-quirks
Open

Liang-HZ wants to merge 3 commits into
spacering-net:mainfrom
Liang-HZ:feat/codebuddy-family-quirks

Conversation

@Liang-HZ

Copy link
Copy Markdown
Contributor

The problem

CodeBuddy's ACP stream needs a handful of special-case handling in codeg — sub-agent chunks that must not leak into the main thread, the Agent-pill title rewrite, deferred-ExecuteTool unwrapping. Every one of those gates was written as agent_type == AgentType::CodeBuddy (or !=, for the guards), i.e. it only recognized the built-in agent.

A custom registration that is the CodeBuddy CLI — the common multi-account / env-isolated setup (codebuddy, codebuddy-cn, codebuddy-intl… in the registry) — streams exactly the same shapes, but matched none of the gates:

  • sub-agent thought/message chunks leaked into the main thread (rendered twice: once in the stream, once inside the Agent pill's window, out of order);
  • the Agent pill missed its title rewrite;
  • deferred MCP results stayed wrapped in the ExecuteTool envelope.

What this changes

1. is_codebuddy_family gates all eight quirk sites — the built-in, or a custom id that is codebuddy / codebuddy-*. The id-prefix scope is deliberate: it keeps every quirk on agents that ARE the CodeBuddy CLI, so no unrelated agent's codebuddy.ai/* _meta keys can alias into the handling.

2. Suppression becomes the fallback; linked chunks are routed. Suppression was the only tool before: a chunk whose wire carried its own link was dropped with everything else once it fell outside the tracking window, so the settled Agent capsule lost the sub-agent's thinking/text. Measured on CodeBuddy 2.164.0: every sub-agent thought/message chunk carries a flat _meta["codebuddy.ai/parentToolCallId"], and no main-thread chunk does — so a linked chunk can be routed exactly like Claude's parented chunks (forwarded with its attribution; the frontend files it into the live Agent capsule). should_suppress_subagent_chunk now yields to a link and only catches unlinked chunks.

The attribution reader is renamed claude_chunk_parent_tool_use_id → subagent_chunk_parent_tool_use_id, since it now serves two wire shapes: Claude Code's nested _meta.claudeCode.parentToolUseId (claude-agent-acp ≥0.63 with the subagent-transcript capability) and the family's flat key. Each family's own id space stays separate — the flat key on a Claude chunk and the nested key on a CodeBuddy chunk both read as absent.

Verification

  • cargo test / cargo clippy -- -D warnings: green on the branch.
  • New/extended unit tests: the family scope (built-in + variants, near-misses rejected), the custom variant's suppression + meta classification path, the flat-key reader (family-only; empty / wrong type / absent key → None), and the routing fallback (linked chunk not suppressed even inside an open window; unlinked chunk still suppressed).
  • Live: on a dual-account setup (custom:codebuddy-intl), sub-agent thinking/text reappears inside the Agent capsule after the turn settles, and a re-opened session rebuilds the same capsules (the finish/route path runs identically at parse time).

…linked sub-agent chunks

A custom registration that IS the CodeBuddy CLI (multi-account or
env-isolated setups — `codebuddy`, `codebuddy-*` in the registry) streams
exactly the shapes the built-in agent does, but every CodeBuddy quirk was
gated on `AgentType::CodeBuddy` alone: its sub-agent thought/message chunks
leaked into the main stream (each also rendered inside the Agent pill's
window, out of order), and it missed the Agent-pill title rewrite and the
deferred-MCP unwraps. `is_codebuddy_family` (the built-in, or a `codebuddy`
/ `codebuddy-*` custom id) now gates all eight sites; the id-prefix scope
keeps every quirk on agents that ARE the CodeBuddy CLI, so no unrelated
agent's `codebuddy.ai/*` keys can alias in.

Suppression of sub-agent chunks becomes the FALLBACK. Measured on CodeBuddy
2.164.0: every sub-agent thought/message chunk carries a flat
`_meta["codebuddy.ai/parentToolCallId"]` (main-thread chunks never do), so a
linked chunk is routed exactly like Claude's parented chunks — forwarded
with its attribution, and the frontend files it into the live Agent capsule,
so the sub-agent's thinking/text survives into the settled card instead of
vanishing with the suppressed stream. The attribution reader is renamed to
`subagent_chunk_parent_tool_use_id` (two wire shapes today: Claude's nested
`_meta.claudeCode.parentToolUseId` and the family's flat key), and each
family's own id space stays separate so neither can alias the other.

Tests: the family scope, the custom variant's suppression + meta
classification path, and the flat-key reader (family-only, empty/wrong-type
absent).
@xintaofei

Copy link
Copy Markdown
Collaborator

@Liang-HZ hi, and thank you for this PR! 🙏

The problem is real, and the description pins it down well: a custom registration that runs the CodeBuddy CLI streams exactly what the built-in agent streams, yet none of codeg's CodeBuddy-specific handling in the connection layer reaches it, so sub-agent chunks spill into the main thread. The measurement is what makes the second half possible, and it holds up. I unpacked @tencent-ai/codebuddy-code@2.164.0, and in dist/codebuddy-headless.js every update converted from a sub-agent's model events gets _meta["codebuddy.ai/parentToolCallId"], message and thought chunks included, while main-thread events never get one. Routing those chunks into the Agent card the way Claude's linked chunks already go, instead of dropping them, is the right direction. Renaming the reader and keeping each family's key apart was careful, the tests cover the predicates' edge cases (near-miss ids, empty and non-string links), and CI is green on all seven jobs.

That said, I'm not going to merge it in its current form. The core reason is that with this PR, codeg would show a conversation one way while it streams and another way after it's reopened. Below is everything in one place (the two places where live and history disagree, a concern about how the family is recognized, a suggested way forward, smaller issues in the diff, and tests) to make the next round easier. If you think I got something wrong, please push back 🙂

Summary

  • §1 Main issue: custom registrations get the live handling, but their history doesn't, so a reopened conversation looks different from the live one.
  • §2 For the built-in agent, the newly routed sub-agent thinking and text have no history counterpart yet, so they disappear from the Agent card on reopen.
  • §3 The family is recognized by the id the user typed, not by what the agent actually runs.
  • §4 Suggested way forward: split the PR.
  • §5 Smaller issues in the current diff.
  • §6 Tests and manual checks.

1. Main issue: custom registrations only get the live half

  • Where a custom agent's history comes from. Every AgentType::Custom is parsed by AcpNativeParser (mod.rs L395). It rebuilds the conversation from codeg's own ACP transcript, and the connection layer queues each raw session/update for that transcript before it calls emit_conversation_update (connection.rs L10623-L10634). So the transcript holds what CodeBuddy sent, without any of the handling this PR extends.

  • That parser deliberately applies none of the per-agent handling. Its module doc says so (acp_native.rs L10-L21): "What is deliberately NOT reproduced is the per-agent quirk handling in emit_conversation_update (CodeBuddy sub-agent folding, Grok use_tool unwrapping, codex sub-agent suppression …): custom agents get no quirk handling anywhere, live or historical, so there is nothing to diverge from." For custom ids equal to codebuddy or starting with codebuddy-, this PR changes the live half of that sentence and leaves the historical half as it is.

  • What a reopened codebuddy-* conversation shows. I ran a CodeBuddy-shaped transcript through project_turns (the test and its output are in the details below). Compared with the live view after this PR:

    • The sub-agent's thinking and text sit in the main thread, between the Agent call and the main agent's reply.
    • The Agent call is named after the last title CodeBuddy sent for it. In 2.164.0 that's the converted call's title: the sub-agent type, such as general-purpose, or @<name> for a named sub-agent. The history view passes it through as the tool name (ai-elements-adapter.ts L2516-L2519), and a title like general-purpose doesn't normalize to agent, so the call renders as an ordinary tool card instead of the Agent card (content-parts-renderer.tsx L2737). Either way, its result carries no agent_stats.
    • The sub-agent's own tool calls are separate cards in the main thread, not nested in the Agent card.
    • A deferred MCP call keeps CodeBuddy's wrapper: the card keeps the DeferExecuteTool title instead of the inner tool's name, and its result keeps the {"type":"text","text":…} envelope.

    These are the same symptoms the PR fixes, now showing up after the reopen instead.

  • Why your reopen check may have looked right. I can't tell exactly how you reopened it, but when a turn finishes, the frontend keeps the copy it streamed (localTurns, conversation-runtime-store.ts L2210-L2222) and doesn't reload that turn from disk (L4386-L4387). The parser's version only replaces it when the conversation's detail is fetched again (L2019-L2023). Closing the conversation's tab and opening it again, restarting the desktop app, or reloading the page in web mode shows what the parser builds.

The projection test (paste it into the tests module of src-tauri/src/parsers/acp_native.rs)

This is a reduced fixture, not a wire capture. The Agent call's two tool_call frames follow 2.164.0's sequence for an unnamed sub-agent: the streamed opening frame titled with the tool name, then the converted call titled with the sub-agent type. The argument-streaming updates and the Agent's description content are left out. It shows what project_turns builds from these frames; it isn't a reopen in the UI.

#[test]
fn codebuddy_shaped_transcript_on_reopen() {
    let turns = project_turns(&[
        prompt(1, "explore"),
        update(2, text_chunk("agent_thought_chunk", "main plans")),
        update(3, serde_json::json!({
            "sessionUpdate": "tool_call",
            "toolCallId": "call-agent",
            "title": "Agent",
            "kind": "other",
            "status": "in_progress",
            "rawInput": {},
            "_meta": {"codebuddy.ai/toolName": "Agent"}
        })),
        update(4, serde_json::json!({
            "sessionUpdate": "tool_call",
            "toolCallId": "call-agent",
            "title": "general-purpose",
            "kind": "other",
            "status": "in_progress",
            "rawInput": {"description": "Explore the repo", "prompt": "Map the modules", "subagent_type": "general-purpose"},
            "_meta": {"codebuddy.ai/toolName": "Agent", "codebuddy.ai/isSubagent": true, "codebuddy.ai/subagentType": "general-purpose"}
        })),
        update(5, serde_json::json!({
            "sessionUpdate": "agent_thought_chunk",
            "content": {"type": "text", "text": "SUB thinks"},
            "_meta": {"codebuddy.ai/parentToolCallId": "call-agent"}
        })),
        update(6, serde_json::json!({
            "sessionUpdate": "tool_call",
            "toolCallId": "call-child",
            "title": "Read src/main.rs",
            "kind": "read",
            "status": "completed",
            "_meta": {"codebuddy.ai/toolName": "Read", "codebuddy.ai/parentToolCallId": "call-agent"}
        })),
        update(7, serde_json::json!({
            "sessionUpdate": "agent_message_chunk",
            "content": {"type": "text", "text": "SUB reports"},
            "_meta": {"codebuddy.ai/parentToolCallId": "call-agent"}
        })),
        update(8, serde_json::json!({
            "sessionUpdate": "tool_call_update",
            "toolCallId": "call-agent",
            "status": "completed",
            "content": [{"type": "content", "content": {"type": "text", "text": "agent done"}}]
        })),
        update(9, serde_json::json!({
            "sessionUpdate": "tool_call",
            "toolCallId": "call-defer",
            "title": "DeferExecuteTool",
            "kind": "other",
            "status": "in_progress",
            "rawInput": {"toolName": "mcp__codeg-mcp__get_delegation_status", "params": {"task_ids": ["t1"]}},
            "_meta": {"codebuddy.ai/toolName": "DeferExecuteTool"}
        })),
        update(10, serde_json::json!({
            "sessionUpdate": "tool_call_update",
            "toolCallId": "call-defer",
            "status": "completed",
            "content": [{"type": "content", "content": {"type": "text", "text": "{\"type\":\"text\",\"text\":\"{\\\"tasks\\\":[]}\"}"}}]
        })),
        update(11, text_chunk("agent_message_chunk", "MAIN summary")),
        entry(12, EntryKind::TurnEnd, serde_json::json!({"stopReason": "end_turn"})),
    ]);
    for b in &turns[1].blocks {
        match b {
            ContentBlock::Text { text } => eprintln!("TEXT     {text:?}"),
            ContentBlock::Thinking { text } => eprintln!("THINKING {text:?}"),
            ContentBlock::ToolUse { tool_use_id, tool_name, .. } => {
                eprintln!("TOOLUSE  {tool_use_id:?} name={tool_name:?}")
            }
            ContentBlock::ToolResult { tool_use_id, output_preview, agent_stats, .. } => eprintln!(
                "RESULT   {tool_use_id:?} out={output_preview:?} agent_stats={}",
                agent_stats.is_some()
            ),
            other => eprintln!("OTHER    {other:?}"),
        }
    }
}

cargo test --no-default-features --features server-bin --lib codebuddy_shaped_transcript_on_reopen -- --nocapture (from src-tauri/) prints one line per block of the projected assistant turn, in order:

THINKING "main plans"
TOOLUSE  Some("call-agent") name="general-purpose"
THINKING "SUB thinks"
TOOLUSE  Some("call-child") name="Read src/main.rs"
TEXT     "SUB reports"
RESULT   Some("call-agent") out=Some("agent done") agent_stats=false
TOOLUSE  Some("call-defer") name="DeferExecuteTool"
RESULT   Some("call-defer") out=Some("{\"type\":\"text\",\"text\":\"{\\\"tasks\\\":[]}\"}") agent_stats=false
TEXT     "MAIN summary"

2. Built-in CodeBuddy: the routed thinking and text have no history counterpart

  • With the routing, the built-in agent now streams the sub-agent's thinking and text into the Agent card. That's exactly the improvement the PR is after, and the live plumbing for it is already in place (see "Checked and fine" below).
  • After a reopen, parsers/codebuddy.rs rebuilds the Agent call and its result from the main transcript, and fills agent_stats from the sub-agent's own transcript. That helper only extracts tool calls: timeline is always empty (codebuddy.rs L1113), and a sub-agent that ran no tools gets no agent_stats at all (L1094-L1096). So the sub-agent's thinking and text disappear from the card on reopen; only its final answer is left, as the Agent call's result.
  • This also breaks an assumption the frontend documents (conversation-runtime-store.ts L2196-L2202): "The only live prose comes from Claude Code; elsewhere the transcript is tool markers alone, which the card renders like stats, so a reload from an agent without a parsed timeline does not change the card's shape."
  • The data for a fix is already on disk: the sub-agent's transcript under <session>/subagents/ has its assistant messages between its tool calls (I checked a local one). Claude's parser builds the same kind of timeline from its sub-agent transcripts, next to the calls it collects (claude.rs L3437-L3488).

3. Recognizing the family by id

  • is_codebuddy_family matches the id the user picks when registering the agent (connection.rs L13788-L13794). A registration called cb-intl that runs the CodeBuddy CLI gets none of this handling. One called codebuddy-wrapper that runs some other adapter gets all of it, including the codebuddy.ai/* reads that the doc comment says the scope keeps away from unrelated agents.
  • Where codeg already gives a custom registration a built-in adapter's treatment, it keys on the launch recipe: registry::uses_cursor_acp_backend (registry.rs L477-L510) checks for cursor-agent with the acp argument, not the id. For CodeBuddy the equivalent would be the codebuddy executable with --acp, which is how the built-in launches @tencent-ai/codebuddy-code.
  • Where it doesn't, it says why: the pi/codex terminal bridge keeps custom ids out on purpose, because those keys are unnamespaced (connection.rs L13955-L13961). Extending CodeBuddy's handling to custom registrations is reasonable, but it should be an explicit decision that covers how the family is detected and what history does (§1).

4. Suggested way forward

I'd split this into two pieces.

  1. Built-in CodeBuddy: the routing and its history, in one PR.

    • Keep the flat-key reader, the rule that a link beats suppression, and the tests, gated on AgentType::CodeBuddy for now.
    • In parsers/codebuddy.rs, build agent_stats.timeline from the sub-agent transcript: thinking and text entries with tool-call markers in between, the same shape Claude's parser produces. Also keep agent_stats for a sub-agent that wrote text but ran no tools.
    • For a sub-agent that finishes within its turn, the Agent card then shows the same entries in the same order live and after a reopen, and the store comment quoted in §2 can be updated to match. (A background sub-agent that outlives the turn will still differ: codeg drops its chunks once the turn has ended, so its card can show more after a reopen than it did live.)
  2. Custom registrations: agree on the direction first. I see two options, and I'd like to settle on one before code is written:

    • (a) A CodeBuddy projection in AcpNativeParser. History would fold linked chunks into the Agent card, show the Agent call as the Agent card, nest its children, and unwrap deferred calls and their results. The family would be detected from the launch recipe and recorded with the session, so editing the agent's command later doesn't change how old transcripts are projected (today the transcript header only stores the agent id). That reverses the boundary the module documents, so it's a bigger change than it looks.
    • (b) Native accounts. feat(accounts): add native accounts and in-place Claude account switching #919 (still under review) adds per-account config dirs for built-in agents, CodeBuddy included (CODEBUDDY_CONFIG_DIR). An account there stays AgentType::CodeBuddy, so it gets the built-in's live handling and history parser. If it lands and fits your setups, moving those accounts over would avoid the problem for new conversations; it wouldn't change existing custom conversations. It may not cover every env-isolated setup you have in mind, so it would help to hear what those need beyond a separate account.

    I know this leaves custom registrations with the live leak you set out to fix for now. I'd still rather fix the live view and history together than make one better and leave a reopened conversation looking different.

5. Smaller issues in the current diff

  1. A branch that can no longer fire. codebuddy_chunk_marks_subagent still returns true for a non-empty codebuddy.ai/parentToolCallId (connection.rs L14983-L15005), but its only caller, should_suppress_subagent_chunk, has already returned false for exactly that case (L15029-L15041). Its doc ("CodeBuddy is not confirmed to populate chunk _meta") is also out of date now that you've measured it. The branch can go, or the doc should say it no longer affects suppression.
  2. The Prose arm comment still describes only Claude's chunks (L17434-L17437), but the same code now routes CodeBuddy's linked chunks too.
  3. Two comments elsewhere become outdated: the AcpNativeParser module doc (§1) and the store comment quoted in §2. Whichever way §4 goes, they should match the code.
  4. The tests stop at the predicates. Nothing drives emit_conversation_update to show that a linked chunk comes out as ContentDelta/Thinking with parent_tool_use_id set, while an unlinked chunk in an open window is dropped. pi_emit_chunk (L29090-L29121) shows the harness. A CodeBuddy version needs to keep one CodeBuddyLiveState across several updates, so an Agent tool_call can open the window first.

Checked and fine

  • The 2.164.0 measurement (above).
  • Downstream, linked chunks are already handled the same way for every agent, so the built-in routing needs no frontend change for the live view:
  • The PR's own tests pass locally too.

6. Tests and manual checks

Tests:

  • Parser: a CodeBuddy session fixture whose sub-agent transcript has text before, between and after tool calls. Expect agent_stats.timeline in that order, with each tool marker pointing at the right call. Add a text-only sub-agent that still gets agent_stats.
  • Connection: with an Agent call open, a linked chunk is emitted with its parent_tool_use_id, an unlinked chunk is dropped, and a main-thread chunk after the Agent call completes is emitted with no parent.

Manual:

  • Built-in CodeBuddy, a prompt that starts a foreground sub-agent which writes some text before and after a tool call. The Agent card should show the text and the tool calls in order while it runs and after it finishes. Then close the conversation's tab, open the conversation again, and check that the card looks the same.
  • A background sub-agent running while the main agent keeps writing: its linked chunks go to its own card, and the main agent's text stays in the main thread.
  • If we go with (a) in §4, the same checks on a custom registration.

Thanks again! The measurement and the routing were the hard part, and they carry straight over to the built-in half in §4. If anything here is unclear, or you disagree with any of it, just reply here. I'm happy to look at the next round.

… timeline, emit-path tests

- drop is_codebuddy_family and every id-prefix gate: the quirk handling keys
  on AgentType::CodeBuddy exactly, so custom registrations whose id merely
  starts with "codebuddy" get none of it (pinned by a negative test)
- remove the unreachable parent-link branch in codebuddy_chunk_marks_subagent
  and rewrite its doc around the measured 2.164.0 behavior
- build agent_stats.timeline from the sub-agent transcript — thinking and
  text interleaved with tool markers, role-guarded, truncated per entry —
  and keep agent_stats for a text-only sub-agent (None only when the
  transcript has neither calls nor prose)
- hoist TIMELINE_ENTRY_MAX_CHARS to parsers/mod.rs
- drive emit_conversation_update end to end in a tokio test over several
  updates sharing one CodeBuddyLiveState: a linked chunk is emitted with its
  parent, an unlinked one is suppressed, and a main-thread chunk after the
  Agent call completes carries no parent
- sync the store/types/context comments that documented the old shape
@Liang-HZ

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — we re-verified every point locally and you're right on all of them. This round implements piece 1 of your suggested split (built-in CodeBuddy: routing + its history together); for custom registrations we'd like to settle direction first, as you proposed.

What changed:

  • §3: is_codebuddy_family and every id-prefix gate are gone. All nine gates now match AgentType::CodeBuddy exactly. A new test (custom_codebuddy_registrations_get_no_builtin_quirks) pins that a Custom("codebuddy-cn") registration gets none of the built-in quirks, so the prefix family can't quietly come back.
  • §2: agent_stats.timeline is now built from the sub-agent transcript — thinking and text entries interleaved with tool markers, the same shape Claude's parser produces. Assistant text is read from content[].output_text with a role == "assistant" guard (the transcript opens with the dispatching user prompt; roleless/malformed lines are ignored), reasoning from rawContent[].text, per-entry truncation. A text-only sub-agent keeps agent_stats (total_tool_use_count: Some(0)); stats are dropped only when there is neither a call nor prose. Still-running workers may surface partial content — accepted and documented in the helper's doc.
  • §5.1–5.3: dead branch removed, its doc rewritten around the measured 2.164.0 behavior; the Prose-arm comment now covers both link shapes (Claude nested / CodeBuddy flat); the store comment and two more frontend comments (types.ts, acp-connections-context.tsx) updated. With custom registrations back to no-quirks anywhere, the AcpNativeParser module doc matches the code again.
  • §5.4 + §6: a #[tokio::test] drives emit_conversation_update over several updates sharing one CodeBuddyLiveState (the pi_emit_chunk pattern): with an Agent call open, a linked chunk is emitted as Thinking with parent_tool_use_id, an unlinked chunk is suppressed, and a main-thread chunk after the call completes is emitted with no parent. Parser tests cover timeline ordering, dispatch-prompt exclusion, truncation, text-only stats, and the None case.
  • Verified with cargo test and cargo clippy -D warnings under both feature sets (server-bin and default+test-utils).

§4.2 custom registrations — we'd prefer (b), native accounts via #919, and would rather not start the projection work until we agree: (1) does #919's per-account CODEBUDDY_CONFIG_DIR cover the env-isolated setups you'd want, in your view? (2) if (a) ever happens, would you take the groundwork of recording the launch recipe in the transcript header? (3) we're reading your §4.1 parenthetical as "a background sub-agent outliving its turn may show more after a reopen than it did live" being an accepted delta — please confirm. We accept that custom codebuddy-* registrations return to the pre-PR live behavior until the direction lands.

…-quirks

# Conflicts:
#	src-tauri/src/parsers/claude.rs
#	src-tauri/src/parsers/codebuddy.rs
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.

2 participants