diff --git a/internal/dictation/transcriber_deepgram.go b/internal/dictation/transcriber_deepgram.go index b8b9319aa..2f25b386a 100644 --- a/internal/dictation/transcriber_deepgram.go +++ b/internal/dictation/transcriber_deepgram.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" + "github.com/Gitlawb/zero/internal/providers/providerio" "github.com/coder/websocket" ) @@ -60,7 +61,15 @@ func (d *deepgramTranscriber) StreamTranscribe(ctx context.Context, chunks <-cha HTTPHeader: http.Header{"Authorization": {"Token " + d.cfg.APIKey}}, }) if err != nil { - return "", fmt.Errorf("connecting to Deepgram: %w", err) + // Preserve the context.Canceled sentinel (it carries no key) so an + // Esc-abort during dial still matches the UI's errors.Is check. Key + // on ctx.Err() itself rather than unwrapping err: a cancellation can + // surface as a plain transport error (e.g. "closed network + // connection") rather than context.Canceled directly. + if ctx.Err() != nil { + return "", ctx.Err() + } + return "", fmt.Errorf("connecting to Deepgram: %s", providerio.Redact(err.Error(), d.cfg.APIKey)) } defer conn.CloseNow() @@ -100,7 +109,17 @@ func (d *deepgramTranscriber) StreamTranscribe(ctx context.Context, chunks <-cha } default: } - return compose(), fmt.Errorf("Deepgram stream error: %w", err) + // A user abort cancels the streaming context; the UI matches it + // with errors.Is(err, context.Canceled), so return the sentinel + // itself (it carries no key) instead of a flat redacted string. + // Key on ctx.Err() rather than unwrapping err: the writeErrCh + // swap above can replace err with a plain transport error (e.g. + // "closed network connection") that doesn't itself unwrap to + // context.Canceled even though the cancellation is what caused it. + if ctx.Err() != nil { + return compose(), ctx.Err() + } + return compose(), fmt.Errorf("Deepgram stream error: %s", providerio.Redact(err.Error(), d.cfg.APIKey)) } if typ != websocket.MessageText { continue diff --git a/internal/dictation/transcriber_deepgram_test.go b/internal/dictation/transcriber_deepgram_test.go new file mode 100644 index 000000000..09bc3c03c --- /dev/null +++ b/internal/dictation/transcriber_deepgram_test.go @@ -0,0 +1,214 @@ +package dictation + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + + "github.com/coder/websocket" +) + +func TestDeepgramStreamTranscribeErrorRedaction(t *testing.T) { + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + // Drain the client's audio frames, then wait for CloseStream so the + // client has flushed its writes before we reject the connection. Closing + // immediately (before CloseStream) risks the client failing on a write + // error and never observing this API-key-bearing close reason. + for { + typ, data, err := c.Read(ctx) + if err != nil { + return + } + if typ == websocket.MessageText && strings.Contains(string(data), "CloseStream") { + break + } + } + c.Close(websocket.StatusPolicyViolation, "invalid key sk-test-key") + }) + + tr, err := NewDeepgramTranscriber(DeepgramConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + chunks := make(chan []byte, 1) + chunks <- make([]byte, 320) + close(chunks) + _, ferr := tr.StreamTranscribe(context.Background(), chunks, func(string, bool) {}) + if ferr == nil { + t.Fatal("expected error") + } + if strings.Contains(ferr.Error(), "sk-test-key") { + t.Errorf("API key leaked: %v", ferr) + } +} + +func TestDeepgramStreamTranscribeCancelKeepsSentinel(t *testing.T) { + firstFrame := make(chan struct{}) + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + // Hold the connection open, never answering, so the client blocks in + // Read until its context is cancelled (the Esc-abort path). + var once sync.Once + for { + if _, _, err := c.Read(ctx); err != nil { + return + } + once.Do(func() { close(firstFrame) }) + } + }) + + tr, err := NewDeepgramTranscriber(DeepgramConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + chunks := make(chan []byte, 1) + defer close(chunks) + chunks <- make([]byte, 320) + // The channel stays open: the session is live when the user aborts. + errCh := make(chan error, 1) + go func() { + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + errCh <- ferr + }() + + select { + case <-firstFrame: + cancel() + ferr := <-errCh + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("cancelled stream error lost the context.Canceled sentinel: %v", ferr) + } + case ferr := <-errCh: + t.Fatalf("StreamTranscribe failed early instead of blocking: %v", ferr) + } +} + +// Esc can cancel while the WebSocket dial is still pending. The dial error is +// redacted with %s, so we must return ctx.Err() rather than a flat string. +func TestDeepgramStreamTranscribeDialCancelKeepsSentinel(t *testing.T) { + tr, err := NewDeepgramTranscriber(DeepgramConfig{ + APIKey: "sk-test-key", + BaseURL: "ws://127.0.0.1:1", // never reached; ctx is already cancelled + }) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + chunks := make(chan []byte) + defer close(chunks) + + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("dial-cancel lost the context.Canceled sentinel: %v", ferr) + } +} + +// Esc right after accept can race the first write or the first Read; both +// setup/write redaction sites must still yield context.Canceled. +func TestDeepgramStreamTranscribeStartupCancelKeepsSentinel(t *testing.T) { + accepted := make(chan struct{}) + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + close(accepted) + for { + if _, _, err := c.Read(ctx); err != nil { + return + } + } + }) + + tr, err := NewDeepgramTranscriber(DeepgramConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + chunks := make(chan []byte, 1) + defer close(chunks) + // No frames yet: the writer blocks on chunks or the reader blocks on Read. + errCh := make(chan error, 1) + go func() { + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + errCh <- ferr + }() + + select { + case <-accepted: + cancel() + ferr := <-errCh + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("startup-cancel lost the context.Canceled sentinel: %v", ferr) + } + case ferr := <-errCh: + t.Fatalf("StreamTranscribe failed before accept: %v", ferr) + } +} + +func TestDeepgramCustomHeaderErrorRedaction(t *testing.T) { + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + for { + typ, data, err := c.Read(ctx) + if err != nil { + return + } + if typ == websocket.MessageText && strings.Contains(string(data), "CloseStream") { + break + } + } + c.Close(websocket.StatusPolicyViolation, "X-Api-Key: sk-custom-secret-key-1234567890 failed") + }) + + tr, err := NewDeepgramTranscriber(DeepgramConfig{APIKey: "sk-custom-secret-key-1234567890", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + chunks := make(chan []byte, 1) + chunks <- make([]byte, 320) + close(chunks) + _, ferr := tr.StreamTranscribe(context.Background(), chunks, func(string, bool) {}) + if ferr == nil { + t.Fatal("expected error") + } + if strings.Contains(ferr.Error(), "sk-custom-secret-key-1234567890") { + t.Errorf("Custom header API key leaked: %v", ferr) + } +} + +func TestDeepgramSinglePassRedaction(t *testing.T) { + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + for { + typ, data, err := c.Read(ctx) + if err != nil { + return + } + if typ == websocket.MessageText && strings.Contains(string(data), "CloseStream") { + break + } + } + c.Close(websocket.StatusPolicyViolation, "error with sk-key1234567890abcdef1234") + }) + + tr, err := NewDeepgramTranscriber(DeepgramConfig{APIKey: "sk-key1234567890abcdef1234", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + chunks := make(chan []byte, 1) + chunks <- make([]byte, 320) + close(chunks) + _, ferr := tr.StreamTranscribe(context.Background(), chunks, func(string, bool) {}) + if ferr == nil { + t.Fatal("expected error") + } + if strings.Contains(ferr.Error(), "[REDACTED:[REDACTED") { + t.Errorf("nested redaction marker created: %v", ferr) + } +} diff --git a/internal/dictation/transcriber_openai_realtime.go b/internal/dictation/transcriber_openai_realtime.go index 590cbc8b4..64ee56f3a 100644 --- a/internal/dictation/transcriber_openai_realtime.go +++ b/internal/dictation/transcriber_openai_realtime.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" + "github.com/Gitlawb/zero/internal/providers/providerio" "github.com/coder/websocket" ) @@ -59,7 +60,15 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks }, }) if err != nil { - return "", fmt.Errorf("connecting to OpenAI Realtime: %w", err) + // Preserve the context.Canceled sentinel (it carries no key) so an + // Esc-abort during dial still matches the UI's errors.Is check. Key + // on ctx.Err() itself rather than unwrapping err: a cancellation can + // surface as a plain transport error (e.g. "closed network + // connection") rather than context.Canceled directly. + if ctx.Err() != nil { + return "", ctx.Err() + } + return "", fmt.Errorf("connecting to OpenAI Realtime: %s", providerio.Redact(err.Error(), o.cfg.APIKey)) } defer conn.CloseNow() @@ -74,7 +83,12 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks }, } if err := writeJSON(ctx, conn, sessionUpdate); err != nil { - return "", fmt.Errorf("configuring OpenAI Realtime session: %w", err) + // Same cancellation short-circuit as the dial above: an Esc-abort + // while the session update is in flight must stay context.Canceled. + if ctx.Err() != nil { + return "", ctx.Err() + } + return "", fmt.Errorf("configuring OpenAI Realtime session: %s", providerio.Redact(err.Error(), o.cfg.APIKey)) } writeErrCh := make(chan error, 1) @@ -113,7 +127,17 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks } default: } - return compose(), fmt.Errorf("OpenAI Realtime stream error: %w", err) + // A user abort cancels the streaming context; the UI matches it + // with errors.Is(err, context.Canceled), so return the sentinel + // itself (it carries no key) instead of a flat redacted string. + // Key on ctx.Err() rather than unwrapping err: the writeErrCh + // swap above can replace err with a plain transport error (e.g. + // "closed network connection") that doesn't itself unwrap to + // context.Canceled even though the cancellation is what caused it. + if ctx.Err() != nil { + return compose(), ctx.Err() + } + return compose(), fmt.Errorf("OpenAI Realtime stream error: %s", providerio.Redact(err.Error(), o.cfg.APIKey)) } if typ != websocket.MessageText { continue @@ -125,6 +149,12 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks if onPartial != nil { onPartial(compose(), false) } + // onPartial may deliver the partial that makes the TUI cancel + // (Esc). Observe that before the next Read, which can already + // have a racing OpenAI error frame buffered. + if ctx.Err() != nil { + return compose(), ctx.Err() + } case realtimeCompleted: // A completed item replaces the in-progress delta buffer with the // server's authoritative transcript for that segment. @@ -138,6 +168,9 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks if onPartial != nil { onPartial(compose(), true) } + if ctx.Err() != nil { + return compose(), ctx.Err() + } // Once we've committed the buffer (user stopped) and the server has // returned a completed transcription, the utterance is done. if committed { @@ -147,7 +180,15 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks case realtimeCommitted: committed = true case realtimeError: - return compose(), fmt.Errorf("OpenAI Realtime error: %s", evt.text) + // Esc can race an incoming error event: conn.Read may already have + // the OpenAI error in hand by the time cancellation lands. Key on + // ctx.Err() first, same as every other return point in this loop, + // so a cancel that wins the race still surfaces as context.Canceled + // instead of a spurious redacted failure alongside it. + if ctx.Err() != nil { + return compose(), ctx.Err() + } + return compose(), fmt.Errorf("OpenAI Realtime error: %s", providerio.Redact(evt.text, o.cfg.APIKey)) } // The writer signals commit completion out of band; observe it so a // stop with no further audio still flips `committed`. @@ -155,6 +196,10 @@ func (o *openAIRealtimeTranscriber) StreamTranscribe(ctx context.Context, chunks case werr := <-writeErrCh: if werr == nil { committed = true + } else if ctx.Err() != nil { + return compose(), ctx.Err() + } else { + return compose(), fmt.Errorf("OpenAI Realtime stream error: %s", providerio.Redact(werr.Error(), o.cfg.APIKey)) } default: } diff --git a/internal/dictation/transcriber_openai_realtime_test.go b/internal/dictation/transcriber_openai_realtime_test.go new file mode 100644 index 000000000..c46f91287 --- /dev/null +++ b/internal/dictation/transcriber_openai_realtime_test.go @@ -0,0 +1,233 @@ +package dictation + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + + "github.com/coder/websocket" +) + +func TestOpenAIRealtimeStreamTranscribeErrorRedaction(t *testing.T) { + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + defer c.Close(websocket.StatusNormalClosure, "") + for { + typ, _, err := c.Read(ctx) + if err != nil { + return + } + if typ != websocket.MessageText { + continue + } + _ = c.Write(ctx, websocket.MessageText, []byte(`{"type":"error","error":{"message":"invalid API key sk-test-key"}}`)) + } + }) + + tr, err := NewOpenAIRealtimeTranscriber(OpenAIRealtimeConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + chunks := make(chan []byte, 1) + chunks <- make([]byte, 480) + close(chunks) + _, ferr := tr.StreamTranscribe(context.Background(), chunks, func(string, bool) {}) + if ferr == nil { + t.Fatal("expected error") + } + if strings.Contains(ferr.Error(), "sk-test-key") { + t.Errorf("API key leaked: %v", ferr) + } +} + +func TestOpenAIRealtimeStreamTranscribeCancelKeepsSentinel(t *testing.T) { + firstFrame := make(chan struct{}) + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + // Hold the connection open, never answering, so the client blocks in + // Read until its context is cancelled (the Esc-abort path). + var once sync.Once + for { + if _, _, err := c.Read(ctx); err != nil { + return + } + once.Do(func() { close(firstFrame) }) + } + }) + + tr, err := NewOpenAIRealtimeTranscriber(OpenAIRealtimeConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + chunks := make(chan []byte, 1) + defer close(chunks) + chunks <- make([]byte, 480) + // The channel stays open: the session is live when the user aborts. + errCh := make(chan error, 1) + go func() { + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + errCh <- ferr + }() + + select { + case <-firstFrame: + cancel() + ferr := <-errCh + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("cancelled stream error lost the context.Canceled sentinel: %v", ferr) + } + case ferr := <-errCh: + t.Fatalf("StreamTranscribe failed early instead of blocking: %v", ferr) + } +} + +// Esc can cancel while the WebSocket dial is still pending. The dial error is +// redacted with %s, so we must return ctx.Err() rather than a flat string. +func TestOpenAIRealtimeStreamTranscribeDialCancelKeepsSentinel(t *testing.T) { + tr, err := NewOpenAIRealtimeTranscriber(OpenAIRealtimeConfig{ + APIKey: "sk-test-key", + BaseURL: "ws://127.0.0.1:1", // never reached; ctx is already cancelled + }) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + chunks := make(chan []byte) + defer close(chunks) + + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("dial-cancel lost the context.Canceled sentinel: %v", ferr) + } +} + +// Esc right after accept can hit the session-update write or the first Read; +// both redaction sites must still yield context.Canceled. +func TestOpenAIRealtimeStreamTranscribeStartupCancelKeepsSentinel(t *testing.T) { + accepted := make(chan struct{}) + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + close(accepted) + // Do not read: leave the client in session-update write or first Read. + <-ctx.Done() + }) + + tr, err := NewOpenAIRealtimeTranscriber(OpenAIRealtimeConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + chunks := make(chan []byte, 1) + defer close(chunks) + errCh := make(chan error, 1) + go func() { + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + errCh <- ferr + }() + + select { + case <-accepted: + cancel() + ferr := <-errCh + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("startup-cancel lost the context.Canceled sentinel: %v", ferr) + } + case ferr := <-errCh: + t.Fatalf("StreamTranscribe failed before accept: %v", ferr) + } +} + +// After a server event the client selects writeErrCh. Cancel while the writer +// is blocked on more chunks so that path observes a cancelled write and must +// still return context.Canceled rather than a redacted flat string. +func TestOpenAIRealtimeStreamTranscribeWriteCancelKeepsSentinel(t *testing.T) { + sessionReceived := make(chan struct{}) + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + if _, _, err := c.Read(ctx); err != nil { + return + } + close(sessionReceived) + for { + if _, _, err := c.Read(ctx); err != nil { + return + } + } + }) + + tr, err := NewOpenAIRealtimeTranscriber(OpenAIRealtimeConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + chunks := make(chan []byte, 1) + chunks <- make([]byte, 480) + defer close(chunks) + + errCh := make(chan error, 1) + go func() { + _, ferr := tr.StreamTranscribe(ctx, chunks, nil) + errCh <- ferr + }() + + select { + case <-sessionReceived: + cancel() + ferr := <-errCh + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("write-path cancel lost the context.Canceled sentinel: %v", ferr) + } + case ferr := <-errCh: + t.Fatalf("StreamTranscribe failed early instead of blocking: %v", ferr) + } +} + +// jatmn (review, PR #710): Esc can race an incoming OpenAI error event. +// conn.Read may already have the error frame in hand by the time the +// cancellation lands. The server fires a delta immediately followed by +// the error, back to back. Cancel from inside the delta callback (same +// path the TUI uses when a partial triggers Esc handling) so the cancel +// is observed before the next event is processed. The returned error +// must still be context.Canceled, not the redacted OpenAI error. +func TestOpenAIRealtimeStreamTranscribeErrorRaceKeepsSentinel(t *testing.T) { + url := wsTestServer(t, func(ctx context.Context, c *websocket.Conn) { + if _, _, err := c.Read(ctx); err != nil { + return + } + _ = c.Write(ctx, websocket.MessageText, []byte( + `{"type":"conversation.item.input_audio_transcription.delta","delta":"hi"}`, + )) + _ = c.Write(ctx, websocket.MessageText, []byte( + `{"type":"error","error":{"message":"invalid API key sk-test-key"}}`, + )) + <-ctx.Done() + }) + + tr, err := NewOpenAIRealtimeTranscriber(OpenAIRealtimeConfig{APIKey: "sk-test-key", BaseURL: url}) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + chunks := make(chan []byte, 1) + defer close(chunks) + chunks <- make([]byte, 480) + var once sync.Once + _, ferr := tr.StreamTranscribe(ctx, chunks, func(string, bool) { + // Cancel synchronously from the partial callback, before the + // stream loop continues to the already-buffered error frame. + once.Do(cancel) + }) + if !errors.Is(ferr, context.Canceled) { + t.Fatalf("StreamTranscribe error = %v, want context.Canceled (cancel must win over a racing OpenAI error event)", ferr) + } +} diff --git a/internal/providers/providerio/providerio.go b/internal/providers/providerio/providerio.go index 374269f65..ab967719b 100644 --- a/internal/providers/providerio/providerio.go +++ b/internal/providers/providerio/providerio.go @@ -429,16 +429,17 @@ func looksLikeToken(w string) bool { // Redact removes known API-key and bearer-token forms from provider messages. func Redact(message string, secrets ...string) string { for _, secret := range secrets { - if secret != "" { + if secret != "" && !strings.HasPrefix(secret, "[REDACTED") { message = strings.ReplaceAll(message, secret, "[REDACTED]") } } words := strings.Fields(message) for index := 0; index < len(words)-1; index++ { - // Only redact the word after "Bearer" when it is actually token-shaped, so the - // provider's own help text ("use Bearer authentication", "Bearer token") is no - // longer corrupted into "authorization [REDACTED]". (AUDIT-H7) - if strings.EqualFold(strings.TrimRight(words[index], ":"), "Bearer") && looksLikeToken(words[index+1]) { + hdr := strings.EqualFold(strings.TrimRight(words[index], ":"), "Bearer") || + strings.EqualFold(strings.TrimRight(words[index], ":"), "Token") || + strings.EqualFold(strings.TrimRight(words[index], ":"), "X-Api-Key") || + strings.EqualFold(strings.TrimRight(words[index], ":"), "api-key") + if hdr && !strings.HasPrefix(words[index+1], "[REDACTED") && looksLikeToken(words[index+1]) { words[index+1] = "[REDACTED]" } } diff --git a/internal/tui/dictation.go b/internal/tui/dictation.go index 3d96d82f6..b995ceac7 100644 --- a/internal/tui/dictation.go +++ b/internal/tui/dictation.go @@ -61,6 +61,7 @@ type dictationController struct { browseVariants []dictation.ModelVariant // in-flight session state + sessionID int64 streaming bool recorder Recorder transcriber Transcriber @@ -108,6 +109,7 @@ type dictationStartedMsg struct { // dictationTranscribedMsg carries the final transcript of a batch (or a // completed streaming) recording. type dictationTranscribedMsg struct { + sessionID int64 text string err error submit bool @@ -276,7 +278,7 @@ func (m model) stopDictation() (model, tea.Cmd) { } return m, nil // final text arrives via the streaming command already running } - return m, transcribeBatchCmd(m.dictation.ctx, m.dictation.recorder, m.dictation.transcriber, m.dictation.cfg.AutoSubmitEnabled()) + return m, transcribeBatchCmd(m.dictation.sessionID, m.dictation.ctx, m.dictation.recorder, m.dictation.transcriber, m.dictation.cfg.AutoSubmitEnabled()) } // cancelDictation aborts an in-flight recording without transcribing. Bound to @@ -311,6 +313,7 @@ func (d dictationController) maxDuration() time.Duration { } func (d *dictationController) reset() { + d.sessionID++ d.phase = dictIdle d.recorder = nil d.transcriber = nil @@ -335,17 +338,17 @@ func startBatchRecordingCmd(rec Recorder) tea.Cmd { // transcribeBatchCmd stops the recording, transcribes the audio, and reports the // final text. Runs off the UI goroutine — Stop waits on the capture tool and // Transcribe does a network round-trip or a local exec. -func transcribeBatchCmd(ctx context.Context, rec Recorder, transcriber Transcriber, submit bool) tea.Cmd { +func transcribeBatchCmd(sessionID int64, ctx context.Context, rec Recorder, transcriber Transcriber, submit bool) tea.Cmd { if ctx == nil { ctx = context.Background() } return func() tea.Msg { audio, err := rec.Stop() if err != nil { - return dictationTranscribedMsg{err: err} + return dictationTranscribedMsg{sessionID: sessionID, err: err} } text, err := transcriber.Transcribe(ctx, audio) - return dictationTranscribedMsg{text: text, err: err, submit: submit} + return dictationTranscribedMsg{sessionID: sessionID, text: text, err: err, submit: submit} } } @@ -375,6 +378,9 @@ func (m model) handleDictationStarted(msg dictationStartedMsg) (model, tea.Cmd) // handleDictationTranscribed inserts the final transcript into the composer (or // submits it when stt.autoSubmit is on), then returns to idle. func (m model) handleDictationTranscribed(msg dictationTranscribedMsg) (tea.Model, tea.Cmd) { + if msg.sessionID != 0 && msg.sessionID != m.dictation.sessionID { + return m, nil + } streaming := m.dictation.streaming || msg.streaming m = m.commitDictationRegion() // A streaming session can end via a transcriber error (not just a user stop), so @@ -392,7 +398,17 @@ func (m model) handleDictationTranscribed(msg dictationTranscribedMsg) (tea.Mode } m.dictation.reset() - if msg.err != nil && !errors.Is(msg.err, context.Canceled) { + if errors.Is(msg.err, context.Canceled) { + // cancelDictation already discarded the live region, reset the + // session, and posted "Dictation cancelled." A streaming transcriber + // can still race a buffered event past that cancel and report back a + // nonempty compose()+context.Canceled; treat that as terminal here, + // before the auto-submit branch below, so Esc can never fall through + // to msg.submit and fire the composer's restored pre-existing text. + return m, nil + } + + if msg.err != nil { // A cloud auth failure (missing/invalid key) is fixable in place: reopen the // API-key prompt for the current provider so the user can paste a key and // retry, instead of hitting a dead-end "run zero auth" line. diff --git a/internal/tui/dictation_stream.go b/internal/tui/dictation_stream.go index 90b74c321..85e73814c 100644 --- a/internal/tui/dictation_stream.go +++ b/internal/tui/dictation_stream.go @@ -14,8 +14,9 @@ import ( // mechanism agent text deltas use (§6). text is the cumulative best transcript // so far (not a delta); final marks a settled segment. type sttPartialMsg struct { - text string - final bool + sessionID int64 + text string + final bool } // startStreamingDictation begins continuous capture and launches the streaming @@ -38,6 +39,7 @@ func (m model) startStreamingDictation() (model, tea.Cmd) { } transcriber := m.dictation.transcriber submit := m.dictation.cfg.AutoSubmitEnabled() + sessionID := m.dictation.sessionID // Tap the audio: compute a mic level per chunk for the live waveform, then // forward the chunk to the transcriber. A small buffer keeps the tap from @@ -63,11 +65,11 @@ func (m model) startStreamingDictation() (model, tea.Cmd) { streamCmd := func() tea.Msg { onPartial := func(text string, final bool) { if sink != nil { - sink(sttPartialMsg{text: text, final: final}) + sink(sttPartialMsg{sessionID: sessionID, text: text, final: final}) } } text, err := transcriber.StreamTranscribe(ctx, tapped, onPartial) - return dictationTranscribedMsg{text: text, err: err, submit: submit, streaming: true} + return dictationTranscribedMsg{sessionID: sessionID, text: text, err: err, submit: submit, streaming: true} } // Streaming drives the waveform from real levels (no synthetic tick needed). return m, streamCmd @@ -77,6 +79,9 @@ func (m model) startStreamingDictation() (model, tea.Cmd) { // composer, replacing the previously-rendered live region wholesale so the text // builds up in place as the user keeps talking. func (m model) handleDictationPartial(msg sttPartialMsg) model { + if msg.sessionID != 0 && msg.sessionID != m.dictation.sessionID { + return m + } // Ignore stragglers that arrive after the session ended (cancel/final). if m.dictation.phase != dictRecording && m.dictation.phase != dictTranscribing { return m diff --git a/internal/tui/dictation_test.go b/internal/tui/dictation_test.go index 5101cbd64..fcfabea9e 100644 --- a/internal/tui/dictation_test.go +++ b/internal/tui/dictation_test.go @@ -136,6 +136,44 @@ func TestDictationStreamingPartialReplacesRegion(t *testing.T) { } } +func TestDictationCanceledStreamRaceDoesNotAutoSubmit(t *testing.T) { + // Esc can race an already-buffered realtime event: cancelDictation discards + // the live region and resets state synchronously, but the streaming + // goroutine's dictationTranscribedMsg (a nonempty compose() alongside + // context.Canceled) can still arrive afterward. With stt.autoSubmit on, + // that must never fall through to msg.submit and fire the composer's + // restored pre-existing text. + m := model{} + m.setComposerState(composerState{text: "existing prompt", cursor: len("existing prompt")}) + m.dictation.phase = dictRecording + m.dictation.streaming = true + + m = m.handleDictationPartial(sttPartialMsg{text: "half-formed transcript"}) + if m.composer.text != "existing prompt half-formed transcript" { + t.Fatalf("partial did not render into composer: %q", m.composer.text) + } + + m, _ = m.cancelDictation() + if m.composer.text != "existing prompt" { + t.Fatalf("cancel should restore the pre-existing composer text, got %q", m.composer.text) + } + + // The already-buffered event's message arrives after the cancel above. + next, cmd := m.handleDictationTranscribed(dictationTranscribedMsg{ + text: "half-formed transcript", + err: context.Canceled, + submit: true, + streaming: true, + }) + got := next.(model) + if got.composer.text != "existing prompt" { + t.Errorf("a raced cancellation must not auto-submit the restored composer text, got %q", got.composer.text) + } + if cmd != nil { + t.Error("a raced cancellation must not return a submit command") + } +} + func TestDictationCommitKeepsStreamedText(t *testing.T) { m := model{} m.setComposerState(composerState{text: "", cursor: 0}) @@ -302,3 +340,32 @@ func TestCurrentModelLabel(t *testing.T) { t.Errorf("no-model label = %q", got) } } + +func TestStaleDictationCompletionIgnoredAfterCancel(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "original composer text", cursor: 22}) + m.dictation.sessionID = 1 + m.dictation.phase = dictRecording + + m, _ = m.cancelDictation() + + if m.composer.text != "original composer text" { + t.Fatalf("composer text = %q, want 'original composer text'", m.composer.text) + } + + staleMsg := dictationTranscribedMsg{ + sessionID: 1, + text: "stale text that should be ignored", + submit: true, + streaming: true, + } + afterStale, cmd := m.handleDictationTranscribed(staleMsg) + got := afterStale.(model) + + if got.composer.text != "original composer text" { + t.Fatalf("composer text changed to %q after stale completion", got.composer.text) + } + if cmd != nil { + t.Fatalf("expected no command for stale completion, got %v", cmd) + } +}