fix: address round-2 voice-cross-guardian review feedback#7552
Conversation
1. Fix mac channel guardian-answer ordering: call answerCall before resolveGuardianActionRequest so failed delivery leaves request pending for retry from another channel (mirrors channel-routes.ts). 2. Persist voice transcripts directly to conversation_store alongside notifier fires so transcript history survives without a live daemon Session listening on the voice thread. 3. Fix SKILL.md codeLength default documentation (4 → 6) to match the actual schema default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| conversationStore.addMessage( | ||
| session.conversationId, | ||
| 'user', | ||
| JSON.stringify([{ type: 'text', text: msg.voicePrompt }]), | ||
| ); |
There was a problem hiding this comment.
🔴 Duplicate transcript persistence when a live daemon Session is listening on the voice conversation
Every caller and assistant transcript is persisted twice to the voice conversation when a daemon Session is actively listening. The new direct conversationStore.addMessage calls write the raw transcript, and then fireCallTranscriptNotifier triggers the notifier callback in session-notifiers.ts:125-129 which also calls conversationStore.addMessage on the same conversationId.
Root Cause and Impact
The new code at assistant/src/calls/relay-server.ts:459-462 persists caller transcripts:
conversationStore.addMessage(
session.conversationId, // voice conversation
'user',
JSON.stringify([{ type: 'text', text: msg.voicePrompt }]),
);Then fireCallTranscriptNotifier on line 464 fires the callback registered in assistant/src/daemon/session-notifiers.ts:119-129, which also persists to the same conversation:
conversationStore.addMessage(
conversationId, // same voice conversation
'assistant',
JSON.stringify([{ type: 'text', text: transcriptText }]),
);The same duplication occurs for assistant transcripts in call-orchestrator.ts:458-462 followed by fireCallTranscriptNotifier on line 463.
When no Session is listening the notifier doesn't fire, so only the new direct write happens (correct). But when a Session IS listening, both writes execute, producing two messages per transcript event. The messages also have inconsistent roles: the direct write uses 'user' for caller utterances while the notifier always writes as 'assistant' with a formatted prefix. This corrupts the voice conversation history with duplicate, inconsistently-roled messages.
Prompt for agents
The fix needs to coordinate between the new direct conversationStore.addMessage calls (in relay-server.ts:459-462 and call-orchestrator.ts:458-462) and the existing notifier callback in session-notifiers.ts:119-142 that also calls conversationStore.addMessage on the same conversation. Two approaches:
1. Remove the conversationStore.addMessage call from the notifier callback in assistant/src/daemon/session-notifiers.ts lines 125-129 (the registerCallTranscriptNotifier callback), since the new direct writes in relay-server.ts and call-orchestrator.ts now handle persistence unconditionally. The notifier would then only push to ctx.messages and send to the client. You'd also need to ensure the notifier's ctx.messages.push uses the same format/role as the direct write.
2. Alternatively, only call conversationStore.addMessage in relay-server.ts/call-orchestrator.ts when no notifier is registered (i.e., no live Session is listening). This could be done by checking if fireCallTranscriptNotifier returned without invoking a callback, but the current API doesn't support that.
Approach 1 is cleaner: keep the direct writes as the single source of truth for persistence, and let the notifier only handle in-memory state and client notifications.
Was this helpful? React with 👍 or 👎 to provide feedback.
| conversationStore.addMessage( | ||
| session.conversationId, | ||
| 'assistant', | ||
| JSON.stringify([{ type: 'text', text: spokenText }]), | ||
| ); |
There was a problem hiding this comment.
🔴 Duplicate assistant transcript persistence when a live daemon Session is listening on the voice conversation
Same duplication issue as the caller transcript path, but for assistant transcripts in the call orchestrator.
Root Cause
The new code at assistant/src/calls/call-orchestrator.ts:458-462 persists the assistant transcript directly:
conversationStore.addMessage(
session.conversationId,
'assistant',
JSON.stringify([{ type: 'text', text: spokenText }]),
);Then on line 463, fireCallTranscriptNotifier triggers the notifier in assistant/src/daemon/session-notifiers.ts:125-129 which persists a second formatted copy to the same conversation as 'assistant' role with "**Live call transcript**\nAssistant: ..." prefix. This results in two assistant messages per LLM turn in the voice conversation when a Session is listening.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bf00b628e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| session.conversationId, | ||
| 'assistant', | ||
| JSON.stringify([{ type: 'text', text: spokenText }]), | ||
| ); | ||
| fireCallTranscriptNotifier(session.conversationId, this.callSessionId, 'assistant', spokenText); |
There was a problem hiding this comment.
This block now writes the assistant transcript directly to conversation_store and then still calls fireCallTranscriptNotifier; when a daemon session is attached, the notifier callback in assistant/src/daemon/session-notifiers.ts also calls conversationStore.addMessage for the same transcript event. The same pattern was added for caller transcripts in assistant/src/calls/relay-server.ts, so any open voice thread now stores each utterance twice, inflating history/memory indexing and polluting later model context with duplicates.
Useful? React with 👍 / 👎.
#7539) * refactor: rename ASK_USER marker to ASK_GUARDIAN (#7507) Co-authored-by: Claude <noreply@anthropic.com> * feat: add voice channel identity and per-call voice conversations (#7512) Co-authored-by: Claude <noreply@anthropic.com> * fix: address M2 review feedback — call session lookup + voice probe (#7524) Co-authored-by: Claude <noreply@anthropic.com> * feat: voice event projection, pointer messages, and bridge removal (#7529) Co-authored-by: Claude <noreply@anthropic.com> * feat: DTMF callee verification for outbound voice calls (#7533) Co-authored-by: Claude <noreply@anthropic.com> * feat: cross-channel guardian data model, store, and dispatch (#7534) Co-authored-by: Claude <noreply@anthropic.com> * feat: cross-channel guardian answer resolution (#7535) When a guardian action request is dispatched to telegram/sms/mac channels during a voice call, replies on any of those channels are now intercepted, validated, and used to resume the call: - Channel inbound (telegram/sms): intercept guardian answers early in handleChannelInbound(), with identity verification, single/multi-delivery disambiguation via request codes, and first-writer-wins resolution - Mac thread: intercept in session-process processMessage() before the agent loop, routing the user message as a guardian answer - Guardian dispatch: create mac conversations server-side with getOrCreateConversation() and seed them with the question text - Store: add getPendingDeliveryByConversation() for mac channel routing Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * feat: guardian action expiry sweep, voice thread visibility, and voice settings card (#7536) Add periodic sweep (60s interval) for expired cross-channel guardian action requests. When a request expires: marks request+deliveries as expired, expires pending questions, and sends expiry notices to external channels and mac threads. Allow voice-channel threads to appear in the desktop thread list by updating the session filter in both ThreadSessionRestorer and ThreadManager to pass through sessions with sourceChannel == "voice". Add a Voice (Phone Calls) card to the Settings Connect tab showing Twilio credential and phone number readiness for voice calls. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * docs: update SKILL.md and ARCHITECTURE.md for voice-cross-guardian M1-M7 changes (#7538) Reflect the cross-channel guardian architecture in documentation: - SKILL.md: add DTMF callee verification section, update answering questions to describe ASK_GUARDIAN cross-channel dispatch with first-response-wins semantics, note mid-call steering via desktop chat is no longer supported, add accepted regressions section - ARCHITECTURE.md: update outgoing calls intro to describe voice as first-class channel with per-call conversations, replace bridge-based Mermaid diagram flow with guardian dispatch flow, replace call-bridge key component with guardian-dispatch/guardian-action-store/guardian- action-sweep, replace Call Bridge section with Cross-Channel Guardian Consultation, add guardian_action_requests and guardian_action_deliveries SQLite tables, add guardian modules to Channel Guardian Security table Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: address voice-cross-guardian review feedback (5 issues) (#7550) 1. Pass broadcast/assistantId to CallOrchestrator from RelayConnection via module-level setRelayBroadcast wired in lifecycle.ts, so mac desktop receives guardian_request_thread_created IPC events and multi-assistant deployments use the correct assistant ID. 2. Thread bearer token through guardian dispatch deliverToExternalChannel so gateway /deliver/{channel} calls include Authorization header. 3. Swap resolve/answerCall ordering in channel-routes guardian answer interception: call answerCall first, resolve only on success, so failed answers leave the request pending for retry. 4. Use content block array format for addMessage calls in guardian-dispatch.ts and guardian-action-sweep.ts to match codebase convention (JSON.stringify([{type:'text',text:'...'}])). 5. Expire deliveries in 'sent' status (not just 'pending') in expireGuardianActionRequest using inArray. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: address round-2 voice-cross-guardian review feedback (#7552) 1. Fix mac channel guardian-answer ordering: call answerCall before resolveGuardianActionRequest so failed delivery leaves request pending for retry from another channel (mirrors channel-routes.ts). 2. Persist voice transcripts directly to conversation_store alongside notifier fires so transcript history survives without a live daemon Session listening on the voice thread. 3. Fix SKILL.md codeLength default documentation (4 → 6) to match the actual schema default. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: make voice transcript/completion persistence session-independent * fix voice call transcript handling and close review gaps --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
Summary
Addresses the second round of review feedback on PR #7539:
P1 — Mac guardian-answer ordering (
session-process.ts): CallanswerCall()beforeresolveGuardianActionRequest()so that a failed delivery (e.g. call timed out) leaves the request pending for retry from another channel. Mirrors the pattern already fixed inchannel-routes.ts.P1 — Voice transcript/completion persistence (
relay-server.ts,call-orchestrator.ts): Persist caller and assistant transcripts directly toconversation_storealongside the notifier fires. This ensures the voice thread retains transcript history even when no live daemonSessionis listening on it.Yellow — SKILL.md codeLength default (
SKILL.md): Fix documented default from4to6to match the actual schema default.Test plan
answerCallbefore resolving the requestcodeLengthdefault as6🤖 Generated with Claude Code