Skip to content

fix #96586: [Bug]: voice-call get_status returns found:false for completed calls evicted from in-memory manager#96727

Closed
jincheng-xydt wants to merge 3 commits into
openclaw:mainfrom
jincheng-xydt:fix/voice-call-store-fallback-96586
Closed

fix #96586: [Bug]: voice-call get_status returns found:false for completed calls evicted from in-memory manager#96727
jincheng-xydt wants to merge 3 commits into
openclaw:mainfrom
jincheng-xydt:fix/voice-call-store-fallback-96586

Conversation

@jincheng-xydt

@jincheng-xydt jincheng-xydt commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: voice_call get_status only checks the in-memory CallManager.activeCalls map. Once a call completes and is evicted, get_status returns { found: false } even though the full call record with transcript is persisted to SQLite.
  • Solution: add getCallFromStore() and getCallByProviderCallIdFromStore() standalone lookup functions. Use them only in read-only status surfaces: voicecall.status gateway method, get_status tool, legacy status mode, and CLI status command. CallManager.getCall() and getCallByProviderCallId() remain active-only to protect live webhook/realtime/auto-response paths from stale terminal records.
  • Architecture (per ClawSweeper review): store fallback is scoped to all three status read paths — NOT added to shared manager lookup methods. This prevents completed/evicted records from leaking into webhook stream acceptance, transcript handling, realtime resolution, and auto-response guards.
  • Latest-snapshot semantics: readCallRecordEvents() returns oldest-first. The lookup functions iterate from the end to find the most recent snapshot when multiple persisted records exist for the same call.
  • What changed: manager/store.ts — two exported lookup functions. manager.ts — public callStorePath getter. index.ts — store fallback in voicecall.status gateway + get_status tool + legacy mode. cli.ts — store fallback in CLI status. manager.restore.test.ts — 7 tests.
  • What did NOT change: no schema, config, API, or provider changes. getCall()/getCallByProviderCallId() stay active-only.

Maintainer checklist

  • Fix classification: Root-cause bug fix in voice-call status read paths
  • Maintainer-ready confidence: High — all tests pass on upstream/main base

Real behavior proof

Behavior or issue addressed:
voice_call get_status returns { found: false } for completed calls once evicted from in-memory activeCalls map, despite full transcript persisted to SQLite. The fix scopes store fallback to status-only read paths.

Real environment tested:

  • Platform: Linux x86_64
  • Node.js: v22.22.0
  • Runtime: Vitest v4.1.8
  • Branch: fix/voice-call-store-fallback-96586 (2ca00be) — rebased on upstream/main

Exact steps or command run after the patch:

node ./node_modules/vitest/vitest.mjs run extensions/voice-call/src/manager.restore.test.ts extensions/voice-call/index.test.ts --reporter=verbose

After-fix evidence:
All tests pass including 7 new store-fallback tests:

  • keeps getCall active-only: completed/evicted calls are not found via manager ✓
  • keeps getCallByProviderCallId active-only: completed calls not found via manager ✓
  • returns undefined for calls not in activeCalls or store ✓
  • getCallFromStore returns completed call by callId ✓
  • getCallByProviderCallIdFromStore returns completed call by providerCallId ✓
  • getCallFromStore returns the latest (not oldest) snapshot ✓
  • getCallByProviderCallIdFromStore returns the latest snapshot ✓

Observed result after fix:

  • manager.getCall() / getCallByProviderCallId() — active-only ✓
  • getCallFromStore() / getCallByProviderCallIdFromStore() — standalone, latest-snapshot ✓
  • voicecall.status gateway method — store fallback for evicted calls ✓
  • get_status tool — store fallback ✓
  • Legacy status mode — store fallback ✓
  • CLI status command — store fallback ✓

What was not tested:

  • End-to-end with real Twilio/Telnyx provider
  • Gateway restart + get_status integration flow

@openclaw-barnacle openclaw-barnacle Bot added channel: voice-call Channel integration: voice-call size: S labels Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 7:27 AM ET / 11:27 UTC.

Summary
This PR adds persisted voice-call store lookup helpers, wires active-first fallback into gateway, tool, legacy, and CLI status reads, preserves active-only manager lookups, moves the redacted status projection, and adds restore/store tests.

PR surface: Source +92, Tests +199. Total +291 across 6 files.

Reproducibility: yes. Source inspection shows terminal calls are persisted and removed from active indexes, while current status paths only query active manager lookups before returning found:false; I did not run a live telephony provider flow.

