Skip to content

fix(voice-call): emit canonical session keys#89884

Merged
steipete merged 48 commits into
openclaw:mainfrom
mushuiyu886:feat/issue-83967-voice-call-session-key
Jun 27, 2026
Merged

fix(voice-call): emit canonical session keys#89884
steipete merged 48 commits into
openclaw:mainfrom
mushuiyu886:feat/issue-83967-voice-call-session-key

Conversation

@mushuiyu886

@mushuiyu886 mushuiyu886 commented Jun 3, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Voice Call could write the same conversation under raw, differently cased, or differently scoped session identities. After a restart, that could split history or—during an unsafe upgrade rewrite—assign an ambiguous row to the wrong agent.

Fixes #83967.

Why This Change Was Made

  • Voice Call now emits one canonical agent:<agentId>:... identity for generated, explicit, and restored calls. Opaque provider-owned tails, including case-sensitive Matrix room IDs, remain byte-distinct.
  • Gateway startup and openclaw doctor --fix use the generic session migration owner. Proven single-owner rows migrate once; shared stores, filesystem aliases, unknown path identities, and ambiguous plugin/main aliases are preserved with an actionable warning before any write.
  • Message routing, send policy, cleanup, and the actual source-reply sender share strict structural parsing. Nested or malformed agent: identities stay opaque and cannot inherit channel delivery permissions.
  • Voice Call declares configured and default agent owners through the plugin doctor contract, so templated stores migrate even when the owner is not present in agents.list.

This keeps one runtime path: current canonical keys only. Upgrade compatibility remains in doctor/startup migration rather than fallback readers or dual writes.

User Impact

Voice Call conversations keep the same agent-owned session across restarts and upgrades. Existing unambiguous state is retained under the canonical key; uncertain state is left untouched for operator resolution instead of being merged or deleted. Outbound calls still use the top-level Voice Call route, while inbound per-number routes retain their configured agent/model/voice.

Evidence

  • Changed-surface regression suite: 669 tests passed across Voice Call, session migration, gateway startup, doctor, routing, send policy, and storage paths.
  • Final exact-tree accepted-review suite: 398 tests passed across Voice Call config, state migrations, session case preservation, message-tool inference, the real message-action sender, session-key routing, send policy, group resolution, and cleanup.
  • Plugin SDK contract/budget proof: 39 tests passed with no new public SDK surface.
  • Touched Oxlint, oxfmt, and git diff --check pass.
  • Broad Testbox check:changed: provider blacksmith-testbox, id tbx_01kw32d957z7kq3ej5kr7zfb2v, Actions run 28269876364.
  • Fresh Codex autoreview: clean, with no accepted/actionable findings (0.90 confidence).
  • Exact-head hosted CI: 66 checks passed with zero failures for ab707179f92ebf84dee3c5da16ba16cd522899f4 (Actions run 28272586915).

No live carrier call was required: the changed contract is session identity, persisted-state migration, and local delivery classification; provider transport code is unchanged.

Merge Risk

Session-state migration is compatibility-sensitive. The migration fails closed whenever ownership or filesystem identity is not proven, is idempotent, and has focused hard-link, symlink, shared-store, malformed-key, and partial-write regressions. No config shape, provider API, protocol version, or public Plugin SDK surface is added.

@openclaw-barnacle openclaw-barnacle Bot added channel: voice-call Channel integration: voice-call size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 3, 2026
@clawsweeper

clawsweeper Bot commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 26, 2026, 8:27 PM ET / 00:27 UTC.

Summary
The PR changes Voice Call session-key resolution, related docs/tests, plugin doctor session-store owner discovery, startup/doctor session migration, and shared session-route parsing.

PR surface: Source +915, Tests +2365, Docs +12. Total +3292 across 43 files.

Reproducibility: yes. Source-level checks show current main and v2026.6.10 still return raw generated voice:* / voice:call:* keys, while the PR head changes the resolver path; I did not run tests in this read-only review.

