Skip to content

fix: stop treating TTS text as espeak flags - #11

Open
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/tts-leading-dash-argv
Open

SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/tts-leading-dash-argv

Conversation

@SebTardif

Copy link
Copy Markdown

What Problem This Solves

Default clawgo run (-chat-subscribe true, -tts-engine system) speaks chat final text through systemTTSEngine.Speak. That function built espeak-ng -v … -s … <text> and passed the utterance as the last argv with no -- and no stdin.

espeak-ng is [options] [words]. A reply that starts with - (a markdown list, -w/tmp/x, -fpath) is parsed as flags. Speak then fails, or the child writes or reads a file, instead of speaking the words.

This is separate from #10, which only adds a Speak deadline. That branch still appends the raw text.

Evidence

Live go run of the old argv shape versus the same shape with -- before the utterance. Parser is espeak-style: a leading -token is an option unless a prior -- ended option parsing, and -wPATH takes the rest of the token as the file operand.

$ go run C:\Users\sebta\AppData\Local\Temp\clawgo-f006-argv.go
without -- argv=["-v" "en-us" "-s" "180" "-w/tmp/x"] opts=map[s:180 v:en-us w:/tmp/x] words=[]
with --    argv=["-v" "en-us" "-s" "180" "--" "-w/tmp/x"] opts=map[s:180 v:en-us] words=["-w/tmp/x"]
md list old argv=["-v" "en-us" "-s" "180" "- item"] opts=map[ :item s:180 v:en-us] words=[]
md list new argv=["-v" "en-us" "-s" "180" "--" "- item"] opts=map[s:180 v:en-us] words=["- item"]

Without --, -w/tmp/x is option w with path /tmp/x and there are no words to speak. With --, -w/tmp/x stays in words.

The same argv is what production Speak now execs. A recorder child printed the slice after this patch:

argv=["-v" "en-us" "-s" "180" "--" "-w/tmp/x"]
argv=["-v" "en-us" "-s" "180" "--" "-foo"]
argv=["-v" "en-us" "-s" "180" "--" "- item"]
argv=["-v" "en-us" "-s" "180" "--" "hello"]
argv=["--" "-fpath"]

Real behavior proof

  • Behavior or issue addressed: Chat text that starts with - is spoken as words. It is no longer parsed as an espeak-ng option.
  • Real environment tested: Windows amd64, Go 1.27.0, branch fix/tts-leading-dash-argv at C:\Users\sebta\AppData\Local\Temp\oc-audit-clawgo.
  • Exact steps or command run after this patch: Ran go run C:\Users\sebta\AppData\Local\Temp\clawgo-f006-argv.go for the old versus new argv parse. Then invoked production systemTTSEngine.Speak with a recorder child so the process argv could be printed.
  • Evidence after fix: terminal output from the live go run helper above, plus the recorder argv slices. After the patch, -- sits immediately before the utterance. -w/tmp/x is in words, not option w.
  • Observed result after fix: espeak-style parse of the new argv keeps -w/tmp/x and - item as words. The old argv treats them as -w / a one-character flag and speaks nothing.
  • What was not tested: a live gateway chat stream, a real espeak-ng binary on this host, and custom -tts-system-command wrappers that reject POSIX --.

Summary

Call chain: chat final -> ChatSubscriber.speak -> TTSQueue.Speak -> systemTTSEngine.Speak -> exec.Command(espeak-ng, -v, …, -s, …, text).

Fix: insert -- before the utterance so the text cannot be parsed as flags. Least behavior change versus --stdin.

Introduced in f601408 (2026-01-04, 246 days). Still present after the c6e4679 rewrite in #8.

Related work:

  • #10 bounds Speak with CommandContext. It does not insert --.
  • #7 stops a leaked TTS queue on reconnect. Different bug.
  • espeak-ng usage is [options] [words]. -w <file> writes a WAV. POSIX -- ends options.
  • GNU getopt documents -- as the end of options, after which remaining argv are operands.

systemTTSEngine.Speak appended chat text as the last espeak-ng argument with no --. A reply that starts with - is parsed as an option, so Speak fails or writes or reads a file.

Insert -- before the utterance so espeak treats it as words.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper

clawsweeper Bot commented Sep 7, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 7, 2026, 5:01 PM ET / 21:01 UTC.

ClawSweeper review

What this changes

The PR separates spoken text from espeak command-line options with -- and adds five argument-recording regression cases.

Merge readiness

Blocked before merge - 3 items remain

The fix remains necessary on main and addresses a distinct problem from the linked timeout and reconnect PRs. No blocking code defect was found, but the supplied simulated parser and argument recorder do not establish real espeak-ng behavior.

Priority: P2
Reviewed head: 69771669d0fcd9bbd88d85a2bbac0fd53d683c5a

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The focused patch is sound by inspection, but surrogate-only proof leaves the runtime merge gate unmet.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: Needs real behavior proof before merge: production Speak was exercised with an argument recorder, while parsing was simulated; neither demonstrates real espeak-ng synthesis or prevention of text-triggered file operations. Provide redacted terminal output or a recording from the patched production path using real espeak-ng, including leading-dash and ordinary text. Redact private paths, endpoints, credentials, and other private information. Updating the PR body should trigger review; otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: Needs real behavior proof before merge: production Speak was exercised with an argument recorder, while parsing was simulated; neither demonstrates real espeak-ng synthesis or prevention of text-triggered file operations. Provide redacted terminal output or a recording from the patched production path using real espeak-ng, including leading-dash and ordinary text. Redact private paths, endpoints, credentials, and other private information. Updating the PR body should trigger review; otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Repository policy and identity: The origin identifies openclaw/clawgo. No AGENTS.md or maintainer-note files were found in the checkout.
Current main still passes text as options: The fetched main revision appends the trimmed utterance directly after voice and rate arguments. It has neither an option terminator nor stdin transport for text.
Complete introduced change: The verified base-to-head diff changes one production line and adds 103 test lines. All five cases use a generated argument recorder, which verifies argv construction but does not synthesize speech.
Findings None None.
Security None None.

