fix: stop previous TTS queue on reconnect

[SebTardif]

What Problem This Solves

Default clawgo run (-chat-subscribe true, -tts-engine system) builds a TTS queue after every hello-ok. The queue loop ranges a channel that is never closed. A bridge drop reconnects, starts a second queue, and leaves the first goroutine ranging forever.

Each later drop adds another leaked loop. On a flaky Raspberry Pi or Linux link this grows without bound until clawgo run is killed.

This PR stops the previous queue before starting the next one after reconnect, and stops the last queue when run returns. Introduced in f601408 (2026-01-04) and present for 237 days. Related reconnect work: #5, #6.

Evidence

Before (binary from upstream/main at 5f1b9d9). Fake bridge accepts, sends hello-ok, then drops. Two drops, third session, then SIGQUIT. Three TTSQueue.loop goroutines still ranging:

$ /tmp/clawgo-f003-old run -bridge 127.0.0.1:62159 -state /tmp/state.json -mdns=false -tts-engine system -tts-system-command /usr/bin/true
connected to bridge 127.0.0.1:62159
hello ok (server=f003-bridge)
chat.subscribe sessionKey=main
bridge error: connection reset by peer
connected to bridge 127.0.0.1:62159
hello ok (server=f003-bridge)
chat.subscribe sessionKey=main
bridge error: connection reset by peer
connected to bridge 127.0.0.1:62159
hello ok (server=f003-bridge)
chat.subscribe sessionKey=main
SIGQUIT
main.(*TTSQueue).loop   created by main.newTTSQueue
main.(*TTSQueue).loop   created by main.newTTSQueue
main.(*TTSQueue).loop   created by main.newTTSQueue
TTSQueue.loop stacks=3

After (this patch). Same fake bridge and two drops. One loop remains (the current session):

$ /tmp/clawgo-f003 run -bridge 127.0.0.1:62414 -state /tmp/state.json -mdns=false -tts-engine system -tts-system-command /usr/bin/true
connected to bridge 127.0.0.1:62414
hello ok (server=f003-bridge)
chat.subscribe sessionKey=main
bridge error: connection reset by peer
connected to bridge 127.0.0.1:62414
hello ok (server=f003-bridge)
chat.subscribe sessionKey=main
bridge error: connection reset by peer
connected to bridge 127.0.0.1:62414
hello ok (server=f003-bridge)
chat.subscribe sessionKey=main
SIGQUIT
main.(*TTSQueue).loop   created by main.newTTSQueue
TTSQueue.loop stacks=1

Reconnect helper without Stop left the old loop ranging. With Stop it exited:

$ go test ./cmd/clawgo -run TestReplaceTTSQueueStopsPreviousLoop -count=1 -timeout 15s -v
=== RUN   TestReplaceTTSQueueStopsPreviousLoop
    tts_queue_test.go:25: previous TTS queue still ranging after reconnect
--- FAIL: TestReplaceTTSQueueStopsPreviousLoop (2.00s)
FAIL

$ go test ./cmd/clawgo -run TestReplaceTTSQueueStopsPreviousLoop -count=1 -timeout 15s -v
=== RUN   TestReplaceTTSQueueStopsPreviousLoop
--- PASS: TestReplaceTTSQueueStopsPreviousLoop (0.00s)
PASS
ok  	github.com/clawdbot/clawgo/cmd/clawgo	0.371s

Real behavior proof

• Behavior or issue addressed: Bridge reconnect no longer leaves the previous TTS queue goroutine ranging a never-closed channel.

• Real environment tested: macOS 26.6.2 (Darwin 25.6.0 arm64), go1.27.0, clawgo built from this branch at /tmp/oc-pr-clawgo-F003. Compared against a binary built from upstream/main (5f1b9d9).

• Exact steps or command run after this patch: Started a local TCP bridge that replies hello-ok and then drops the socket. Wrote a state file with a token so pairing is skipped. Ran clawgo run -bridge 127.0.0.1:$PORT -mdns=false -tts-engine system -tts-system-command /usr/bin/true. After three hello-ok sessions (two reconnects), sent SIGQUIT and counted main.(*TTSQueue).loop stacks. Also ran go test ./cmd/clawgo -run TestReplaceTTSQueueStopsPreviousLoop -count=1 -timeout 15s -v before and after calling Stop on the previous queue.

