fix(dictation): redact API keys from streaming transcriber errors

[euxaristia]

Summary

Port of the Sentinel security finding (originally 🛡️ Sentinel: [CRITICAL] Fix API key leak in streaming dictation) from the fork into upstream.

• Streaming dictation (Deepgram, OpenAI Realtime) injected API keys into websocket URLs/headers and echoed unredacted transport errors and server-side error payloads back to callers/logs.
• Wrap those error endpoints with providerio.Redact and switch the connect-error format specifier from %w to %s so the raw error cannot leak via errors.Unwrap().

Test plan

• Added regression tests TestOpenAIRealtimeStreamTranscribeErrorRedaction and TestDeepgramStreamTranscribeErrorRedaction asserting the API key is not present in the returned stream error for both transcribers.
• go test ./internal/dictation/... passes; go vet ./internal/dictation/... clean.

Notes

• The original finding also added a .jules/sentinel.md note; that's a Jules-specific artifact and was intentionally omitted from this upstream PR.

Summary by CodeRabbit

• Bug Fixes
  • Prevented configured Deepgram and OpenAI Realtime API keys from appearing in transcription-related error messages, including connection, streaming, and protocol/event failures.
  • Improved cancellation handling so cancellation errors preserve the original cancellation signal and won’t trigger transcript insertion or auto-submit.
• Tests
  • Added Deepgram tests covering API-key redaction and multiple cancellation/race scenarios for streaming transcription.
  • Added OpenAI Realtime tests covering API-key redaction and cancellation/race scenarios, including cancel winning over concurrent error events.
  • Added a dictation UI test to ensure canceled stream races do not auto-submit.

[coderabbitai]

Walkthrough

Deepgram and OpenAI Realtime transcribers now redact configured API keys from provider errors while preserving context.Canceled. The TUI treats canceled transcription messages as terminal no-ops, preventing raced events from triggering submission.

Changes

Transcriber cancellation and error safety