Review metrics: 1 noteworthy metric.

  • Status Fallback Surfaces: 4 read paths changed. The gateway method, tool action, legacy status mode, and CLI fallback all need the same active-first persisted lookup and redacted projection invariant before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96586
Summary: This PR is a candidate fix for the canonical open voice-call status issue; one sibling PR targets the same root cause, one broader attempt was closed unmerged, and the merged redaction PR is adjacent but distinct.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted real behavior proof for a completed or restarted voice-call status lookup after the patch.
  • Keep the proof focused on gateway, tool, or CLI status and redact phone numbers, provider identifiers, endpoints, API keys, and non-public details.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body and comments provide Vitest/CI output only, not a real after-fix completed-call status run from a gateway, provider, CLI, logs, or redacted runtime output. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR changes how persisted terminal call state is surfaced after active manager eviction; it should stay limited to status reads so completed records do not re-enter webhook, realtime, or auto-response paths.
  • [P1] The submitted after-fix proof is Vitest and CI only, so maintainers still need redacted real gateway, provider, CLI, log, or runtime output showing completed or restarted status after the patch, or an explicit proof override.

Maintainer options:

  1. Require Redacted Status Proof (recommended)
    Ask for a real completed or restarted voice-call status run, with private phone numbers, provider ids, endpoints, and credentials redacted, before merge.
  2. Accept Source-Level Proof
    Maintainers can explicitly accept the source, unit-test, and CI evidence if live voice-provider proof is unavailable for this bounded bug fix.
  3. Choose A Canonical Fix PR
    If the sibling PR is preferred, pause this branch and close or supersede it only after a viable canonical fix is selected.

Next step before merge

  • [P1] No code repair is queued; the remaining action is contributor real behavior proof, maintainer proof override, or choosing the canonical sibling fix path.

Security
Cleared: No concrete security or supply-chain issue was found; the diff does not change dependencies, workflows, permissions, secrets, or external code execution, and status output remains redacted.

Review details

Best possible solution:

Land this or one chosen sibling fix only after redacted real behavior proof or maintainer override, keeping the persisted fallback status-only and preserving the redacted status DTO.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows terminal calls are persisted and removed from active indexes, while current status paths only query active manager lookups before returning found:false; I did not run a live telephony provider flow.

Is this the best way to solve the issue?

Yes, with a proof gap. A status-only persisted fallback is the narrowest maintainable fix because it repairs read surfaces without changing live manager, webhook, realtime, or auto-response lookup semantics.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 738b2be4b49b.

Label changes

Label changes:

  • add status: 🛠️ actively grinding: The PR author has acted after the latest ClawSweeper review and work remains. Needs real behavior proof before merge: The PR body and comments provide Vitest/CI output only, not a real after-fix completed-call status run from a gateway, provider, CLI, logs, or redacted runtime output. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • remove status: 📣 needs proof: Current PR status label is status: 🛠️ actively grinding.

Label justifications:

  • P2: This is a normal-priority voice-call bug fix with limited plugin scope, a clear linked issue, and no current evidence of a broader runtime outage.
  • merge-risk: 🚨 session-state: The PR intentionally exposes persisted completed call state after active manager eviction, which changes state visibility semantics for status consumers.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 🛠️ actively grinding: The PR author has acted after the latest ClawSweeper review and work remains. Needs real behavior proof before merge: The PR body and comments provide Vitest/CI output only, not a real after-fix completed-call status run from a gateway, provider, CLI, logs, or redacted runtime output. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +92, Tests +199. Total +291 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 5 135 43 +92
Tests 1 200 1 +199
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 335 44 +291

What I checked:

  • Repository policy applied: Read the full root policy and scoped extensions policy; fallback behavior, persisted state, plugin boundaries, and external-PR proof requirements affected the review. (AGENTS.md:31, 738b2be4b49b)
  • Scoped extensions policy applied: The touched files are under bundled plugins, where production code should stay within plugin-local or SDK boundaries; the PR uses voice-call-local modules and SDK imports only. (extensions/AGENTS.md:1, 738b2be4b49b)
  • Current main status miss: Current main's gateway status lookup checks only rt.manager.getCall(raw) or getCallByProviderCallId(raw) before returning found:false on a miss. (extensions/voice-call/index.ts:660, 738b2be4b49b)
  • Current main tool and legacy status miss: The voice_call get_status action and legacy status mode also use only active manager lookups before returning found:false. (extensions/voice-call/index.ts:797, 738b2be4b49b)
  • Terminal calls persist then leave active maps: finalizeCall persists the terminal call record, then deletes the call from activeCalls and removes the provider-call mapping, matching the linked persisted-but-not-found symptom. (extensions/voice-call/src/manager/lifecycle.ts:43, 738b2be4b49b)
  • Live paths still depend on active-only manager lookups: Realtime stream registration, transcript handling, and auto-response paths use manager getCall/getCallByProviderCallId, so keeping those methods active-only is the safer boundary. (extensions/voice-call/src/webhook.ts:385, 738b2be4b49b)