• Evidence after fix: terminal output from the patched binary:

  connected to bridge 127.0.0.1:62414
  hello ok (server=f003-bridge)
  chat.subscribe sessionKey=main
  SIGQUIT
  main.(*TTSQueue).loop   created by main.newTTSQueue
  TTSQueue.loop stacks=1

  The unfixed binary after the same two reconnects:

  connected to bridge 127.0.0.1:62159
  hello ok (server=f003-bridge)
  chat.subscribe sessionKey=main
  SIGQUIT
  main.(*TTSQueue).loop   created by main.newTTSQueue
  main.(*TTSQueue).loop   created by main.newTTSQueue
  main.(*TTSQueue).loop   created by main.newTTSQueue
  TTSQueue.loop stacks=3

• Observed result after fix: After two bridge drops, the patched clawgo run had one TTSQueue.loop goroutine. The unfixed binary had three, one per hello-ok.

• What was not tested: Live TTS against a real OpenClaw gateway and espeak-ng audio output. The leak is the ranging loop, not the speak binary. /usr/bin/true was used as -tts-system-command so LookPath succeeds on this host.

Command: live clawgo run against a dropping TCP bridge, then SIGQUIT. Also go test ./cmd/clawgo -run TestReplaceTTSQueueStopsPreviousLoop -count=1 -timeout 15s -v.

Observed: unfixed binary, 3 TTSQueue.loop stacks after two reconnects. Patched binary, 1 stack. Red step failed at 2.00s (previous TTS queue still ranging after reconnect). Green step passed at 0.00s.

Expected: each reconnect stops the previous queue so only the current session loop remains.

Time: 11:29:13 PDT

Date: 2026-08-29

Environment: macOS 26.6.2, Darwin 25.6.0 arm64, go1.27.0 darwin/arm64

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs changes before merge. Reviewed September 2, 2026, 2:09 PM ET / 18:09 UTC.

ClawSweeper review

What this changes

The PR replaces the local text-to-speech queue after each successful bridge reconnect and adds a test that an idle prior worker exits.

Merge readiness

⚠️ Needs maintainer review before merge - 3 items remain

Keep open: the patch stops idle prior queue loops but cannot stop speech already running during a reconnect. Likely related people: Mariano Belinky, feature-history contributor (medium confidence).

Priority: P2
Reviewed head: e23643fed6e23b1c32aedaaf8d08057b034e78eb

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The real reconnect proof is strong, but a concrete in-flight playback shutdown defect remains.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body provides a real local dropping-bridge run of the changed runNode reconnect path: after two reconnects, idle queue-loop stacks fall from three to one. That proof is sufficient for the claimed idle-loop repair but does not cover an active Speak call.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body provides a real local dropping-bridge run of the changed runNode reconnect path: after two reconnects, idle queue-loop stacks fall from three to one. That proof is sufficient for the claimed idle-loop repair but does not cover an active Speak call.
Evidence reviewed	5 items	Active playback cannot observe queue stop: The introduced replacement path calls Stop, which only closes the queue stop channel. A worker already inside Speak does not return to either stop-channel select until that call completes. System TTS process is uncancelable: The unchanged system engine invokes its configured child with exec.Command(...).Run() and has no cancellation path, so an active old worker can remain alive through reconnect or indefinitely if the child hangs. Existing test covers only an idle worker: The added test replaces a queue whose no-op engine has no queued or active speech, so it cannot establish shutdown of an in-flight playback call.
Findings	1 actionable finding	[P2] Cancel active TTS playback before replacing the queue
Security	None	None.

How this fits together

clawgo run reconnects to a gateway bridge, subscribes to chat, and sends final chat text through a serial local TTS queue. Reconnection replaces the chat session and must retire the previous queue and any active local speech work.

flowchart LR
  A[Gateway bridge] --> B[Reconnect loop]
  B --> C[Chat subscription]
  C --> D[TTS queue]
  D --> E[System speech process]
  B --> F[Replace prior queue]
  F --> D
  E --> G[Audio output]

Loading

Before merge

• Cancel active TTS playback before replacing the queue (P2) - Stop only closes q.stop. Once the worker has selected text and entered q.engine.Speak, it cannot observe that signal; the system engine uses uncancelable exec.Command(...).Run(), so a reconnect can retain the old worker and speech process until it completes or indefinitely if it hangs. This is the unchanged prior blocker.
• Resolve merge risk (P1) - On reconnect, an utterance already inside system Speak continues in the prior worker because queue stop does not interrupt the subprocess; stale playback and a retained worker can persist until the command exits.
• Complete next step (P2) - Cancel active TTS playback when a queue stops and add deterministic coverage for the in-flight process path.

Findings

• [P2] Cancel active TTS playback before replacing the queue — cmd/clawgo/main.go:906-909

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +54/-14, tests +28	The patch changes reconnect lifecycle and queue shutdown behavior while adding one focused idle-worker test.

Merge-risk options

Maintainer options:

