fix(android): keep stale PTT from restarting capture#100552
Conversation
381c93e to
08c95cf
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 1:50 AM ET / 05:50 UTC. Summary PR surface: Docs +1, Other +648. Total +649 across 6 files. Reproducibility: yes. from source inspection: gateway node.invoke preserves paramsJSON, Android dispatch forwards paramsJson, but the PR-head TalkHandler adapter ignores it and the changed stop/cancel handlers still call the no-arg active-capture methods. I did not run an Android device repro. Review metrics: none identified. 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 findings
Review detailsBest possible solution: Parse the optional Android PTT stop/cancel captureId from node.invoke paramsJson, pass it into the capture-scoped TalkModeManager methods, add a regression for stale requested capture IDs preserving a newer capture, and include real Android mic/background proof before merge. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: gateway node.invoke preserves paramsJSON, Android dispatch forwards paramsJson, but the PR-head TalkHandler adapter ignores it and the changed stop/cancel handlers still call the no-arg active-capture methods. I did not run an Android device repro. Is this the best way to solve the issue? No: the ownership refactor is in the right layer, but it is not yet the best complete fix because request-scoped release/cancel still does not reach the capture-scoped methods. A narrow paramsJson-to-captureId bridge plus regression coverage is the safer completion. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7e81f1596107. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Docs +1, Other +648. Total +649 across 6 files. View PR surface stats
Acceptance criteria:
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
Review history (3 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
removed
rating: 🐚 platinum hermit
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcf3e5a928
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| runTalkPttCommand { | ||
| val payload = talkMode.endPushToTalk() | ||
| finishTalkCaptureIfIdle() | ||
| val payload = stopPreparedTalkPttCapture { talkMode.endPushToTalk() } |
There was a problem hiding this comment.
When a delayed talk.ptt.stop/talk.ptt.cancel arrives after a newer PTT capture has started, this still calls the no-arg endPushToTalk()/cancelPushToTalk(), which resolves activePttCaptureId at execution time (TalkModeManager.kt:421-423) and stops the newer capture. The gateway already forwards paramsJson unchanged (src/gateway/node-invoke-sanitize.ts:7-28) and GatewaySession passes it to this handler, while this commit added capture-scoped stale-safe methods in TalkModeManager; parse the requested captureId from paramsJson and pass it through so stale stop/cancel requests return idle instead of tearing down the current session.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🐚 platinum hermit
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab816c4de9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val payload = | ||
| withPreparedTalkPttCommand(lifecycleEpoch, commandEpoch) { ownershipEpoch -> | ||
| val started = |
There was a problem hiding this comment.
When a foreground talk.ptt.start is just an idempotent retry for an already-active capture, this path still enters serialized preparation before TalkModeManager can return the existing capture. That preparation calls refreshConfig()/talk.config via GatewaySession.request with the 15s default timeout, and stop/cancel now acquire the same voiceCapturePreparationMutex, so if a retry is waiting on config, the user's release cannot stop or cancel the active recognizer until that request finishes. Short-circuit the existing activePushToTalkCaptureId retry before serialized preparation, or avoid sharing this lock with stop/cancel for retry-only work.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ea513089e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (start is TalkPttOnceStart.Started) { | ||
| finishTalkCaptureIfIdleAfterPreparation(start.captureId) |
There was a problem hiding this comment.
When a duplicate talk.ptt.once has already read commandEpoch at the start of handleTalkPttOnce and is delayed in prepareTalkCapture/talk.config, the original one-shot can auto-stop via silence or timeout before the duplicate reaches beginPushToTalkOnce; this cleanup only clears ownership and never advances talkPttCommandEpoch, so the stale duplicate still passes the epoch check and opens a second microphone capture. Advance the command epoch as part of terminal one-shot cleanup so only requests that begin after the previous capture ends can start a new capture.
Useful? React with 👍 / 👎.
|
Land-ready review complete at exact head What changed:
Proof:
The remaining broad CI failures do not touch this Android diff: they report current UI navigation type drift, Workboard hook-allowlist drift, and generated docs-map drift from the synthetic merge with current Proof gap: no physical Android microphone device was available. Both Android flavors and deterministic Robolectric lifecycle coverage exercise the changed ownership state machine. |
a5bcdd8 to
ea0b099
Compare
clawsweeper
Bot
added
rating: 🧂 unranked krab
81d24ba to
2dbd12f
Compare
clawsweeper
Bot
removed
rating: 🧂 unranked krab
|
Land-ready at exact head
Known proof gap: no physical-device microphone session was available. The changed lifecycle/ownership state machine is covered deterministically in Robolectric and both Android CI flavors. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Merged via squash.
|
* fix(android): serialize background PTT lifecycle Follow-up hardening for openclaw#99840 after the initial fix landed in openclaw#100483. Co-authored-by: xialonglee <[email protected]> * docs(changelog): note Android PTT lifecycle hardening * fix(android): order PTT ownership transitions * test(android): opt in to coroutine scheduler API * fix(android): serialize voice capture ownership * test(android): drive PTT ordering with test dispatcher * test(android): isolate PTT ordering preferences * test(android): control PTT invoke dispatchers * test(android): isolate PTT coroutine dispatchers * test(android): use current Robolectric resolver API * test(android): register speech service without deprecated APIs * test(android): control stale PTT cleanup dispatch * test(android): isolate voice mode reassertion * fix(android): clean voice capture on permission loss * fix(android): invalidate PTT before cancel wait * test(android): preserve starts queued after cancel * fix(android): keep one-shot PTT retries idempotent --------- Co-authored-by: xialonglee <[email protected]>
* fix(android): serialize background PTT lifecycle Follow-up hardening for openclaw#99840 after the initial fix landed in openclaw#100483. Co-authored-by: xialonglee <[email protected]> * docs(changelog): note Android PTT lifecycle hardening * fix(android): order PTT ownership transitions * test(android): opt in to coroutine scheduler API * fix(android): serialize voice capture ownership * test(android): drive PTT ordering with test dispatcher * test(android): isolate PTT ordering preferences * test(android): control PTT invoke dispatchers * test(android): isolate PTT coroutine dispatchers * test(android): use current Robolectric resolver API * test(android): register speech service without deprecated APIs * test(android): control stale PTT cleanup dispatch * test(android): isolate voice mode reassertion * fix(android): clean voice capture on permission loss * fix(android): invalidate PTT before cancel wait * test(android): preserve starts queued after cancel * fix(android): keep one-shot PTT retries idempotent --------- Co-authored-by: xialonglee <[email protected]>
1 participant
Related: #99840
Follow-up to #100483
What Problem This Solves
Fixes an issue where Android gateway push-to-talk could restart microphone capture after the app backgrounded, or let a stale stop/cancel tear down a newer Manual Mic or push-to-talk session when preparation, retries, and lifecycle changes overlapped.
Why This Change Was Made
The initial background-stop fix landed from #99840 in #100483, but its check occurred before suspended configuration work and its cleanup was not capture-scoped. This follow-up gives each capture explicit ownership, performs lifecycle-sensitive preparation on Main, serializes preparation and cleanup, and rejects stale work again at the recognizer-start boundary.
User Impact
Android no longer leaves Talk foreground-service state active after a cancelled, permission-revoked, or backgrounded gateway PTT request, and delayed work cannot replace or stop a newer microphone session. Cancellation invalidates preparation before waiting, while later starts retain their new command epoch. Idempotent one-shot retries return the active capture as busy without opening a second recognizer.
Evidence
2dbd12f4fd2e85d2c5cb5ed9c182c3e8ac4ad4aa, rebased onto89f911f322c90c3e7e3ec51120686256de19b4b1.run_8168a951e06epassed the completeTalkModeManagerTestandGatewayBootstrapAuthTestclasses on both Play and third-party Android flavors (65 Gradle tasks) for the identical pre-rebase patch.pnpm native:i18n:checkpassed with 2,583 entries and no drift;git diff --check origin/main...HEADpassed.git range-diffshowed the production patch unchanged apart from regenerated i18n line metadata.