fix(agent-api): serialize voice_desired_state mutations under a lock by john-the-dev · Pull Request #1928 · sonichi/sutando

john-the-dev · 2026-07-05T04:03:37Z

Fixes #1922 (follow-up from the #1921 review).

What

/voice/toggle does a read-modify-write on the module-global voice_desired_state. Under a threaded server (#1921, already running as a hotfix on qingyun-mac-mini) two simultaneous toggles can interleave the read and the write and land on the wrong state. This adds one threading.Lock:

/voice/toggle — RMW wrapped in with voice_state_lock:
/voice/set — same lock, so a set can't slip inside a toggle
responses are sent outside the lock (no I/O while holding it)
bare reads (/voice/state at agent-api.py:321) stay lock-free — a str read/assignment is atomic under the GIL

Hunks are disjoint from #1921 (which touches lines ~284/~917) — the two merge cleanly in either order, and the lock is inert-but-harmless until #1921 lands.

Test (real module, revert-checked)

tests/agent-api-voice-state-lock.test.py importlib-loads the actual src/agent-api.py and serves api.Handler on a real ThreadingHTTPServer, then proves the handler acquires the lock: with voice_state_lock held by the test, a /voice/toggle and a /voice/set request each block, and complete with the correct state only after release. 7/7 pass; run against unfixed code it exits 1. The three existing agent-api suites stay green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F9E832S1QZiwRdujtdKPs7

Fixes sonichi#1922. /voice/toggle does a read-modify-write on the module-global voice_desired_state; under a threaded server (sonichi#1921) two simultaneous toggles can interleave and land on the wrong state. /voice/set is wrapped in the same lock so a set can't slip inside a toggle. Responses are sent outside the lock (no I/O while holding it). Bare reads stay lock-free — a str assignment is atomic under the GIL. Test imports the real module over a real ThreadingHTTPServer and proves the handler acquires the lock: with voice_state_lock held by the test, toggle and set requests block until release. Goes red (AttributeError / non-blocking) when the fix is reverted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F9E832S1QZiwRdujtdKPs7

qingyun-wu

LGTM. The lock scope is right: RMW serialized, send_json outside the lock (no I/O while holding it), bare /voice/state reads left lock-free — a str read is atomic under the GIL, and a stale read there is indistinguishable from arriving a moment earlier. /voice/set sharing the same lock closes the set-inside-toggle interleave too.

Test is the strong kind: it importlib-loads the real src/agent-api.py and proves the handler acquires voice_state_lock (blocked-while-held, completes-after-release) rather than just asserting end states — reverting the handler hunk goes red. Confirmed hunks are disjoint from #1921 and the lock is inert until that lands. CI green.

github-actions · 2026-07-07T07:13:34Z

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 47%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

src/agent-api.py (100%)

Summary

Total: 10 lines
Missing: 0 lines
Coverage: 100%

qingyun-wu · 2026-07-08T04:34:34Z

+REPO = Path(__file__).resolve().parent.parent
+
+# No token in env → check_auth() passes without Authorization headers.
+os.environ.pop("AGENT_API_TOKEN", None)


Automated review: tests/agent-api-voice-state-lock.test.py:33 clears AGENT_API_TOKEN, but src/agent-api.py reads SUTANDO_API_TOKEN. If a developer or CI environment already exports the real token, the test server will still require auth and the /voice/toggle / /voice/set requests in this test will fail with 401 for the wrong reason. Please clear SUTANDO_API_TOKEN here as well (or send the auth header explicitly) so the regression test stays hermetic.

Fixed in e5b2e7d — now clears both SUTANDO_API_TOKEN (the current name agent-api.py reads) and AGENT_API_TOKEN (legacy) so the test is hermetic regardless of which name a CI/dev env exports. Tests still pass.

qingyun-wu · 2026-07-08T14:10:43Z

Automated review:

tests/agent-api-voice-state-lock.test.py:32-33 clears AGENT_API_TOKEN, but src/agent-api.py reads SUTANDO_API_TOKEN. If that real env var is exported in the shell/CI, every unauthenticated /voice/* request in this test starts returning 401 and the suite fails for the wrong reason. The test needs to clear SUTANDO_API_TOKEN (or patch api.API_TOKEN) before starting the server.

qingyun-wu review: test cleared AGENT_API_TOKEN (an old name) but src/agent-api.py reads SUTANDO_API_TOKEN. A CI or dev environment that exports the real SUTANDO_API_TOKEN would silently put the test server into token-required mode, causing 401s for the wrong reason. Clear both names so the test is hermetic regardless of env. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

qingyun-wu · 2026-07-09T20:29:49Z

Automated mergeability review (2026-07-09)

Signal: NOT MERGEABLE

Current blocker(s):

Branch is behind current main; update/merge main and rerun checks before merging.

This is a conservative backlog triage signal. After the blockers are cleared, please rerun checks and request a focused review.

liususan091219

Triage change-request (owner-approved batch, 2026-07-12 sweep). Conditions found on the current diff:

live-test evidence required (voice/vision policy) — Voice runtime changed; body has no live repro, transcript, or concurrent request evidence.

Justification: kb triage standards (source-grep test standard, voice/vision live-test policy, necessity/coverage checks) — see sonichi/sutando-pr-triage kb/.
Address the asks and push; re-request review to clear this. If a finding is wrong, reply with the counter-evidence and it will be withdrawn.

cr-reason: live_test_required

liususan091219

Triage change-request (owner-approved batch, 2026-07-12 sweep). Conditions found on the current diff:

live-test evidence required (voice/vision policy) — Voice runtime changed; body has no live repro, transcript, or concurrent request evidence.

Justification: kb triage standards (source-grep test standard, voice/vision live-test policy, necessity/coverage checks) — see sonichi/sutando-pr-triage kb/.
Address the asks and push; re-request review to clear this. If a finding is wrong, reply with the counter-evidence and it will be withdrawn.

cr-reason: live_test_required

Dismissing exact duplicate of the same sweep review (retry double-post); the original stands.

liususan091219 · 2026-07-15T01:32:46Z

Before/after verification (origin/main 6388faf vs this head):

BEFORE (main): deterministic lost-update demo — injected slow interleaving on a real ThreadingHTTPServer, two concurrent /voice/toggle: final state connected where correct is disconnected → LOST UPDATE. Note current main really runs ThreadingHTTPServer (fix(agent-api): ThreadingHTTPServer + handler timeout so one stalled client can't wedge the API #1921), so this race is live in production paths, not theoretical.
AFTER (this head): all 7 lock checks pass exit=0; same interleaved repro → final disconnected, no lost update.

Verdict: fix is real, and this had the strongest evidence of the five (deterministic repro of the race both sides). BEHIND main, MERGEABLE.

— Lucy (Mac Studio bot)

github-actions · 2026-07-15T01:34:54Z

@cla-assistant check

john-the-dev mentioned this pull request Jul 5, 2026

fix(agent-api): ThreadingHTTPServer + handler timeout so one stalled client can't wedge the API #1921

Merged

qingyun-wu approved these changes Jul 5, 2026

View reviewed changes

john-the-dev mentioned this pull request Jul 7, 2026

fix(agent-api): guard voice_desired_state toggle with a Lock (closes #1922) #1975

Closed

2 tasks

Merge branch 'main' into fix/voice-state-lock

198a94b

qingyun-wu reviewed Jul 8, 2026

View reviewed changes

qingyun-wu mentioned this pull request Jul 8, 2026

fix(agent-api): make free-form pending questions answerable + unblock /answer #1855

Closed

6 tasks done

liususan091219 requested review from liususan091219 and qingyun-wu July 11, 2026 13:50

liususan091219 requested changes Jul 12, 2026

View reviewed changes

liususan091219 previously requested changes Jul 12, 2026

View reviewed changes

liususan091219 added cr:live-test-required request-change Has an active CHANGES_REQUESTED review voice Voice / vision / wake runtime live-test-requested A change-request asks for live-test/repro evidence (voice/vision/runtime policy) and removed cr:live-test-required labels Jul 12, 2026

liususan091219 added bug Something isn't working and removed bug Something isn't working labels Jul 13, 2026

john-the-dev added the ag2product AG2 product related (packages, PyPI, task-relay) label Jul 14, 2026

liususan091219 removed the ag2product AG2 product related (packages, PyPI, task-relay) label Jul 14, 2026

Merge branch 'main' into fix/voice-state-lock

cefd133

liususan091219 merged commit c09ccd2 into sonichi:main Jul 15, 2026
8 checks passed

Navigation Menu

Repository navigation

fix(agent-api): serialize voice_desired_state mutations under a lock - #1928

fix(agent-api): serialize voice_desired_state mutations under a lock#1928
liususan091219 merged 4 commits into
sonichi:mainfrom
john-the-dev:fix/voice-state-lock

john-the-dev commented Jul 5, 2026

Uh oh!

qingyun-wu left a comment

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

qingyun-wu Jul 8, 2026

Uh oh!

john-the-dev Jul 8, 2026

Uh oh!

qingyun-wu commented Jul 8, 2026

Uh oh!

qingyun-wu commented Jul 9, 2026

Uh oh!

liususan091219 left a comment

Uh oh!

liususan091219 left a comment

Uh oh!

liususan091219 commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

john-the-dev commented Jul 5, 2026

What

Test (real module, revert-checked)

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Gate

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

Summary

Uh oh!

qingyun-wu Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

john-the-dev Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

qingyun-wu commented Jul 8, 2026

Uh oh!

qingyun-wu commented Jul 9, 2026

Uh oh!

liususan091219 left a comment

Choose a reason for hiding this comment

Uh oh!

liususan091219 left a comment

Choose a reason for hiding this comment

Uh oh!

liususan091219 commented Jul 15, 2026

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 7, 2026 •

edited

Loading