Skip to content

feat(acpbridge): add display_message to trigger for improved transcri… - #707

Merged
pikann merged 5 commits into
masterfrom
feature/enhance-acp-agent
Nov 5, 2026
Merged

pikann merged 5 commits into
masterfrom
feature/enhance-acp-agent

Conversation

@pikann

@pikann pikann commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three fixes for ACP-bridged agents and the ask_user_question card:

  • Transcript shows what the user typed. The bridge recorded the user_message event from the prompt-wrapped message (skill trigger, project context, trigger context). agent-runner now sends the raw text as display_message in start_turn, and the bridge uses it for the user_message event while still sending the full message to the agent. Falls back to message when display_message is missing or blank (older server, auto-assigned task with no text).
  • Agent knows whom to mention when asking. A bridged ACP agent runs the user's own Paca MCP server, so PACA_CONVERSATION_ID is never set and ask_user_question could not resolve who to @-mention on task/doc comments. BuildACPMessage now adds an "Asking the user" section with mention = owner (id: <member id>) whenever the trigger has an actor member.
  • Ask card no longer breaks with long content. Added min-w-0, full width and word-breaking to the card, question, option label/description and answered line, so long tokens wrap instead of overflowing and getting clipped.

Changes

  • apps/acp-bridge/internal/runner/runner.go: read display_message and context_items, thread them through runTurn, emit the user_message event from them (context badges are kept even when the text is blank).
  • services/agent-runner/internal/acpbridge/dispatch.go: send display_message (trigger.Message) and context_items.
  • services/agent-runner/internal/acpbridge/message.go: add the mention hint to the ACP prompt.
  • apps/web/src/components/shared/ask-question-card.tsx, apps/web/src/components/assistant-ui/tool-fallback.tsx: layout fixes.

Testing

  • go test ./internal/acpbridge/ (agent-runner) and go test ./internal/runner/ (acp-bridge) pass; web tsc -b and biome pass.
  • New tests cover display_message (present, absent, blank), context_items, and the mention hint in BuildACPMessage.
  • Not exercised against a live ACP conversation.

Notes

  • Rebuild both the agent-runner container and the ACP bridge to pick this up.
  • Conversations already recorded keep the long user message in their stored events.
  • The in-app question notification path for ACP chats was not verified.

@pullfrog pullfrog Bot left a comment

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.

Important

The transcript fix skips the trigger that needs it most: task assignments with no note still render the full internal prompt. Details inline.

Reviewed changes

  • display_message threaded end to end — agent-runner sends trigger.Message alongside the prompt-wrapped message, and the bridge now emits it as the user_message event, falling back to message when blank.
  • Asking the user prompt section — BuildACPMessage tells the agent to pass mention = owner (id: <member id>) when ask_user_question posts to a task/doc, since the user's own MCP server has no PACA_CONVERSATION_ID to derive the conversation owner.
  • Ask-card layout hardening — min-w-0, full width, word-breaking and overflow-hidden on the card and its children so long tokens wrap instead of overflowing.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues found. Both prior findings are addressed by the new commit.

Reviewed changes

  • Distinguished an absent display_message from a present-but-blank one — only a missing key falls back to the prompt-wrapped message; a blank value records no user_message, matching the llm path, with regression tests covering all three cases.
  • Threaded trigger.ContextItems through start_turn into the persisted user_message payload, so ACP transcripts render attached-context badges like the llm path, with test coverage.
  • Verified the persisted context_items shape is identical to the llm path's and is accepted by parseContextItems; both Go modules build and the targeted tests pass.

Minor note: the PR description still says a blank display_message falls back to message; the code now intentionally skips the event in that case, so that bullet is worth updating.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No critical issues — one minor suggestion inline. It concerns the new context-only user_message, which is currently emitted but never rendered.

Reviewed changes

Reviewed the single commit since the prior pullfrog review (7d5968bf → 2289f4e8): the bridge's user_message emission rule and the ask-card class change.

  • Recorded a user_message for context-only turns — emitUserMessage now only skips when both the trimmed text and contextItems are empty, so a blank display_message with attached context still records an event, covered by TestBlankDisplayMessageStillRecordsContextItems.
  • Removed overflow-hidden from the ask-card root — wrapping now relies on min-w-0 plus break-words on the card's children; the dispatch.go comment now warns that display_message must never gain omitempty.
  • Made the mention hint's member id explicit — trigger.ActorMemberID.String() instead of %s on the pointer (same output, *uuid.UUID already satisfied Stringer), with new tests for the section being present and absent.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No critical issues — one small visual follow-up inline.

Reviewed changes

Reviewed the single commit since the prior pullfrog review (2289f4e8 → 77dde99): how the transcript converter treats a user_message that carries context items but no text.

  • Kept context-only turns in the transcript — eventsToThreadMessages now parses context_items before its drop guard and skips a user_message only when it has neither text nor context, so the bridge's context-only event renders instead of being persisted and discarded.
  • Covered both branches with tests — a blank-text message with context items is kept with metadata.custom.contextItems, a blank-text message without them is dropped, and a normal message still gets no metadata.custom.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues found. The context-only transcript bubble fix from the last review is in place.

Reviewed changes

Reviewed the single commit since the prior pullfrog review (77dde99 → f3d9f1c): the transcript converter's handling of a user_message that carries context items but no text.

  • Hid the empty bubble for context-only turns — eventsToThreadMessages now pushes content: [] instead of an empty text part when there is no text, so the bubble and its action bar stay hidden while the context badges render, and the regression test asserts the empty content array.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@pikann
pikann merged commit 55bdeae into master Nov 5, 2026
9 checks passed
@pikann
pikann deleted the feature/enhance-acp-agent branch November 5, 2026 17:24
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