Review metrics: 3 noteworthy metrics.

  • Explicit session-key behavior: 1 pass-through path changed. Existing integrations that provide a custom Voice Call session key can be moved into a different persisted namespace on upgrade.
  • Plugin doctor contract surface: 1 resolver hook added. Session migration now asks plugin doctor contracts for extra store owners, so plugin API and upgrade behavior need maintainer-visible acceptance.
  • Session migration scope: 1 startup/doctor migration path expanded. The PR rewrites more legacy session keys during startup/doctor, which makes upgrade proof necessary beyond fresh-install tests.

Stored data model
Persistent data-model change detected: migration/backfill/repair: extensions/voice-call/doctor-contract-api.test.ts, migration/backfill/repair: extensions/voice-call/doctor-contract-api.ts, migration/backfill/repair: src/commands/doctor.e2e-harness.ts, migration/backfill/repair: src/flows/doctor-health-contributions.test.ts, migration/backfill/repair: src/flows/doctor-health-contributions.ts, migration/backfill/repair: src/gateway/server-startup-session-migration.test.ts, and 38 more. Migration or upgrade compatibility proof is recorded; maintainers should verify it before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #83967
Summary: This PR is the current candidate fix for the open canonical Voice Call generated-session-key issue; older similar PRs are closed unmerged, and the broader multi-agent routing issue only partially overlaps.

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: 🦪 silver shellfish
Result: blocked until stronger 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:

  • Update the PR proof against the exact current head, including explicit-key behavior and raw generated session migration.
  • Resolve the explicit-key contract by restoring pass-through or documenting/proving the maintainer-approved scoped-key migration.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes terminal-style local proof, but it appears stale for the current head because it claims explicit keys remain unchanged while current code/tests scope them; current-head proof for that behavior and migration boundary is still needed. 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] Existing integrations that pass explicit Voice Call session keys can be moved from caller-provided keys into agent-scoped keys, which can split stored history unless maintainers intentionally accept and prove that upgrade behavior.
  • [P1] The PR expands startup/doctor migration and plugin doctor contract behavior, so maintainers need upgrade proof for both templated stores and fixed/custom stores before merge.
  • [P1] The shared session delivery and send-policy parsing changes touch a privacy/security boundary; I found no concrete security flaw, but this deserves maintainer-visible review beyond green tests.
  • [P1] The supplied terminal proof is useful but stale or contradictory for the current head because it says explicit keys remain unchanged while the current code and tests scope them.

Maintainer options:

  1. Restore Explicit-Key Pass-Through (recommended)
    Keep this PR focused on generated voice:* keys by preserving caller-provided explicit keys, then update docs, tests, and proof to match.
  2. Sponsor Scoped Explicit Keys
    If maintainers want explicit Voice Call keys to become agent-scoped too, land that as an intentional compatibility change with migration coverage for existing explicit rows.
  3. Pause For A Narrow Replacement
    If the branch cannot be narrowed without reopening the compatibility decision, pause or replace it with a smaller generated-key-only PR that credits this work.

Next step before merge

  • [P1] Manual maintainer review is needed for the explicit-key compatibility decision and current-head real behavior proof; this is not a safe automated repair lane.

Security
Cleared: No concrete security or supply-chain flaw was found, but the diff touches send-policy and session-delivery routing boundaries that should stay in maintainer review.

Review findings

  • [P1] Preserve explicit Voice Call session keys — extensions/voice-call/src/config.ts:730-734
Review details

Best possible solution:

Land a narrow Voice Call fix that emits canonical keys for generated per-phone/per-call sessions, keeps explicit keys stable unless a separate scoped-key migration is explicitly sponsored, and includes current-head upgrade proof for raw generated rows.

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

Yes. Source-level checks show current main and v2026.6.10 still return raw generated voice:* / voice:call:* keys, while the PR head changes the resolver path; I did not run tests in this read-only review.

Is this the best way to solve the issue?

No. Canonicalizing generated keys at the resolver boundary is the right fix, but scoping explicit keys is a compatibility-sensitive contract change that should either be removed from this PR or separately approved and proven.

Full review comments:

  • [P1] Preserve explicit Voice Call session keys — extensions/voice-call/src/config.ts:730-734
    resolveVoiceCallSessionKey now sends every explicit key through resolveVoiceCallAgentSessionKey, so existing callers that pass a shared key like meet-room-1 start using agent:<agentId>:meet-room-1 instead. The PR body still says explicit overrides are unchanged; restore pass-through here or make the scoped-key migration an explicit maintainer-approved compatibility change with matching proof.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.83

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 12ea61a08dbc.

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: Explicit session keys, plugin doctor contract shape, and migration behavior change in ways that can affect existing setups during upgrade.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • remove mantis: telegram-visible-proof: Current Telegram visible-proof status is not_needed.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.
  • remove merge-risk: 🚨 automation: Current PR review merge-risk labels are merge-risk: 🚨 compatibility, merge-risk: 🚨 session-state, merge-risk: 🚨 security-boundary.

Label justifications:

  • P1: The PR addresses a high-impact Voice Call session-continuity bug and changes upgrade-sensitive persisted session behavior.
  • merge-risk: 🚨 compatibility: Explicit session keys, plugin doctor contract shape, and migration behavior change in ways that can affect existing setups during upgrade.
  • merge-risk: 🚨 session-state: The PR changes persisted Voice Call session identity and orphan-key migration behavior.
  • merge-risk: 🚨 security-boundary: Shared send-policy and session-delivery parsing changes affect whether replies stay on the private source sink or route to external channels.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal-style local proof, but it appears stale for the current head because it claims explicit keys remain unchanged while current code/tests scope them; current-head proof for that behavior and migration boundary is still needed. 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 +915, Tests +2365, Docs +12. Total +3292 across 43 files.

View PR surface stats
Area Files Added Removed Net
Source 22 1140 225 +915
Tests 20 2420 55 +2365
Docs 1 12 0 +12
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 43 3572 280 +3292

What I checked:

Likely related people:

  • steipete: Recent path history connects this person to Voice Call per-call session scope, per-number routing, plugin SDK session helpers, and state/session refactors that overlap the PR surface. (role: feature owner / adjacent owner; confidence: high; commits: e4aab1419ab8, 39a931f1bfe0, cabb55380f84; files: extensions/voice-call/src/config.ts, extensions/voice-call/src/response-generator.ts, src/infra/state-migrations.ts)
  • vincentkoc: Recent GitHub path history shows focused Voice Call config refactors and multiple state/doctor migration changes near the upgrade path touched by this PR. (role: recent area contributor; confidence: high; commits: 905c9759a716, fb06df6cadf1, 6daabd23f821; files: extensions/voice-call/src/config.ts, src/infra/state-migrations.ts)
  • jalehman: Recent response-generator history includes routing Voice Call agent runs through session targets, adjacent to this PR's session-key path. (role: recent adjacent contributor; confidence: medium; commits: d716900929db; files: extensions/voice-call/src/response-generator.ts)
  • lukeboyett: Recent session-key utility history added Matrix case-preservation behavior that this PR extends through nested and ambiguous session-key parsing. (role: adjacent session-key owner; confidence: medium; commits: b5bd6e882885; files: src/sessions/session-key-utils.ts, src/sessions/session-chat-type-shared.ts)
  • mushuiyu886: Beyond authoring this PR, this person appears in recent merged state-migration history for a plugin-owned migration path that overlaps the upgrade-sensitive surface touched here. (role: recent migration contributor; confidence: medium; commits: 414c250af90d; files: src/infra/state-migrations.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.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 3, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 3, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 3, 2026
@mushuiyu886

mushuiyu886 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 3, 2026 •
edited
Loading

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.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 3, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 15, 2026
Closed
@mushuiyu886
mushuiyu886 force-pushed the feat/issue-83967-voice-call-session-key branch from 7c89479 to 17c2172 Compare June 18, 2026 05:27
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@mushuiyu886
mushuiyu886 force-pushed the feat/issue-83967-voice-call-session-key branch from 17c2172 to 9f87e67 Compare June 25, 2026 02:28
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 25, 2026
@steipete steipete self-assigned this Jun 26, 2026
@openclaw-barnacle openclaw-barnacle Bot added the docs Improvements or additions to documentation label Jun 26, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Closed
@steipete
steipete requested a review from a team as a code owner June 26, 2026 23:12
steipete added 20 commits June 27, 2026 01:19
@steipete
fix(sessions): close legacy policy gaps
8dda311
@steipete
fix(sessions): preserve aliases across migrations
3a65edf
@steipete
fix(sessions): resolve first-run store ownership
5449b6c
@steipete
fix(sessions): preserve hostile legacy keys
17cae00
@steipete
fix(sessions): inspect unlisted store owners
653913c
@steipete
test(doctor): refresh migration harness
5aacac2
@steipete
fix(sessions): preserve opaque route segments
df66ac1
@steipete
fix(sessions): retain metadata during migration
8a63408
@steipete
fix(sessions): fail closed on store alias uncertainty
a96d778
@steipete
fix(sessions): defer aliased store rewrites
7e65560
@steipete
fix(sessions): retain legacy row owners
a169318
@steipete
test(sessions): harden migration proof
8e998e5
@steipete
fix(sessions): migrate opaque agent keys
c10b1c2
@steipete
chore(plugin-sdk): refresh API baseline
fea121d
@steipete
fix(voice-call): reuse public routing parser
35e6d05
@steipete
fix(sessions): retain readable alias warnings
a4abcfa
@steipete
fix(sessions): reject opaque nested routes
b0ee0ca
@steipete
fix(sessions): share strict delivery parsing
4f6d35f
@steipete
test(voice-call): preserve malformed Matrix case
3223b3e
@steipete
fix(sessions): reject legacy peer overlap
ab70717
@steipete
steipete force-pushed the feat/issue-83967-voice-call-session-key branch from 0de8563 to ab70717 Compare June 27, 2026 00:19
@openclaw-barnacle openclaw-barnacle Bot removed channel: discord Channel integration: discord channel: imessage Channel integration: imessage channel: signal Channel integration: signal labels Jun 27, 2026
@steipete

steipete commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Land-ready verification complete for ab707179f92ebf84dee3c5da16ba16cd522899f4.

  • Voice Call now emits one configured-agent canonical identity for generated, explicit, and restored calls while preserving opaque case-sensitive peer tails.
  • Startup and doctor migrate only proven single-owner state. Shared stores, filesystem aliases, unknown path identities, and ambiguous plugin/main rows remain untouched with an actionable warning before any write.
  • Routing, send policy, cleanup, inference, and the actual sender share strict structural parsing; nested, malformed, and legacy/canonical-overlap identities fail closed.
  • Changed-surface regression proof: 669 tests passed across nine shards. Final accepted-review suite: 398 tests passed across five lanes. Plugin SDK contract/budget proof: 39 tests passed with no new public surface.
  • Touched Oxlint, oxfmt, and git diff --check passed. Broad Testbox check:changed: blacksmith-testbox lease tbx_01kw32d957z7kq3ej5kr7zfb2v, Actions run 28269876364.
  • Fresh Codex autoreview: clean, no accepted/actionable findings (0.90 confidence).
  • Repository exact-head hosted CI: 66 checks passed with zero failures (Actions run 28272586915).

No known proof gaps.

@steipete

steipete commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@Haderach-Ram Haderach-Ram mentioned this pull request Jun 27, 2026
Open
@github-actions github-actions Bot mentioned this pull request Jun 27, 2026
Open
This was referenced Jun 27, 2026
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

agents Agent runtime and tooling channel: voice-call Channel integration: voice-call commands Command implementations docs Improvements or additions to documentation gateway Gateway runtime merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XL 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.

Follow-up: voice-call canonical session-key fix for #80064 is still not on main

2 participants

@mushuiyu886 @steipete