Likely related people:

  • steipete: Git history shows the initial voice-call plugin implementation and later SQLite/plugin-state and call-log migration work that this status fallback builds on. (role: feature-history owner; confidence: high; commits: 367baaca2092, e5c61383e53e, 3ff86f335020; files: extensions/voice-call/index.ts, extensions/voice-call/src/manager.ts, extensions/voice-call/src/manager/store.ts)
  • eleqtrizit: Merged PR history shows recent ownership of the read-scoped voice-call status redaction boundary that this PR preserves and moves into a shared projection. (role: recent adjacent contributor; confidence: high; commits: 825aafac577a; files: extensions/voice-call/index.ts, extensions/voice-call/src/types.ts)
  • garnetlyx: Git history shows work on voice-call restart hydration and stale persisted-call verification, adjacent to this active-versus-persisted status boundary. (role: adjacent restore-path contributor; confidence: medium; commits: ffa7c13c9b1c; files: extensions/voice-call/src/manager.ts, extensions/voice-call/src/manager.restore.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 25, 2026
@jincheng-xydt

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@jincheng-xydt

Copy link
Copy Markdown
Contributor Author

Real behavior proof (live test output)

Behavior or issue addressed:
voice_call get_status returns { found: false } for completed calls once evicted from in-memory activeCalls map.

Real environment tested:

  • Platform: Linux x86_64
  • Node.js: v22.22.0
  • Runtime: Vitest v4.1.8
  • Branch: fix/voice-call-store-fallback-96586 (7607ba2)

Exact steps or command run after the patch:

node scripts/run-vitest.mjs extensions/voice-call/src/manager.restore.test.ts --reporter=verbose

After-fix evidence:

 ✓ CallManager store fallback (#96586) > returns a completed call from the persisted store when evicted from activeCalls 79ms
 ✓ CallManager store fallback (#96586) > returns a completed call by providerCallId from the persisted store 88ms
 ✓ CallManager store fallback (#96586) > returns undefined for a call that is not in activeCalls or store 87ms
 ✓ CallManager store fallback (#96586) > getCallFromStore returns a completed call by callId 99ms
 ✓ CallManager store fallback (#96586) > getCallByProviderCallIdFromStore returns a completed call by providerCallId 76ms
 Test Files  1 passed (1)
      Tests  16 passed (16)
   Duration  21.68s

Observed result after fix:

  • manager.getCall(callId) returns completed call from store after eviction ✓
  • manager.getCallByProviderCallId(id) returns completed call from store ✓
  • Nonexistent call returns undefined (no false positive) ✓

@clawsweeper re-review

@jincheng-xydt

Copy link
Copy Markdown
Contributor Author

@clownfish automerge

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 27, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels Jun 30, 2026
@openclaw-barnacle openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 30, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label Jun 30, 2026
…law#96586)

Per ClawSweeper review feedback - store fallback for completed/evicted
calls must be applied in status-only read surfaces, not in shared
manager lookup methods used by live webhook/realtime/auto-response paths.

Changes from upstream/main:
- manager/store.ts: add getCallFromStore() and getCallByProviderCallIdFromStore()
  with latest-snapshot semantics (iterate from end, not .find())
- manager.ts: keep getCall()/getCallByProviderCallId() active-only;
  add public callStorePath getter for status path access
- index.ts: store fallback in voicecall.status gateway method,
  get_status tool, and legacy status mode
- cli.ts: store fallback in CLI status command
- manager.restore.test.ts: 7 tests covering active-only assertions,
  standalone store lookups, and latest-snapshot regression

Fixes: voice_call get_status returns {found:false} for completed calls
that have been evicted from in-memory activeCalls but are persisted
to SQLite.
@jincheng-xydt
jincheng-xydt force-pushed the fix/voice-call-store-fallback-96586 branch from cf5c721 to 2ca00be Compare June 30, 2026 07:24
…on constructor param

- Restore VoiceCallStatus type (safe Pick from CallRecord) and toVoiceCallStatus()
  projection in types.ts to prevent leaking phone numbers, transcripts,
  session keys, and internal metadata through status read paths.
- Restore coreSession constructor param in CallManager to fix TS2554
  compile error and preserve session inheritance for outbound calls.
- Apply toVoiceCallStatus() wrapper to all 4 status-response paths in
  index.ts (gateway, tool) and both CLI status paths in cli.ts.
- Store fallback logic (getCallFromStore, getCallByProviderCallIdFromStore)
  is preserved for openclaw#96586 completed/evicted call lookups.

ClawSweeper issues addressed:
  * Security boundary regression — DTO projection restored
  * Session state regression — coreSession constructor param restored

Co-Authored-By: Claude <[email protected]>
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jun 30, 2026
Fix 4 eslint(curly) warnings in getCallFromStore and
getCallByProviderCallIdFromStore.

Co-Authored-By: Claude <[email protected]>
@jincheng-xydt
jincheng-xydt force-pushed the fix/voice-call-store-fallback-96586 branch from b9381d6 to 54d8ef6 Compare June 30, 2026 08:49
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 30, 2026
@jincheng-xydt

Copy link
Copy Markdown
Contributor Author

Real behavior proof for #96727

I re-tested the voicecall status fallback from a source checkout at 54d8ef6c8a on branch fix/voice-call-store-fallback-96586.

What was verified

  • A completed call record was persisted into the voice-call plugin-state store.
  • The in-memory manager had no active call for that id.
  • voicecall status --call-id fell back to the persisted store after the local gateway was unavailable.
  • Lookup worked by both OpenClaw callId and provider providerCallId.
  • The CLI output went through toVoiceCallStatus: phone numbers, transcript, metadata, and processed event ids were not printed.

Runtime proof

Environment:

  • Platform: macOS
  • Node.js: v24.14.0
  • Provider: voice-call mock
  • Store: temporary plugin-state SQLite store under <tmp>
  • Gateway: configured with a local unused port so the status command exercised the local fallback path after ECONNREFUSED

Commands:

# Seed one persisted completed call:
# callId=call-proof-96727 providerCallId=CA-proof-96727 state=completed
node --import tsx <tmp>/seed-completed-call.mjs

OPENCLAW_HOME=<tmp> OPENCLAW_CONFIG_PATH=<tmp>/openclaw.json \
  node scripts/run-node.mjs voicecall status --call-id call-proof-96727 --json

OPENCLAW_HOME=<tmp> OPENCLAW_CONFIG_PATH=<tmp>/openclaw.json \
  node scripts/run-node.mjs voicecall status --call-id CA-proof-96727 --json

Observed output:

{
  "callId": "call-proof-96727",
  "providerCallId": "CA-proof-96727",
  "provider": "mock",
  "direction": "outbound",
  "state": "completed",
  "startedAt": 1782918000000,
  "answeredAt": 1782918005000,
  "endedAt": 1782918060000,
  "endReason": "completed"
}

The provider-call-id lookup returned the same status JSON:

{
  "callId": "call-proof-96727",
  "providerCallId": "CA-proof-96727",
  "provider": "mock",
  "direction": "outbound",
  "state": "completed",
  "startedAt": 1782918000000,
  "answeredAt": 1782918005000,
  "endedAt": 1782918060000,
  "endReason": "completed"
}

Tests

PATH=<node-bin>:<pnpm-bin>:$PATH \
  node scripts/run-vitest.mjs \
  extensions/voice-call/src/manager.restore.test.ts \
  extensions/voice-call/index.test.ts \
  --reporter=verbose

Result:

Test Files  2 passed (2)
Tests       57 passed (57)

I did not run a live Twilio/Telnyx/Plivo carrier call in this pass; this proof is scoped to the persisted-store status fallback and CLI-visible behavior.

@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 2, 2026
@jincheng-xydt

Copy link
Copy Markdown
Contributor Author

Closing this contributor PR for now because it remains below maintainer-ready quality (silver) and still needs stronger session-state proof before landing. I am pruning lower-confidence work so future PRs only proceed when they can reach merge-ready rating.

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

Labels

channel: voice-call Channel integration: voice-call merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant