fix(server): extract shared runtime into jobs_registry.py (P0: double-imported server.py) - #140
Merged
Merged
Conversation
… __main__/import double-execution Production runs server.py as __main__ (python server.py --http). http_api.py read shared state via lazy `from server import _bus`/`_jobs`/`_start_speech`/ etc. — since a module run as __main__ is never the same module object as one later imported by its own name, this silently re-executed server.py's entire top level a second time under the name "server": a second job registry, speech queue, ModalityBus, and _bargein_watcher thread with its own pipeline_state. Confirmed empirically via a real subprocess repro: the __main__ watcher never saw the live job's pipeline_state as speaking, so it misread the other instance's utterance as a foreign process and forcibly cleared the shared speaking-lock file out from under it. jobs_registry.py now holds all of it (job registry, speech queue, bus, pipeline_state, barge-in machinery, dashboard-chat relay, and the MCP tool definitions); server.py is a thin CLI entrypoint that imports from it, same as http_api.py. Neither ever imports from the other. GET /diagnostics gained a topology.server_reimported sentinel for direct observability going forward. Also fixes a related gap found while auditing this: a barge-in-interrupted job never reached a terminal state (pipeline_state.interrupt() silences audio but never told the generation loop to stop pulling chunks), so it sat at "speaking" until the engine finished synthesizing the rest of the unheard text. The generation loop now finalizes as "interrupted" with partial delivery metrics on either an in-process interrupt or lock loss; manual stop() routes through pipeline_state.interrupt() too.
Local ruff (0.11) didn't flag this; CI runs 0.15.22, which drops the now- redundant parens around a nested lambda.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Field incident
PR #139 (job retention + adaptive pre-playback buffer) was merged to
main(198910c) and deployed. Shortly after, a P0 surfaced in production and the deploy was rolled back toce42990(pinned, healthy). Observed in order:/v1/speakutterance began playing audibly.GET /v1/jobs/{id}poll hit the daemon."speaking"/ HUD"encoding"/ progress0.0for minutes, never terminal./diagnosticsshowedjobs: {total: 0}whileGET /v1/jobs/{id}did return the record.Root causes — confirmed vs refuted
Confirmed: server.py's
__main__/import duality double-executes the whole speech runtime. In production,python server.py --httpmakes server.py execute as__main__.http_api.pyread shared state via six separate lazyfrom server import ...calls (_bus,_jobs,_speech_queue/pipeline_state,_start_speech/check_bargein_gate,_dashboard_chat_broadcast,_dashboard_chat_register/_dashboard_chat_unregister) — some going back further than PR #139. Because a module run as__main__is never the same module object as one later imported by its own name, the very first such import silently re-executes server.py's entire top-level body a second time under the module name"server". I verified this empirically with a temporary instrumentation probe (module-level log line + thread id) launched via a real subprocess exactly matching production's invocation: two distinct executions, two distinct_bargein_watcherthread ids, same OS pid. Both instances (__main__and the phantom"server") construct their ownModalityBus, job registry,SpeechQueue,pipeline_state, and barge-in watcher thread.install_mcp_route()'smcpreference resolves through__main__'s own globals (it's defined in server.py itself), so MCP-tool calls (speak,diagnostics, ...) and HTTP calls (/v1/speak,/v1/jobs) could end up talking to different instances depending on which surface was used — exactly matching symptom (5):/diagnostics(MCP-tool-backed) read__main__'s empty registry while/v1/jobs/{id}(HTTP, using the lazily-imported "server") found the record.Refuted (or: unproven and out of scope) — the double-import causing the audible mid-utterance cutoff specifically. Re-examined against the daemon log (
~/workspaces/cog/.cog/run/services/mod3.log): both the mid-utterance cut on the new build (~13:47) and the identical cut on the rolled-back pre-merge build (13:54:11, jobd7105d13) show the same sequence —Barge-in: cross-process interrupt (pid=10048)→Barge-in from mic_vad: paused local playback (36%/52 of 156 chars delivered)→Speaking lock no longer ours — stopping generation. pid 10048 is the daemon's own process (its own mic_vad listener; SuperWhisper also running on the host) — this is a real, in-room voice triggering barge-in, not an artifact of the double-import. It reproduces on the rolled-back build too, so it predates this PR and isn't attributable to the import bug. Barge-in is behaving as designed here.Confirmed (new, found via the same log evidence): a barge-in-interrupted job never reaches a terminal state.
pipeline_state.interrupt()callsplayer.flush()directly — silences audio immediately — but nothing told_run_speech_job's generation loop to stop calling the TTS engine for more chunks. Its only prior stop signal was the file-based speaking-lock check, which an in-process interrupt never touches. The loop kept synthesizing the rest of the (unheard) text and the job stayed"speaking"/HUD"encoding"until that finished on its own — this is what actually produced symptom (4), on both builds, independent of the import bug. (The/diagnostics: total:0disagreement, symptom (5), also has a simpler standalone explanation I confirmed by reading the code directly: the HTTPGET /diagnosticsendpoint only ever readhttp_api.py's own_jobsledger, never the/v1/speakregistry at all — now fixed as part of the same-class merge below.)Fix
jobs_registry.py— the single shared runtime: job registry (_jobs,_prune_jobs),SpeechQueue/_start_speech/_run_speech_job, theModalityBus(_bus),pipeline_state, the barge-in watcher thread + registry + speaking-lock file helpers, the dashboard-chat relay,check_bargein_gate, and the@mcp.tool()-decorated MCP surface (speak,stop,speech_status,diagnostics,list_voices,set_output_device,register_session,output,mod3_dashboard_post, ...). This module is never run as__main__, so it is only ever imported once, under one name, regardless of who imports it first.server.pyis now a thin CLI entrypoint: argument parsing,_run_http,install_mcp_route,_start_inbound_pipeline_if_enabled,_prewarm_tts_if_enabled— importing everything else fromjobs_registry.py.http_api.pynow imports directly fromjobs_registry.pyeverywhere (no more lazyfrom server import ...anywhere in the file — audited and converted all six call sites).GET /diagnosticsnow reportstopology.server_reimported("server" in sys.modules) — should always beFalse; a direct, permanent observability hook for this exact regression class, plus its job counts now include the/v1/speakregistry (closing the standalone gap noted above).InterruptInfoits own interrupt callback already receives (in addition to the pre-existing lock-loss check); either path finalizes the job as"interrupted"(not"done") with partialspoken_pct/delivered_text/reasonplus the buffer/starvation fields from the adaptive-buffer feature./v1/stopand MCPstop()'s active-job branch now route throughpipeline_state.interrupt()too (falling back to a directplayer.flush()only if pipeline_state didn't think anything was speaking), so a manual stop finalizes the job record the same way a barge-in now does.MOD3_BARGEIN_SIGNAL_PATH/MOD3_SPEAKING_LOCK_PATH(default unchanged) let tests (and any isolated invocation) point the barge-in file/lock at a tmp path instead of the real daemon's files.New tests
tests/test_server_topology.py— launchesserver.py --httpas a real subprocess (the actual deployed topology; an in-process import never exhibits this bug because pytest always imports server.py under its own name). Starts a real speak job, pollsGET /v1/jobs/{id}mid-flight (the exact operator action from the incident), and asserts: the poll doesn't disturb the job, the job reaches a clean"done"(not killed), andGET /diagnosticsreportstopology.server_reimported == False. Gated on Apple Silicon /mlx+mcpavailability (same skip convention as the rest of the suite).tests/test_barge_in_finalization.py— an in-process interrupt mid-generation finalizes the job as"interrupted"with correct partial metrics and buffer fields (not stuck at"speaking"), the generation loop stops pulling chunks promptly rather than draining a 200-chunk fake utterance, the pre-existing lock-loss path finalizes identically, andstop()'s active-job branch routes throughpipeline_state.interrupt()(with a direct-flush fallback verified for the edge case where nothing was speaking per pipeline_state).server.Xfor names that moved tojobs_registry.X(SpeechQueue,_jobs,_run_speech_job,speak/stop/speech_status/diagnostics, the barge-in/speaking-lock helpers,_dashboard_chat_*, etc.) — audited everyfrom server import/import server/patch("server....")/monkeypatch.setattr(server, ...)in the test suite; only the ones for names that genuinely stayed in server.py (install_mcp_route,_start_inbound_pipeline_if_enabled) were left as-is.Test evidence
ruff check .,ruff format --check .clean.pyright --pythonversion 3.12 http_api.py server.py jobs_registry.pyclean (the one new pyright complaint the split introduced —mcp._mod3_route_installed = Truelooking like an unknown attribute oncemcparrives via cross-module import rather than local construction — fixed withsetattr).server.pyexecutions (distinct_bargein_watcherthread ids, same pid) on a bareserver.py --httpstartup, before any request ever arrived; after this fix,"server" not in sys.modulesthroughout.Deployment
Do NOT restart or redeploy anything —
~/worktrees/mod3-deploystays pinned atce42990per the coordinator's instruction; deploy/redeploy stays with the seat. This PR is ready to merge tomain; the fix takes effect on the next intentional daemon restart, same as the original PR.