1. Interrupt active playback before replacement (recommended)
   Propagate queue shutdown to the active system speech process and cover the in-flight path with a deterministic regression test.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Interrupt active system TTS playback when a queue stops; add a blocking-engine regression test and preserve reconnect queue replacement.

Technical review

Best possible solution:

Carry queue shutdown into the active system speech process, then demonstrate that reconnect terminates an in-flight prior worker before the replacement queue proceeds.

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

Yes: the supplied real reconnect trace targets the former default-branch revision, and current main has not changed the affected command source. Source confirms that an active Speak call cannot observe the new stop signal.

Is this the best way to solve the issue?

No: the patch repairs idle queue workers but not an active speech call; shutdown must reach the subprocess to fulfill the queue-replacement lifecycle.

Full review comments:

• [P2] Cancel active TTS playback before replacing the queue — cmd/clawgo/main.go:906-909
Stop only closes q.stop. Once the worker has selected text and entered q.engine.Speak, it cannot observe that signal; the system engine uses uncancelable exec.Command(...).Run(), so a reconnect can retain the old worker and speech process until it completes or indefinitely if it hangs. This is the unchanged prior blocker.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

• P2: An active prior TTS worker can survive reconnect and consume local process resources or continue stale playback.
• merge-risk: 🚨 availability: The introduced queue stop path does not interrupt an already-running system speech process.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body provides a real local dropping-bridge run of the changed runNode reconnect path: after two reconnects, idle queue-loop stacks fall from three to one. That proof is sufficient for the claimed idle-loop repair but does not cover an active Speak call.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a real local dropping-bridge run of the changed runNode reconnect path: after two reconnects, idle queue-loop stacks fall from three to one. That proof is sufficient for the claimed idle-loop repair but does not cover an active Speak call.

Evidence

Acceptance criteria:

• [P2] go test ./cmd/clawgo -run 'TestReplaceTTSQueueStopsPreviousLoop|Test.*TTS.*Stop' -count=1 -timeout 15s -v.

What I checked:

• Active playback cannot observe queue stop: The introduced replacement path calls Stop, which only closes the queue stop channel. A worker already inside Speak does not return to either stop-channel select until that call completes. (cmd/clawgo/main.go:908, e23643fed6e2)
• System TTS process is uncancelable: The unchanged system engine invokes its configured child with exec.Command(...).Run() and has no cancellation path, so an active old worker can remain alive through reconnect or indefinitely if the child hangs. (cmd/clawgo/main.go:985, e23643fed6e2)
• Existing test covers only an idle worker: The added test replaces a queue whose no-op engine has no queued or active speech, so it cannot establish shutdown of an in-flight playback call. (cmd/clawgo/tts_queue_test.go:17, e23643fed6e2)
• Current-main context: Current default-branch commit c6e4679 has base 5f1b9d9 and changes only workflow, README, and Go module files; it does not alter the affected command source. The proposed PR therefore remains necessary. (cmd/clawgo/main.go:985, c6e46796a1c8)
• Feature-history routing: Followed file history identifies Mariano Belinky as the dominant prior contributor to this command path, including commit f601408. This is routing context, not an attribution of the defect. (cmd/clawgo/main.go:878, f60140892c55)

Likely related people:

• Mariano Belinky: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Interrupt active system speech when queue shutdown occurs.
• Add deterministic coverage with a blocking playback engine and provide redacted after-fix reconnect output.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

History

Review history (9 earlier review cycles; latest 8 shown)

• reviewed 2026-08-30T01:59:02.426Z sha e23643f :: needs changes before merge. :: [P2] Cancel an in-flight TTS process when stopping a queue
• reviewed 2026-08-30T08:51:40.125Z sha e23643f :: needs changes before merge. :: [P2] Cancel in-flight TTS before replacing its queue
• reviewed 2026-08-30T16:06:55.995Z sha e23643f :: needs changes before merge. :: [P2] Cancel in-flight TTS before replacing its queue
• reviewed 2026-08-31T07:13:03.312Z sha e23643f :: needs real behavior proof before merge. :: [P2] Cancel active TTS playback before replacing the queue
• reviewed 2026-09-01T03:12:32.761Z sha e23643f :: needs real behavior proof before merge. :: [P2] Cancel active TTS playback before replacing the queue
• reviewed 2026-09-01T07:14:22.741Z sha e23643f :: needs changes before merge. :: [P2] Cancel active TTS playback before replacing the queue
• reviewed 2026-09-01T18:13:36.349Z sha e23643f :: needs changes before merge. :: [P2] Cancel active TTS playback before replacing the queue
• reviewed 2026-09-02T15:10:54.820Z sha e23643f :: needs changes before merge. :: [P2] Cancel active TTS playback before replacing the queue