How this fits together

Clawgo receives chat responses from the gateway and queues them for local speech. Its system TTS adapter launches espeak-ng with configured voice and rate settings followed by the response text.

flowchart LR
  A[Gateway chat response] --> B[Chat subscriber]
  B --> C[Speech queue]
  D[Voice and rate settings] --> E[System speech adapter]
  C --> E
  E --> F[Separate options from text]
  F --> G[espeak-ng audio output]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: Needs real behavior proof before merge: production Speak was exercised with an argument recorder, while parsing was simulated; neither demonstrates real espeak-ng synthesis or prevention of text-triggered file operations. Provide redacted terminal output or a recording from the patched production path using real espeak-ng, including leading-dash and ordinary text. Redact private paths, endpoints, credentials, and other private information. Updating the PR body should trigger review; otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - Existing custom -tts-system-command wrappers receive an additional argument; wrappers that reject or speak -- may stop working correctly after upgrade. Their compatibility is not demonstrated.
  • Complete next step (P2) - Add redacted after-fix proof using real espeak-ng and clarify and verify compatibility for existing custom speech commands.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +1/-1, tests +103/-0; 5 cases Production size is unchanged, and the added tests cover argument separation with and without voice and rate settings.

Merge-risk options

Maintainer options:

  1. Verify the custom-command contract (recommended)
    Document that custom commands must accept espeak-style arguments including --, and provide compatibility evidence for an existing wrapper alongside the default engine.

Technical review

Best possible solution:

Retain the option separator, demonstrate real espeak-ng synthesis, and document and verify the espeak-compatible argument contract for custom commands.

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

Yes, source establishes a concrete path: a leading-dash chat response reaches espeak-ng's option parser as an unseparated argument on main. No runtime reproduction or tests were executed during this read-only review.

Is this the best way to solve the issue?

Yes for the documented espeak-ng backend: ending option parsing is a narrow repair that preserves voice and rate settings. Custom-command compatibility still needs confirmation.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

  • add P2: This repairs a concrete local speech failure and unintended option interpretation with limited subsystem scope.
  • add merge-risk: 🚨 compatibility: The added separator also changes argv for user-selected speech wrappers whose acceptance of -- is unverified.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Needs real behavior proof before merge: production Speak was exercised with an argument recorder, while parsing was simulated; neither demonstrates real espeak-ng synthesis or prevention of text-triggered file operations. Provide redacted terminal output or a recording from the patched production path using real espeak-ng, including leading-dash and ordinary text. Redact private paths, endpoints, credentials, and other private information. Updating the PR body should trigger review; otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This repairs a concrete local speech failure and unintended option interpretation with limited subsystem scope.
  • merge-risk: 🚨 compatibility: The added separator also changes argv for user-selected speech wrappers whose acceptance of -- is unverified.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Needs real behavior proof before merge: production Speak was exercised with an argument recorder, while parsing was simulated; neither demonstrates real espeak-ng synthesis or prevention of text-triggered file operations. Provide redacted terminal output or a recording from the patched production path using real espeak-ng, including leading-dash and ordinary text. Redact private paths, endpoints, credentials, and other private information. Updating the PR body should trigger review; otherwise ask a maintainer for @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Repository policy and identity: The origin identifies openclaw/clawgo. No AGENTS.md or maintainer-note files were found in the checkout. (69771669d0fc)
  • Current main still passes text as options: The fetched main revision appends the trimmed utterance directly after voice and rate arguments. It has neither an option terminator nor stdin transport for text. (cmd/clawgo/main.go:940, c6e46796a1c8)
  • Complete introduced change: The verified base-to-head diff changes one production line and adds 103 test lines. All five cases use a generated argument recorder, which verifies argv construction but does not synthesize speech. (cmd/clawgo/tts_speak_args_test.go:13, 69771669d0fc)
  • Actual dependency and configurable command boundary: README identifies system speech as espeak-ng; the CLI also permits a custom executable through -tts-system-command. The new argument therefore reaches both the default engine and custom wrappers. (cmd/clawgo/main.go:178, 69771669d0fc)
  • Speech engine parser contract: The upstream implementation uses getopt_long, handles file input/output options, and passes argv[optind] to speech synthesis. This supports separating text from options; it is source evidence, not an after-fix runtime observation. (src/espeak-ng.c)
  • Captured contributor proof: The complete supplied body, captured under sourceRevision 1cf5e72c107be31f44e6717c3e2fc68a97c69724460d617679223128f1a551d9, describes Windows runs of an espeak-style parser and production Speak with a recorder child. It explicitly excludes a real espeak-ng binary and custom-wrapper compatibility. (69771669d0fc)

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.

  • Show patched production Speak invoking real espeak-ng, with leading-dash text synthesized and no unintended file operation.
  • Document and verify custom-command acceptance of the new option separator.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant