Skip to content

fix: address voice-cross-guardian review feedback (5 issues)#7550

Merged
noanflaherty merged 1 commit into
feature/voice-cross-guardianfrom
voice-cross-guardian/fix-review-feedback
Feb 24, 2026
Merged

fix: address voice-cross-guardian review feedback (5 issues)#7550
noanflaherty merged 1 commit into
feature/voice-cross-guardianfrom
voice-cross-guardian/fix-review-feedback

Conversation

@noanflaherty
Copy link
Copy Markdown
Contributor

@noanflaherty noanflaherty commented Feb 24, 2026

Summary

  • P1: broadcast/assistantId propagation — Wire setRelayBroadcast in lifecycle.ts and pass { broadcast, assistantId } opts to CallOrchestrator from RelayConnection.handleSetup(), so mac desktop receives guardian_request_thread_created IPC events and multi-assistant deployments use the correct assistant ID.
  • P1: bearer auth on guardian deliveries — Thread bearer token through deliverToExternalChannel via readHttpToken() so gateway /deliver/{channel} calls include the Authorization header in secure setups.
  • P1: resolve-before-answerCall ordering — In channel-routes guardian answer interception, call answerCall first and only resolveGuardianActionRequest on success. Failed answers leave the request pending for retry from another channel.
  • P2: bare JSON string in addMessage — Use content block array format ([{type:'text',text:'...'}]) for addMessage calls in guardian-dispatch.ts and guardian-action-sweep.ts to match codebase convention.
  • P2: delivery expiry status filter — Use inArray(['pending','sent']) in expireGuardianActionRequest so deliveries in 'sent' status are also expired (not just 'pending').

Test plan

  • Verify mac desktop receives guardian_request_thread_created IPC when a voice call triggers ASK_GUARDIAN
  • Verify external channel guardian deliveries include Authorization header
  • Verify answering a guardian question after the call's pending question has timed out does not mark the request as resolved
  • Verify mac guardian thread messages render correctly (content block array format)
  • Verify expiry sweep correctly expires deliveries that were in 'sent' status

🤖 Generated with Claude Code


Open with Devin

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>
@noanflaherty noanflaherty merged commit 829d0cd into feature/voice-cross-guardian Feb 24, 2026
@noanflaherty noanflaherty deleted the voice-cross-guardian/fix-review-feedback branch February 24, 2026 04:18
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

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.

🟡 cancelGuardianActionRequest only cancels 'pending' deliveries, missing 'sent' deliveries

The PR correctly fixed expireGuardianActionRequest to use inArray(['pending', 'sent']) so that deliveries already in 'sent' status are expired. However, cancelGuardianActionRequest at guardian-action-store.ts:293-300 has the exact same pre-existing bug that was not fixed: it only cancels deliveries with status = 'pending', leaving deliveries in 'sent' status untouched.

Root Cause and Impact

When a guardian action request is cancelled, cancelGuardianActionRequest updates delivery rows but only those with eq(guardianActionDeliveries.status, 'pending'). In the dispatch flow (guardian-dispatch.ts:139-140), mac deliveries are immediately set to 'sent' via updateDeliveryStatus(delivery.id, 'sent'), and external channel deliveries are set to 'sent' after successful HTTP POST. This means most active deliveries will be in 'sent' status by the time a cancellation occurs.

As a result, cancelling a guardian action request leaves sent deliveries in 'sent' status instead of 'cancelled'. Any logic that checks delivery status to determine if a response should still be accepted would incorrectly treat these as active deliveries. The same reasoning that motivated the inArray(['pending', 'sent']) fix for expireGuardianActionRequest applies equally here.

(Refers to lines 293-300)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

noanflaherty added a commit that referenced this pull request Feb 24, 2026
#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>
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