fix #96586: [Bug]: voice-call get_status returns found:false for completed calls evicted from in-memory manager#96727
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 7:27 AM ET / 11:27 UTC. Summary 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.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +92, Tests +199. Total +291 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Real behavior proof (live test output)Behavior or issue addressed: Real environment tested:
Exact steps or command run after the patch: After-fix evidence: Observed result after fix:
@clawsweeper re-review |
|
@clownfish automerge |
…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.
cf5c721 to
2ca00be
Compare
…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]>
Fix 4 eslint(curly) warnings in getCallFromStore and getCallByProviderCallIdFromStore. Co-Authored-By: Claude <[email protected]>
b9381d6 to
54d8ef6
Compare
Real behavior proof for #96727I re-tested the What was verified
Runtime proofEnvironment:
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 --jsonObserved 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"
}TestsPATH=<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=verboseResult: 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. |
|
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. |
Summary
voice_call get_statusonly checks the in-memoryCallManager.activeCallsmap. Once a call completes and is evicted,get_statusreturns{ found: false }even though the full call record with transcript is persisted to SQLite.getCallFromStore()andgetCallByProviderCallIdFromStore()standalone lookup functions. Use them only in read-only status surfaces:voicecall.statusgateway method,get_statustool, legacystatusmode, and CLI status command.CallManager.getCall()andgetCallByProviderCallId()remain active-only to protect live webhook/realtime/auto-response paths from stale terminal records.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.manager/store.ts— two exported lookup functions.manager.ts— publiccallStorePathgetter.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.getCall()/getCallByProviderCallId()stay active-only.Maintainer checklist
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:
Exact steps or command run after the patch:
After-fix evidence:
All tests pass including 7 new store-fallback tests:
Observed result after fix:
What was not tested: