Skip to content

fix(voice-call): fall back to persisted call store on status misses#96624

Closed
palomyates516-alt wants to merge 1 commit into
openclaw:mainfrom
palomyates516-alt:fix/96586-voice-call-status-persisted-fallback
Closed

fix(voice-call): fall back to persisted call store on status misses#96624
palomyates516-alt wants to merge 1 commit into
openclaw:mainfrom
palomyates516-alt:fix/96586-voice-call-status-persisted-fallback

Conversation

@palomyates516-alt

@palomyates516-alt palomyates516-alt commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #96586.

What Problem This Solves

The voice_call tool's get_status action, the legacy mode: "status" path, and the voicecall.status gateway method only check the in-memory call manager. Once a call is evicted from the active map (completion cleanup, gateway restart, or time), they return { found: false } even though the full call record - including transcript - is still persisted in the plugin's SQLite store. Any agent workflow that "calls and reports back" silently fails at the reporting step.

Evidence

  • Reporter production data (issue [voice-call] get_status returns found:false for completed calls once evicted from in-memory manager — no fallback to persisted call store #96586): get_status returns { found: false } for completed calls; a manual SQLite query confirms the full persisted record (transcript + state) is present under plugin_state_entries (plugin_id=voice-call), keyed as event:<seq6>:<sequence>:<uuid>:chunk:NNNN.
  • Root cause verified against current main (a21144d8a6): index.ts get_status case, mode === "status", and the voicecall.status gateway method all use rt.manager.getCall(id) || rt.manager.getCallByProviderCallId(id) (in-memory only); CallManager.getCall reads the activeCalls map with no persisted fallback.
  • Unit tests (9 green in store.test.ts): getPersistedCallByCallId/getPersistedCallByProviderCallId find persisted records by id and return null on miss. Existing index.test.ts (38) and store.test.ts (5) unchanged.

Fix

  • store.ts: add getPersistedCallByCallId(storePath, callId) and getPersistedCallByProviderCallId(storePath, providerCallId) - bounded lookups reusing the existing readCallRecordEvents path.
  • manager.ts (CallManager): add getPersistedCallByCallId/getPersistedCallByProviderCallId methods that delegate to the store using the private storePath.
  • index.ts: add a findCallWithPersistedFallback(rt, id) helper (in-memory first, then persisted by callId, then by providerCallId) and use it in all three status surfaces.

Scope / non-goals

  • No new get_transcript action (the issue suggests it, but the ClawSweeper review flags it as "not needed to repair").
  • No change to the in-memory call manager, eviction policy, store schema, or recovery thresholds.
  • Read-only: status lookups only; no state mutation.

Test plan

  • New unit tests (store.test.ts): by-callId and by-providerCallId hit/miss.
  • Existing index.test.ts (38) and store.test.ts (5) green - no regression.
  • CI: pnpm tsgo + pnpm check (oxlint/oxfmt).
  • Live regression (maintainer env, needs a voice provider): complete a call, evict/restart, get_status returns found: true with the persisted record.

@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 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 12:14 AM ET / 04:14 UTC.

Summary
The PR adds persisted voice-call store lookup helpers and uses them as fallbacks for voicecall.status, the voice_call get_status action, and legacy status mode.

PR surface: Source +84, Tests +39. Total +123 across 4 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 Lookup Surfaces: 3 status paths changed. The gateway method, tool action, and legacy status mode all depend on the new persisted fallback semantics.

Stored data model
Persistent data-model change detected: serialized state: extensions/voice-call/src/manager/store.test.ts, serialized state: extensions/voice-call/src/manager/store.ts. Confirm migration or upgrade compatibility proof 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; sibling PRs target the same root cause but do not safely supersede this branch.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • Return the newest persisted snapshot for both call-id and provider-call-id lookups, with a multi-snapshot regression test.
  • [P1] Add redacted runtime proof or live output for a completed-and-evicted or restarted get_status flow.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body has unit-test output and before-fix production data, but no after-fix runtime or live output for a completed-and-evicted or restarted get_status flow; proof should be redacted and added to the PR body for re-review. 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 new persisted lookup can return an initial or otherwise stale snapshot for a completed call, causing found:true with outdated state instead of the final transcript-bearing record.
  • [P1] The PR body has unit-test output and before-fix reporter data, but no after-fix real completed-and-evicted or restarted get_status output from a real setup.

Maintainer options:

  1. Return The Latest Snapshot (recommended)
    Search persisted events newest-first or collapse records by call id/provider call id before returning status, then add a test with multiple snapshots for one call.
  2. Accept Partial Recovery
    Maintainers could intentionally land first-match behavior as a partial mitigation, but it may report stale call state instead of the terminal transcript-bearing record.
  3. Pause For Runtime Proof
    Keep the PR open but paused until the contributor adds real completed-call proof and the stale-snapshot behavior is fixed or explicitly accepted.

Next step before merge

  • [P1] Contributor or maintainer follow-up is needed for the newest-snapshot fix and external-PR real behavior proof; repair automation should not be queued while proof remains missing.

Security
Cleared: The diff only adds voice-call plugin lookup code and tests; no dependency, workflow, permission, secret-handling, or external code execution surface changed.

Review findings

  • [P2] Return the newest persisted snapshot for status lookups — extensions/voice-call/src/manager/store.ts:416
Review details

Best possible solution:

Keep the status-only persisted fallback, but return the newest canonical stored record by call id or provider call id, add a multi-snapshot regression test, and require redacted runtime proof for the completed-call status flow.

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?

No. The status-only fallback is the right layer, but the implementation must return the newest persisted snapshot rather than the first matching event.

Full review comments:

  • [P2] Return the newest persisted snapshot for status lookups — extensions/voice-call/src/manager/store.ts:416
    readCallRecordEvents() returns snapshots oldest-to-newest, and a call is persisted repeatedly as provider ids, transcript entries, and terminal state arrive. The new events.find(...) fallback can return an early no-transcript or nonterminal record for a completed evicted call; search newest-first or collapse by id/provider id and add a multi-snapshot regression.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 85ee71223f0d.

Label changes

Label justifications:

  • P2: This is a normal-priority voice-call bug fix with limited plugin scope, but the current branch still has a correctness blocker and proof gap.
  • merge-risk: 🚨 session-state: The persisted fallback can return stale call/session state for completed calls because the new lookup selects the first matching stored snapshot.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body has unit-test output and before-fix production data, but no after-fix runtime or live output for a completed-and-evicted or restarted get_status flow; proof should be redacted and added to the PR body for re-review. 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 +84, Tests +39. Total +123 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 88 4 +84
Tests 1 39 0 +39
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 127 4 +123

What I checked:

  • Repository policy applied: Root and scoped extension policy were read; fallback behavior and persisted session state are compatibility-sensitive review areas for bundled plugins. (AGENTS.md:31, 85ee71223f0d)
  • Current status paths are active-only: Current main still checks rt.manager.getCall(...) || rt.manager.getCallByProviderCallId(...) for voicecall.status and returns found:false on a miss. (extensions/voice-call/index.ts:660, 85ee71223f0d)
  • Manager lookup contract is active-only: Current manager methods read only active call maps, which explains why finalized or restarted calls disappear from status lookups without a status-specific persisted fallback. (extensions/voice-call/src/manager.ts:436, 85ee71223f0d)
  • Persisted history order is oldest-to-newest: readCallRecordEvents sorts by persisted time, sequence, and key in ascending order, so callers resolving one call id must choose the newest matching snapshot. (extensions/voice-call/src/manager/store.ts:284, 85ee71223f0d)
  • Calls are persisted repeatedly: Outbound initiation, provider id assignment, transcript/event processing, and finalization all persist later snapshots for the same call, including terminal transcript-bearing records. (extensions/voice-call/src/manager/outbound.ts:208, 85ee71223f0d)
  • PR head uses first-match lookup: At PR head, both new persisted lookup helpers call events.find(...), which selects the oldest matching snapshot from the ordered event list. (extensions/voice-call/src/manager/store.ts:416, d4d370f7838f)

Likely related people:

  • steipete: Git history shows substantial voice-call manager/status/store refactor and adjacent merge work around the active-versus-persisted boundary. (role: feature-history owner; confidence: high; commits: 89574f30cb00, 782247b42388, fe14be235261; files: extensions/voice-call/index.ts, extensions/voice-call/src/manager.ts, extensions/voice-call/src/manager/store.ts)
  • garnetlyx: Authored the merged restore verification PR that checks persisted calls before rehydrating active state, adjacent to this active-versus-persisted lookup boundary. (role: adjacent restore-path contributor; confidence: medium; commits: ffa7c13c9b1c, fe14be235261; files: extensions/voice-call/src/manager.ts, extensions/voice-call/src/manager.restore.test.ts)
  • eleqtrizit: Recently changed the same voice-call status response surface to use redacted status DTOs. (role: recent adjacent contributor; confidence: medium; commits: 825aafac577a; files: extensions/voice-call/index.ts, extensions/voice-call/index.test.ts)
  • Dallin Romney: Current blame for the active-only status, manager lookup, and store ordering lines points to a broad recent grafted-history commit, so this is a weak routing signal only. (role: current-line blame signal; confidence: low; commits: beab0ecb02dd; files: extensions/voice-call/index.ts, extensions/voice-call/src/manager.ts, extensions/voice-call/src/manager/store.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
@clawsweeper clawsweeper Bot mentioned this pull request Jun 25, 2026
Closed
@palomyates516-alt
palomyates516-alt force-pushed the fix/96586-voice-call-status-persisted-fallback branch from f1e9940 to 530c9c3 Compare June 26, 2026 03:18
@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 Jun 27, 2026
@palomyates516-alt
palomyates516-alt force-pushed the fix/96586-voice-call-status-persisted-fallback branch from 530c9c3 to d4d370f Compare June 30, 2026 03:59
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 30, 2026
@NiTeCoMM-code NiTeCoMM-code mentioned this pull request Jul 3, 2026
Closed
@Darren2030 Darren2030 mentioned this pull request Jul 4, 2026
Merged
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Thanks @palomyates516-alt. This is now superseded by #99797, landed on main as 39b5bf38.

The landed fix keeps completed-call lookup in the persisted-store owner path, covers the affected status surfaces with 70 focused passing tests, and passed a live authenticated Gateway regression that recovered the terminal hangup-bot state after provider completion. Closing this duplicate in favor of the landed implementation.

@steipete steipete closed this Jul 5, 2026
@palomyates516-alt
palomyates516-alt deleted the fix/96586-voice-call-status-persisted-fallback branch July 13, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[voice-call] get_status returns found:false for completed calls once evicted from in-memory manager — no fallback to persisted call store

2 participants

@palomyates516-alt @steipete