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: blocked before merge. Reviewed September 4, 2026, 7:55 AM ET / 11:55 UTC.

ClawSweeper review

What this changes

The PR replaces the text-to-speech queue after each bridge reconnect, adds queue-stop signaling, and tests that an idle prior worker exits.

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep open: the PR fixes idle queue workers after reconnect, but it still cannot stop a worker already running system TTS, so the old goroutine and child process can remain active until speech returns.

Priority: P2
Reviewed head: e23643fed6e23b1c32aedaaf8d08057b034e78eb

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The idle-worker behavior has strong production-path evidence, but the patch leaves active system speech outside its shutdown mechanism.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The supplied terminal trace runs the changed clawgo reconnect path against a dropping local bridge and observes one idle TTS worker after two reconnects instead of three; it does not cover in-flight playback, which is the separate remaining correctness gap.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The supplied terminal trace runs the changed clawgo reconnect path against a dropping local bridge and observes one idle TTS worker after two reconnects instead of three; it does not cover in-flight playback, which is the separate remaining correctness gap.
Evidence reviewed	5 items	Introduced shutdown behavior: The PR replacement helper calls Stop on the previous queue but does not wait for it or provide a cancellation path to the active engine call. Active speech remains uncancelable: After a worker has selected text, it calls Speak synchronously; the system engine starts the configured command with exec.Command and waits with Run, neither of which observes the queue stop signal. Supplied behavior proof covers the idle case: The PR body shows a real dropping-bridge run reducing idle TTS worker stacks from three to one, but it deliberately uses /usr/bin/true and does not exercise a worker already inside system speech.
Findings	1 actionable finding	[P2] Cancel active speech before replacing its queue
Security	None	None.

How this fits together

Clawgo is a headless node that reconnects to an OpenClaw gateway bridge and can speak subscribed chat responses through a local system TTS command. Bridge reconnects create chat subscriptions and TTS workers, which then launch the local speech process.

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

Loading

Before merge

• Cancel active speech before replacing its queue (P2) - replaceTTSQueue only closes q.stop. If the prior worker has already selected text, it is blocked in synchronous Speak; the system engine uses uncancelable exec.Command(...).Run(). A reconnect starts the new queue immediately while the old worker and child can continue until the command returns. This remains the prior blocker; propagate queue cancellation into the active speech call and cover blocking playback.
• Resolve merge risk (P1) - A reconnect while the prior worker is already inside system speech only closes the queue signal; it does not stop the old worker or child process, so speech can continue and overlap the new connection until the command returns.
• Complete next step (P2) - Make active system TTS cancellable during TTSQueue.Stop and add blocking-playback regression coverage before merge.

Findings

• [P2] Cancel active speech before replacing its queue — cmd/clawgo/main.go:906-910

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Queue lifecycle change	production +54/-14, tests +28 across 2 files	The patch changes runtime worker ownership and adds one focused idle-worker regression test.

Merge-risk options

Maintainer options:

1. Cancel active speech before merge (recommended)
   Pass queue-owned cancellation into system speech and add blocking-playback coverage showing the prior worker and child exit on reconnect.
2. Accept completion-bound shutdown
   Merge the idle-worker cleanup while accepting that an existing local speech command may outlive a reconnect until it returns.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Make queue shutdown cancel in-flight system speech and add deterministic blocking-playback regression coverage.

Technical review

Best possible solution:

Make queue shutdown cancel in-flight system TTS as well as queued work, then prove a blocking speech command and its child exit on reconnect without changing normal TTS configuration.

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

Yes for the idle-worker defect: the supplied dropping-bridge trace demonstrates it, and current-main source retains the same queue construction. The remaining active-speech gap is directly established by the synchronous, uncancelable system TTS path.

Is this the best way to solve the issue?

No: closing the queue signal is sufficient only before a worker enters Speak; queue-owned cancellation must reach the active system speech process.

Full review comments:

• [P2] Cancel active speech before replacing its queue — cmd/clawgo/main.go:906-910
replaceTTSQueue only closes q.stop. If the prior worker has already selected text, it is blocked in synchronous Speak; the system engine uses uncancelable exec.Command(...).Run(). A reconnect starts the new queue immediately while the old worker and child can continue until the command returns. This remains the prior blocker; propagate queue cancellation into the active speech call and cover blocking playback.
  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: A reconnect can leave active local TTS work running, but the defect is bounded to the optional node speech path.
• merge-risk: 🚨 availability: The introduced replacement path cannot interrupt an in-flight speech process, so reconnect cleanup may leave a worker active.
• 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 supplied terminal trace runs the changed clawgo reconnect path against a dropping local bridge and observes one idle TTS worker after two reconnects instead of three; it does not cover in-flight playback, which is the separate remaining correctness gap.
• proof: sufficient: Contributor real behavior proof is sufficient. The supplied terminal trace runs the changed clawgo reconnect path against a dropping local bridge and observes one idle TTS worker after two reconnects instead of three; it does not cover in-flight playback, which is the separate remaining correctness gap.

Evidence

Acceptance criteria:

• [P2] go test ./cmd/clawgo -run 'TestReplaceTTSQueue|Test.*TTS' -count=1 -timeout 30s.
• [P2] go test ./cmd/clawgo -count=1 -timeout 60s.

What I checked:

• Introduced shutdown behavior: The PR replacement helper calls Stop on the previous queue but does not wait for it or provide a cancellation path to the active engine call. (cmd/clawgo/main.go:906, e23643fed6e2)
• Active speech remains uncancelable: After a worker has selected text, it calls Speak synchronously; the system engine starts the configured command with exec.Command and waits with Run, neither of which observes the queue stop signal. (cmd/clawgo/main.go:985, c6e46796a1c8)
• Supplied behavior proof covers the idle case: The PR body shows a real dropping-bridge run reducing idle TTS worker stacks from three to one, but it deliberately uses /usr/bin/true and does not exercise a worker already inside system speech. (e23643fed6e2)
• Current main does not already contain this fix: The fetched default-branch revision retains the pre-PR per-connection queue construction and removes this branch's stop signaling and regression test. (cmd/clawgo/main.go:403, c6e46796a1c8)
• Feature-history routing: The available main.go history traces the early node runtime to Mariano Belinky's f601408 commit; the raw commit record has no parent, so exact line-introduction provenance could not be reconstructed from the partial clone. (cmd/clawgo/main.go, 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 blocking-playback coverage and a redacted reconnect trace showing the prior worker and child process exit.

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 (11 earlier review cycles; latest 8 shown)

• 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
• reviewed 2026-09-02T18:09:01.292Z sha e23643f :: needs changes before merge. :: [P2] Cancel active TTS playback before replacing the queue
• reviewed 2026-09-03T21:50:56.223Z sha e23643f :: blocked before merge. :: [P2] Cancel active speech before replacing a queue