Layer / File(s)	Summary
Redact provider error paths internal/dictation/transcriber_deepgram.go, internal/dictation/transcriber_openai_realtime.go	Provider connection, stream, session, protocol, and writer errors use providerio.Redact; canceled operations return ctx.Err(), including cancellation races after callbacks.
Verify provider behavior internal/dictation/*transcriber*_test.go	WebSocket tests cover API-key redaction and cancellation during dialing, startup, streaming, writer blocking, and racing provider errors.
Stop canceled TUI results internal/tui/dictation.go, internal/tui/dictation_test.go	Canceled transcription messages return before submit handling, with a regression test for buffered cancellation events.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• Gitlawb/zero#557: Introduced the streaming transcriber implementations and TUI dictation behavior updated here.

Suggested reviewers: vasanthdev2004, anandh8x, kevincodex1

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: redacting API keys from streaming transcriber errors.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

internal/dictation/transcriber_openai_realtime.go (2)

156-161: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle the write error instead of discarding it.

If a non-nil werr is pulled from writeErrCh here, it is silently discarded. While the subsequent conn.Read will likely fail and return an error anyway, the original write error context is lost. Return the redacted write error directly to correctly report the failure.

🐛 Proposed fix

 		select {
 		case werr := <-writeErrCh:
 			if werr == nil {
 				committed = true
+			} else {
+				return compose(), fmt.Errorf("OpenAI Realtime stream error: %s", providerio.Redact(werr.Error(), o.cfg.APIKey))
 			}
 		default:
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/dictation/transcriber_openai_realtime.go` around lines 156 - 161,
Update the writeErrCh handling in the surrounding transcription flow so a
non-nil werr is redacted and returned immediately, preserving the original write
failure context; retain the committed = true behavior when werr is nil and the
existing default path.

---

77-79: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Redact API key in session configuration error.

If the server immediately closes the connection with an error message (such as an invalid key rejection) before or during the writeJSON call, the returned WebSocket close error will contain the server's reason. Since this error is currently wrapped with %w rather than redacted, the API key could leak here. Apply providerio.Redact to ensure it is sanitized.

🔒️ Proposed fix

-	if err := writeJSON(ctx, conn, sessionUpdate); err != nil {
-		return "", fmt.Errorf("configuring OpenAI Realtime session: %w", err)
+	if err := writeJSON(ctx, conn, sessionUpdate); err != nil {
+		return "", fmt.Errorf("configuring OpenAI Realtime session: %s", providerio.Redact(err.Error(), o.cfg.APIKey))
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/dictation/transcriber_openai_realtime.go` around lines 77 - 79,
Update the writeJSON error handling in the session configuration flow to apply
providerio.Redact to the error before wrapping and returning it. Preserve the
existing “configuring OpenAI Realtime session” context while ensuring server
close reasons cannot expose the API key.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/dictation/transcriber_stream_test.go`:
- Around line 233-286: Move TestOpenAIRealtimeStreamTranscribeErrorRedaction
into transcriber_openai_realtime_test.go and move
TestDeepgramStreamTranscribeErrorRedaction into transcriber_deepgram_test.go.
Remove both test functions from transcriber_stream_test.go without changing
their assertions or behavior.

---

Outside diff comments:
In `@internal/dictation/transcriber_openai_realtime.go`:
- Around line 156-161: Update the writeErrCh handling in the surrounding
transcription flow so a non-nil werr is redacted and returned immediately,
preserving the original write failure context; retain the committed = true
behavior when werr is nil and the existing default path.
- Around line 77-79: Update the writeJSON error handling in the session
configuration flow to apply providerio.Redact to the error before wrapping and
returning it. Preserve the existing “configuring OpenAI Realtime session”
context while ensuring server close reasons cannot expose the API key.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 921826ef-803f-4f87-90a7-f4c1744d5429

📥 Commits

Reviewing files that changed from the base of the PR and between 9acb411 and a807aba.

📒 Files selected for processing (3)

• internal/dictation/transcriber_deepgram.go
• internal/dictation/transcriber_openai_realtime.go
• internal/dictation/transcriber_stream_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/dictation/transcriber_deepgram_test.go`:
- Around line 12-25: Update the wsTestServer callback in the Deepgram test to
wait for the client’s expected messages or otherwise synchronize on CloseStream
before closing the connection. Ensure StreamTranscribe observes the
API-key-bearing policy-violation close reason instead of failing earlier on a
write error, while preserving the existing invalid-key scenario.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2948fe8f-3e13-4fe3-9e34-e309792ae886

📥 Commits

Reviewing files that changed from the base of the PR and between a807aba and 5b9b294.

📒 Files selected for processing (3)

• internal/dictation/transcriber_deepgram_test.go
• internal/dictation/transcriber_openai_realtime.go
• internal/dictation/transcriber_openai_realtime_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/dictation/transcriber_openai_realtime.go

[coderabbitai]

🧹 Nitpick comments (1)

internal/dictation/transcriber_openai_realtime.go (1)

63-63: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Cover every redaction path with regression tests.

The supplied test exercises only a server realtimeError event. Please add or verify focused tests for connection failures, initial session-write failures, WebSocket read failures, and writer/commit failures so future changes cannot reintroduce API-key leakage in the other newly sanitized paths.

Also applies to: 78-78, 117-117, 151-160

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/dictation/transcriber_openai_realtime.go` at line 63, Expand the
tests for the OpenAI Realtime transcriber to cover API-key redaction in
connection failures, initial session-write failures, WebSocket read failures,
and writer/commit failures, alongside the existing realtimeError event case.
Exercise each error path and assert the configured API key never appears in the
returned or logged error while preserving the relevant failure behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/dictation/transcriber_openai_realtime.go`:
- Line 63: Expand the tests for the OpenAI Realtime transcriber to cover API-key
redaction in connection failures, initial session-write failures, WebSocket read
failures, and writer/commit failures, alongside the existing realtimeError event
case. Exercise each error path and assert the configured API key never appears
in the returned or logged error while preserving the relevant failure behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 025a5a81-3924-4707-9e18-44a20686732b

📥 Commits

Reviewing files that changed from the base of the PR and between 5b9b294 and b024094.

📒 Files selected for processing (4)

• internal/dictation/transcriber_deepgram.go
• internal/dictation/transcriber_deepgram_test.go
• internal/dictation/transcriber_openai_realtime.go
• internal/dictation/transcriber_openai_realtime_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/dictation/transcriber_openai_realtime_test.go
• internal/dictation/transcriber_deepgram_test.go
• internal/dictation/transcriber_deepgram.go

[Vasanthdev2004]

Requesting changes — the redaction itself is correct and well-scoped, but the %w → %s switch has a real side effect: it breaks cancellation detection for streaming dictation.

The issue: Esc-abort of streaming dictation now shows a spurious error toast

internal/tui/dictation.go:395 tells a user abort from a real failure with errors.Is(msg.err, context.Canceled). On Esc, cancelDictation() cancels the streaming ctx while StreamTranscribe is blocked in conn.Read(ctx). coder/websocket then returns ctx.Err() — i.e. context.Canceled itself (its finishRead swaps the read error for ctx.Err() on a canceled context).

• Before this PR: fmt.Errorf("Deepgram stream error: %w", err) kept that in the unwrap chain, so errors.Is(msg.err, context.Canceled) was true → the abort was treated as a clean interrupt, no toast.
• After: fmt.Errorf("...: %s", providerio.Redact(err.Error(), key)) is a flat string "Deepgram stream error: context canceled" with no unwrap chain → errors.Is is false → the handler falls through to appendDictationNotice(...) and shows a "dictation error" toast. cancelDictation also posts "Dictation cancelled.", so the user sees both notices on one Esc.

The normal stop path (release/stop gesture) is fine — stopDictation() only closes the chunk channel, the server closes cleanly (StatusNormalClosure), and StreamTranscribe returns nil. So this only bites the Esc-abort path — but that's a common gesture, and it regresses behaviour that worked before.

It also makes the providers inconsistent: transcriber_local_stream.go (sherpa) still uses %w, so an Esc-abort on a local streaming session stays clean, while Deepgram / OpenAI Realtime now toast. Same gesture, three providers, two behaviours.

Suggested fix

Short-circuit the cancel signal before redacting — context.Canceled carries no key, so it's safe to return un-redacted:

if websocket.CloseStatus(err) == websocket.StatusNormalClosure {
    return compose(), nil
}
select {
case werr := <-writeErrCh:
    if werr != nil {
        err = werr
    }
default:
}
if errors.Is(err, context.Canceled) {
    return compose(), ctx.Err()
}
return compose(), fmt.Errorf("Deepgram stream error: %s", providerio.Redact(err.Error(), d.cfg.APIKey))

…at both transcriber_deepgram.go:104 and transcriber_openai_realtime.go:117. The single site covers both the Read-error and the writeErrCh-overwrite, since the overwrite happens before the return. That keeps redaction for genuine errors and restores the abort signal so dictation.go:395 matches again. A regression test that cancels the streaming ctx mid-transcribe and asserts errors.Is(err, context.Canceled) on the returned error would lock it in — the two new tests only cover redaction, not the cancel path.

What's good

• Redaction is right and complete for the streaming paths: connect, session-config, read-error, server error event, and the writeErrCh write error (the fixup that added the else around line 159 closes the last gap). The close-reason vector is real — coder/websocket's CloseError.Error() is "status = %v and reason = %q", so a server close reason containing the key flows into the error string, and Redact scrubs it. Verified the Deepgram test would fail without Redact and isn't flaky as landed (the wait-for-CloseStream-before-close dance in the last fixup is load-bearing).
• Switching to %s is the correct security call beyond the display string: under %w, a caller could errors.As(err, &websocket.CloseError{}) and pull the raw Reason (the key) back out via Unwrap. %s closes that. The cancel-signal branch above is the only thing the switch cost.
• Keys live in headers (Authorization: Token / Bearer), not the URLs — buildURL's query is encoding/model/language only — so dial errors don't echo the key to begin with; Redact at the dial sites is belt-and-suspenders.
• Tests use a fake key, no real secrets, no new outbound headers, no provider-preset or trust-anchor changes. Clean security-only scope.

Nits (non-blocking)

• transcriber_cloud.go:76 (%s transcription request failed: %w, unredacted transport error) isn't a real leak — the key is in the Authorization header and Go's http transport error echoes the URL (no key), never the header — but redacting it for parity with lines 81/99 would be consistent. Optional.
• providerio.Redact does strings.Fields/Join, which collapses whitespace — a multi-line server error body gets flattened to one line. Cosmetic; only matters if a caller parses the structure.

CI is green across all three OSes; the only thing in the way is the behavioural side effect above.

[Vasanthdev2004]

To be clear on the request-changes above the one thing actually blocking me is the %w → %s switch killing Esc-cancel.

On Esc, coder/websocket returns context.Canceled itself from the read, and dictation.go:395 relies on errors.Is(err, context.Canceled) to treat that as a clean abort and skip the toast. Flattening to %s drops the unwrap chain, so errors.Is misses it and every Esc now shows a spurious "dictation error" toast on top of the normal "Dictation cancelled." one. Normal stop is fine; only Esc. Local/sherpa still uses %w, so the three providers now abort differently.

Easiest fix: branch if errors.Is(err, context.Canceled) { return compose(), ctx.Err() } before the redacted return there's no key in context.Canceled, so it's safe to return unwrapped, and it restores the abort signal. A cancel-path test would lock it in; the two new tests only cover redaction. Everything else I checked was clean the rest above is just detail.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Preserve cancellation identity after redacting stream errors
  internal/dictation/transcriber_deepgram.go:104 and internal/dictation/transcriber_openai_realtime.go:117
  These paths now format the conn.Read(ctx) failure with %s, so an Esc cancellation becomes only a string rather than an error matching context.Canceled. cancelDictation cancels that same streaming context, while handleDictationTranscribed suppresses the expected abort only when errors.Is(msg.err, context.Canceled) succeeds. As a result, canceling an active Deepgram or OpenAI Realtime dictation shows a spurious provider-failure notice after “Dictation cancelled.” Preserve the cancellation sentinel before rendering the redacted error (or use a redacting wrapper that still unwraps it), and cover the canceled-stream path with a regression test.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/dictation/transcriber_openai_realtime_test.go`:
- Around line 75-80: Update the test around StreamTranscribe and firstFrame so
it waits with a select for either the first-frame signal or an error from errCh.
Fail immediately with the received error if StreamTranscribe exits before
signaling firstFrame, while preserving the existing cancellation and
context.Canceled assertion for the successful startup path.
- Around line 66-68: Add a deferred close for the chunks channel in the
StreamTranscribe test after creating it, preserving the open-channel behavior
during the abort while ensuring the writer goroutine blocked on ranging over
chunks can exit when the test completes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 964956a4-458d-4f8c-b3f4-32a1a2052e3d

📥 Commits

Reviewing files that changed from the base of the PR and between cc3d2ee and 1b0f029.

📒 Files selected for processing (4)

• internal/dictation/transcriber_deepgram.go
• internal/dictation/transcriber_deepgram_test.go
• internal/dictation/transcriber_openai_realtime.go
• internal/dictation/transcriber_openai_realtime_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/dictation/transcriber_deepgram_test.go
• internal/dictation/transcriber_deepgram.go
• internal/dictation/transcriber_openai_realtime.go

[euxaristia]

Pushed 1b0f029 for the cancellation-identity finding: both transcribers now short-circuit before redaction when errors.Is(err, context.Canceled) and return the sentinel itself (it carries no key), so the Esc-abort path matches the TUI's errors.Is check again while genuine failures keep the redacted flat string. Added regression tests that cancel a live stream mid-transcribe and assert the sentinel survives, for both Deepgram and OpenAI Realtime. (The redaction tests were already moved to their per-source test files in an earlier push.)

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P2] Preserve cancellation identity on all setup/write error paths
  internal/dictation/transcriber_deepgram.go:65, internal/dictation/transcriber_openai_realtime.go:64,79,167
  The new %s formatting correctly prevents the raw transport error from being unwrapped, but the cancellation special case only runs after conn.Read. Esc can cancel the streaming context while either WebSocket dial is pending, while OpenAI sends transcription_session.update, or when its writer error is selected after an event. Those context-aware calls can return context.Canceled; flattening them causes errors.Is(msg.err, context.Canceled) in the TUI to fail, so a normal early abort displays a dictation-error notice after “Dictation cancelled.” Preserve/return ctx.Err() for cancellation before redacting the genuine-error paths, and add startup/write cancellation coverage.

• [P3] Make the new cancellation tests terminate their helper goroutines and fail promptly
  internal/dictation/transcriber_deepgram_test.go:69, internal/dictation/transcriber_openai_realtime_test.go:66
  Both tests intentionally keep chunks open but never close it. Once the reader returns after cancel(), the production writer remains blocked forever in its for range chunks, leaking a goroutine per test. They also wait unconditionally on firstFrame, so any dial/session/write regression that exits before the first frame makes the suite hang until its outer timeout instead of reporting errCh. Close chunks during cleanup and select between firstFrame and an early result from errCh; this also addresses the currently unresolved CodeRabbit requests for the OpenAI test, and the identical Deepgram test needs the same treatment.

[euxaristia]

Pushed fea74d9 addressing the remaining review feedback.

Fixed:

• Cancellation detection was racing against writer-goroutine transport errors: errors.Is(err, context.Canceled) could miss a real cancellation if a transport error overwrote err right before the check. Switched to checking ctx.Err() directly at all 6 redaction sites in both transcriber_deepgram.go and transcriber_openai_realtime.go, and removed the now-unused errors import.
• The two CancelKeepsSentinel tests leaked a writer goroutine (chunks channel never closed) and could hang on <-firstFrame. Added deferred channel close and a select against errCh in both.

Not fixing, with reasons:

• The ask to add regression tests for every redaction path (dial, session-write, writer/commit failures): the one real leak vector (server close-reason via CloseError) is already covered by the read-path tests for both providers, and it's already been confirmed that dial/write transport errors don't echo the Authorization header. A synthetic test wouldn't exercise a real leak, and a deterministic write-path-failure test would need the same handshake synchronization as the CloseStream fix for a path that's otherwise identical to what's covered.
• The transcriber_cloud.go:76 parity and providerio.Redact whitespace-collapsing nits: both were explicitly marked optional/cosmetic, and transcriber_cloud.go isn't touched by this PR's diff at all, so leaving it out of scope.

[Vasanthdev2004]

Re-verified on the current head. The blocking one is fixed: the switch that was swallowing the cancel signal is resolved. Both streaming read paths now return ctx.Err() before the redacted branch, so an Esc-abort no longer fires a spurious dictation-error toast alongside "Dictation cancelled." You went past what I asked and covered the dial, session-update, and post-event paths too, and the two new cancel-path tests would fail without the short-circuit. Redaction still covers every genuine-error path, so nothing leaked back in. CI is green on all three OSes.

My two remaining notes were explicitly non-blocking nits (the out-of-scope non-streaming cloud path, and the whitespace-collapse in Redact), so they don't hold this up. Approving.

[euxaristia]

@jatmn Ready for re-review on head fea74d9.

That commit covers the remaining cancel-identity paths (dial, session update, read, writeErrCh) via ctx.Err() before redaction, and the CancelKeepsSentinel tests now close chunks and select on firstFrame vs errCh so they fail promptly without leaking the writer goroutine.

[euxaristia]

Ready for re-review on head 3a19432 (rebased onto current main).

Addressed jatmn P2/P3 from the 2026-07-19 review:

P2 - cancellation identity on all setup/write paths (in af37da2, landed earlier as fea74d9):

• Dial (Deepgram + OpenAI): return ctx.Err() before redacting
• OpenAI session-update write: return ctx.Err() before redacting
• Read path (both): key on ctx.Err() rather than unwrapping err (writer transport errors can overwrite the cancel sentinel)
• OpenAI post-event writeErrCh: return ctx.Err() when the context is cancelled

P3 - cancellation tests (same commit):

• defer close(chunks) so the writer goroutine exits
• select on firstFrame vs errCh so early failures fail promptly instead of hanging

Follow-up in 3a19432 (startup/write cancellation coverage requested in P2):

• TestDeepgramStreamTranscribeDialCancelKeepsSentinel / TestOpenAIRealtimeStreamTranscribeDialCancelKeepsSentinel
• TestDeepgramStreamTranscribeStartupCancelKeepsSentinel / TestOpenAIRealtimeStreamTranscribeStartupCancelKeepsSentinel
• TestOpenAIRealtimeStreamTranscribeWriteCancelKeepsSentinel (post-event writeErrCh path)

Verified: go test ./internal/dictation/... -count=1 and go vet ./internal/dictation/... both pass.

[jatmn]

Findings

• [P2] Preserve cancellation identity for a racing realtime error event
  internal/dictation/transcriber_openai_realtime.go:173
  If Esc cancels the stream after conn.Read has received an OpenAI error event but before this switch handles it, this branch returns a newly formatted redacted error instead of ctx.Err(). handleDictationTranscribed recognizes a clean abort only through errors.Is(err, context.Canceled), so that race displays a dictation-failure notice after the user's cancellation—the same regression the other new ctx.Err() guards avoid. Check ctx.Err() before returning the redacted event error and add coverage for the cancellation/event race.

[euxaristia]

Addressed in 9826b8f (rebased onto current main).

[P2] Preserve cancellation identity for a racing realtime error event

• After a delta/completed partial callback, and on the realtimeError path, the stream now checks ctx.Err() first so Esc that lands after conn.Read already has an OpenAI error frame still returns context.Canceled instead of a redacted failure notice.
• Added TestOpenAIRealtimeStreamTranscribeErrorRaceKeepsSentinel: cancels synchronously from the delta callback (same shape as TUI Esc handling) with the server sending delta then error back-to-back.

Verified: go test ./internal/dictation/ -count=3 passes.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P2] Do not auto-submit after normalizing the cancelled realtime-error race
  internal/dictation/transcriber_openai_realtime.go:188-191
  This new cancellation branch makes the delta -> Esc -> already-buffered error sequence return a nonempty transcript with context.Canceled. cancelDictation has already discarded the live region and reset the session, but handleDictationTranscribed suppresses the canceled error and then falls through to msg.submit. Consequently, with stt.autoSubmit enabled, Esc can submit the restored pre-existing composer text—the prompt the user explicitly chose not to send. Handle canceled streaming completions as terminal before the auto-submit path, and cover the TUI-level race.

[euxaristia]

Done in 56c6a73. handleDictationTranscribed suppressed context.Canceled but then fell through to the msg.submit branch, so the delta -> Esc -> already-buffered error race you flagged returned a nonempty transcript alongside context.Canceled, and with stt.autoSubmit on, Esc could submit the composer's restored pre-existing text.

I moved the context.Canceled check ahead of the auto-submit path: a canceled result now returns immediately as terminal, before any text/submit handling runs. cancelDictation has already discarded the live region and posted "Dictation cancelled." by the time that message arrives, so there's nothing left to do.

Added TestDictationCanceledStreamRaceDoesNotAutoSubmit in internal/tui/dictation_test.go, which drives the same race at the TUI level: partial text into the composer, cancelDictation, then a dictationTranscribedMsg with a nonempty text, submit: true, and context.Canceled arriving after the cancel. It asserts the composer keeps the pre-existing text and no submit command is returned. Verified it fails without the fix (composer got cleared) and passes with it.

go build ./..., go test ./internal/dictation/... ./internal/tui/..., and gofmt -l . are all clean.

[coderabbitai]

🧹 Nitpick comments (1)

internal/dictation/transcriber_openai_realtime_test.go (1)

147-199: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This test never actually reaches the writeErrCh branch it claims to cover.

chunks is buffered with exactly one item before the writer goroutine starts; once consumed, the writer blocks on range chunks (never closed until the test's defer), so it never calls conn.Write/writeJSON again and writeErrCh is never populated during the test. Cancellation is only ever observed by the main goroutine's blocked conn.Read, i.e. the same path already covered by TestOpenAIRealtimeStreamTranscribeCancelKeepsSentinel. The select { case werr := <-writeErrCh: ... } branch in transcriber_openai_realtime.go (with its own ctx.Err() check) is not exercised here, despite the comment/name suggesting otherwise.

Either retitle/adjust the comment to reflect what's actually tested, or drive an actual write failure (e.g. close chunks right after cancel so the writer's final writeJSON fails and lands in writeErrCh before the main loop's next non-blocking select) to genuinely cover that branch.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/dictation/transcriber_openai_realtime_test.go` around lines 147 -
199, The test named TestOpenAIRealtimeStreamTranscribeWriteCancelKeepsSentinel
does not exercise the writeErrCh path because the writer blocks waiting for
another chunk. Adjust the test to trigger a final write after cancellation—such
as closing chunks immediately after cancel—so the writer’s writeJSON failure
reaches writeErrCh and the test validates context.Canceled there; otherwise
rename and update the test comments to describe the read-cancellation path it
actually covers.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/dictation/transcriber_openai_realtime_test.go`:
- Around line 147-199: The test named
TestOpenAIRealtimeStreamTranscribeWriteCancelKeepsSentinel does not exercise the
writeErrCh path because the writer blocks waiting for another chunk. Adjust the
test to trigger a final write after cancellation—such as closing chunks
immediately after cancel—so the writer’s writeJSON failure reaches writeErrCh
and the test validates context.Canceled there; otherwise rename and update the
test comments to describe the read-cancellation path it actually covers.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 36b58590-8b91-4278-90b3-5e644a8a8413

📥 Commits

Reviewing files that changed from the base of the PR and between fea74d9 and 56c6a73.

📒 Files selected for processing (6)

• internal/dictation/transcriber_deepgram.go
• internal/dictation/transcriber_deepgram_test.go
• internal/dictation/transcriber_openai_realtime.go
• internal/dictation/transcriber_openai_realtime_test.go
• internal/tui/dictation.go
• internal/tui/dictation_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/dictation/transcriber_openai_realtime.go
• internal/dictation/transcriber_deepgram.go

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P3] Exercise the writeErrCh cancellation branch the new regression names
  internal/dictation/transcriber_openai_realtime_test.go:176
  This test enqueues one chunk and leaves chunks open until after StreamTranscribe returns. The writer consumes that chunk and then blocks on range chunks; cancelling therefore releases the reader through conn.Read(ctx) before any failed write can populate writeErrCh. Consequently, the new writeErrCh cancellation branch at transcriber_openai_realtime.go:195-203 is untested despite the test name and comment. Synchronize a failed append/commit write into that channel (or rename the test to describe the read-cancellation case).