diff --git a/internal/acp/translate.go b/internal/acp/translate.go index ae03a7032..7523114bd 100644 --- a/internal/acp/translate.go +++ b/internal/acp/translate.go @@ -139,7 +139,7 @@ func browserToolTitle(command, rawArgs string) string { } // browserTitleTextSafe validates text after URL parsing has decoded escaped -// UTF-8 in the host. Valid UTF-8 alone is not presentation-safe: control, +// UTF-8 in the host. Valid UTF-8 alone is not presentation-safe: whitespace, control, // format/bidi, and line/paragraph separator runes can reorder or split the // permission label shown to a user. The execution URL remains unchanged. func browserTitleTextSafe(text string) bool { @@ -147,7 +147,7 @@ func browserTitleTextSafe(text string) bool { return false } for _, r := range text { - if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) { + if unicode.IsSpace(r) || unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) { return false } } diff --git a/internal/acp/translate_test.go b/internal/acp/translate_test.go index 729830cf4..053274b05 100644 --- a/internal/acp/translate_test.go +++ b/internal/acp/translate_test.go @@ -224,6 +224,9 @@ func TestBrowserPermissionTitlesMirrorSafeToolArguments(t *testing.T) { func TestBrowserOpenTitlesRejectDecodedUnicodePresentationControls(t *testing.T) { for _, rawURL := range []string{ + "https://safe.example%C2%A0evil.test/path", + "https://safe.example%E2%80%83evil.test/path", + "https://safe.example%E3%80%80evil.test/path", "https://safe.example%E2%80%AEevil.test/path", "https://safe.example%E2%81%A6evil.test/path", "https://safe.example%C2%85evil.test/path", @@ -243,6 +246,9 @@ func TestBrowserOpenTitlesRejectDecodedUnicodePresentationControls(t *testing.T) if err != nil { t.Fatal(err) } + if title := browserToolTitle("open", string(args)); title != "browser open" { + t.Errorf("unsafe origin accepted before permission translation: %q", title) + } updates := []ToolCallUpdate{ toolCallStart(agent.ToolCall{ID: "start", Name: "browser_open", Arguments: string(args)}), permissionToolCall(agent.PermissionRequest{ToolCallID: "permission", ToolName: "browser_open", Args: map[string]any{"url": rawURL}}), diff --git a/internal/specialist/resume_model_test.go b/internal/specialist/resume_model_test.go index 9710e54ac..f4f25b0b4 100644 --- a/internal/specialist/resume_model_test.go +++ b/internal/specialist/resume_model_test.go @@ -229,8 +229,9 @@ func TestRunResumePassesTheParentModelThroughToTheChild(t *testing.T) { Prompt: "keep going", Resume: "child_task", }, TaskRunOptions{ - ParentSessionID: parent.SessionID, - ParentModel: "claude-opus-4.1", + ParentSessionID: parent.SessionID, + ParentModel: "claude-opus-4.1", + ParentReasoningEffort: "high", }); err != nil { t.Fatalf("Run(resume): %v", err) } @@ -246,6 +247,9 @@ func TestRunResumePassesTheParentModelThroughToTheChild(t *testing.T) { if model != "claude-opus-4.1" { t.Fatalf("the resumed child was launched with --model %q, want the parent's %q", model, "claude-opus-4.1") } + if effort, ok := argValue(captured, "--reasoning-effort"); !ok || effort != "high" { + t.Fatalf("the resumed child was launched with --reasoning-effort %q (present=%t), want the parent's", effort, ok) + } } // THE FRESH CALL SITE NEEDS THE SAME GUARD, FOR THE SAME REASON. diff --git a/internal/tui/dictation.go b/internal/tui/dictation.go index adac424f7..9ab48592d 100644 --- a/internal/tui/dictation.go +++ b/internal/tui/dictation.go @@ -77,9 +77,12 @@ type dictationController struct { regionStart int regionEnd int regionPrefix string + // regionRendered is the exact text last inserted into [regionStart, + // regionEnd). A later partial or cancel may remove that range only while it + // still matches, so stale bounds never consume user replacement text. + regionRendered string // regionAnchor snapshots the text BEFORE the live region, so the next - // partial can detect external edits (typing, paste) and shift [start,end) - // to stay aligned. Updated alongside regionStart on each render. + // partial can detect edits before the tracked range. regionAnchor string // waveBars is the recording waveform's recent bar heights (a scrolling ring): @@ -327,6 +330,7 @@ func (d *dictationController) reset() { d.streamStop = nil d.regionActive = false d.regionPrefix = "" + d.regionRendered = "" d.waveBars = nil d.waveTick = 0 } diff --git a/internal/tui/dictation_stream.go b/internal/tui/dictation_stream.go index 5e994eedd..2c81a6cfd 100644 --- a/internal/tui/dictation_stream.go +++ b/internal/tui/dictation_stream.go @@ -2,7 +2,6 @@ package tui import ( "context" - "strings" tea "charm.land/bubbletea/v2" @@ -92,52 +91,30 @@ func (m model) handleDictationPartial(msg sttPartialMsg) model { func (m *model) applyStreamingText(text string) { state := m.currentComposerState() + stateRunes := []rune(state.text) if !m.dictation.regionActive { m.dictation.regionActive = true m.dictation.regionStart = state.cursor m.dictation.regionEnd = state.cursor m.dictation.regionPrefix = "" - // Anchor the prefix text BEFORE the live region. If the user types or - // pastes outside the region, the next partial can detect the change in - // this prefix and shift [start,end) to stay aligned. - m.dictation.regionAnchor = state.text + // Anchor the prefix text before the live region. A later partial removes + // the tracked range only while both this prefix and the rendered region + // still match. + m.dictation.regionAnchor = string(stateRunes[:state.cursor]) if needsLeadingSpace(state) { // Fold the separator into the region so a cancel removes it too. m.dictation.regionPrefix = " " } - } else { - // Compare the prefix before the live region. If it changed (the user - // typed/pasted there) shift [start,end) by the length delta so the - // next partial's slice targets the right span. If the user edited - // INSIDE the region, drop the live region and re-anchor at the - // current cursor — we can't tell the partial from the user's text - // after that point. - prefix := string([]rune(state.text)[:m.dictation.regionStart]) - anchor := m.dictation.regionAnchor - switch { - case prefix == anchor: - // External edit at or after the region — no shift needed. - case len(prefix) > len(anchor) && strings.HasPrefix(prefix, anchor): - // External edit inserted text just before the region (i.e. at - // position regionStart, pushing the region right). - delta := len([]rune(prefix)) - len([]rune(anchor)) - m.dictation.regionStart += delta - m.dictation.regionEnd += delta - m.dictation.regionAnchor = prefix - case len(anchor) > len(prefix) && strings.HasPrefix(anchor, prefix): - // External delete just before the region. - delta := len([]rune(anchor)) - len([]rune(prefix)) - m.dictation.regionStart -= delta - m.dictation.regionEnd -= delta - m.dictation.regionAnchor = prefix - default: - // User edited inside or across the region — can't safely - // overwrite. Drop the live region and re-anchor at the cursor - // so the next partial inserts as a fresh span. - m.dictation.regionStart = state.cursor - m.dictation.regionEnd = state.cursor - m.dictation.regionPrefix = "" - m.dictation.regionAnchor = string([]rune(state.text)[:state.cursor]) + } else if !m.dictation.liveRegionMatches(stateRunes) { + // The composer changed before or inside the tracked range. Its old + // bounds no longer prove which bytes belong to dictation, so preserve + // everything and insert the next partial as a fresh live region. + m.dictation.regionStart = state.cursor + m.dictation.regionEnd = state.cursor + m.dictation.regionPrefix = "" + m.dictation.regionAnchor = string(stateRunes[:state.cursor]) + if needsLeadingSpace(state) { + m.dictation.regionPrefix = " " } } // Replace [regionStart, regionEnd) with prefix + the new cumulative text. @@ -153,14 +130,22 @@ func (m *model) applyStreamingText(text string) { } updated := insertComposerText(cleared, rendered) m.dictation.regionEnd = m.dictation.regionStart + len([]rune(rendered)) + m.dictation.regionRendered = rendered m.setComposerState(updated) } +func (d dictationController) liveRegionMatches(stateRunes []rune) bool { + return d.regionStart >= 0 && d.regionEnd >= d.regionStart && d.regionEnd <= len(stateRunes) && + string(stateRunes[:d.regionStart]) == d.regionAnchor && + string(stateRunes[d.regionStart:d.regionEnd]) == d.regionRendered +} + // commitDictationRegion keeps the streamed text in the composer and stops // tracking it as a live region (used on successful completion — the final // transcript equals the last partial already rendered). func (m model) commitDictationRegion() model { m.dictation.regionActive = false + m.dictation.regionRendered = "" return m } @@ -169,8 +154,11 @@ func (m model) commitDictationRegion() model { func (m model) discardDictationRegion() model { if m.dictation.regionActive { state := m.currentComposerState() - m.setComposerState(deleteComposerRange(state, m.dictation.regionStart, m.dictation.regionEnd)) + if m.dictation.liveRegionMatches([]rune(state.text)) { + m.setComposerState(deleteComposerRange(state, m.dictation.regionStart, m.dictation.regionEnd)) + } m.dictation.regionActive = false + m.dictation.regionRendered = "" } return m } diff --git a/internal/tui/dictation_test.go b/internal/tui/dictation_test.go index b164a826c..b02a87ade 100644 --- a/internal/tui/dictation_test.go +++ b/internal/tui/dictation_test.go @@ -3,6 +3,7 @@ package tui import ( "context" "errors" + "fmt" "strings" "testing" @@ -175,6 +176,145 @@ func TestDictationStreamingPartialReplacesRegion(t *testing.T) { } } +func TestDictationStreamingPartialWithCaretInsideExistingText(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "hello world", cursor: 5}) + m.dictation.phase = dictRecording + m.dictation.streaming = true + + m = m.handleDictationPartial(sttPartialMsg{text: "there"}) + if m.composer.text != "hello there world" { + t.Fatalf("after first partial: %q", m.composer.text) + } + m = m.handleDictationPartial(sttPartialMsg{text: "there friend"}) + m = m.handleDictationPartial(sttPartialMsg{text: "there friend again"}) + if m.composer.text != "hello there friend again world" { + t.Fatalf("after third partial: %q", m.composer.text) + } +} + +func TestDictationStreamingPartialReanchorsRegionAfterComposerShrink(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "hello world", cursor: 11}) + m.dictation.phase = dictRecording + m.dictation.streaming = true + m = m.handleDictationPartial(sttPartialMsg{text: "there"}) + + // Simulate the user replacing the composer while a later partial is in + // flight. The old live-region bounds now exceed the new composer length. + m.setComposerState(composerState{text: "hi ", cursor: 3}) + m = m.handleDictationPartial(sttPartialMsg{text: "again"}) + if m.composer.text != "hi again" { + t.Fatalf("partial after composer shrink: %q", m.composer.text) + } +} + +func TestDictationStreamingPartialDoesNotClampStaleRegionIntoUserText(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "aOLDz", cursor: 1}) + m.dictation.phase = dictRecording + m.dictation.streaming = true + m = m.handleDictationPartial(sttPartialMsg{text: "first"}) + + // Replace the composer while another cumulative partial is in flight. The + // old region starts after the same prefix but no longer contains dictation + // text, so its clamped bounds must not consume the user's "b". + m.setComposerState(composerState{text: "ab", cursor: 2}) + m = m.handleDictationPartial(sttPartialMsg{text: "next"}) + if m.composer.text != "ab next" { + t.Fatalf("stale live region replaced user text: %q", m.composer.text) + } +} + +func TestDictationStreamingCancelDoesNotClampStaleRegionIntoUserText(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "aOLDz", cursor: 1}) + m.dictation.phase = dictRecording + m.dictation.streaming = true + m = m.handleDictationPartial(sttPartialMsg{text: "first"}) + + m.setComposerState(composerState{text: "ab", cursor: 2}) + m = m.discardDictationRegion() + if m.composer.text != "ab" { + t.Fatalf("discarded stale live region from user text: %q", m.composer.text) + } +} + +func TestDictationStreamingSameLengthRegionEditPreserved(t *testing.T) { + for _, partial := range []bool{false, true} { + t.Run(fmt.Sprintf("later partial=%v", partial), func(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "hello world", cursor: 11}) + m.applyStreamingText("there") + // Keep the prefix and rune bounds unchanged, but replace precisely + // the rendered region with user text. + m.setComposerState(composerState{text: "hello world MINE!", cursor: 17}) + if partial { + m.applyStreamingText("next") + if !strings.Contains(m.composer.text, " MINE!") { + t.Fatalf("partial deleted same-length user edit: %q", m.composer.text) + } + } + m = m.discardDictationRegion() + if m.composer.text != "hello world MINE!" { + t.Fatalf("cancel deleted same-length user edit: %q", m.composer.text) + } + }) + } +} + +func TestDictationStreamingBackspaceAcrossRegionPreservesUserTextOnCancel(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "hello world", cursor: 11}) + m.dictation.phase = dictRecording + m.dictation.streaming = true + m = m.handleDictationPartial(sttPartialMsg{text: "there"}) + + // The user backspaces through the live transcript and into their original + // text while newer cumulative partials are still in flight. + m.setComposerState(composerState{text: "hello wor", cursor: 9}) + m = m.handleDictationPartial(sttPartialMsg{text: "there friend"}) + m = m.handleDictationPartial(sttPartialMsg{text: "there friend again"}) + if m.composer.text != "hello wor there friend again" { + t.Fatalf("partials replaced user text after backspace: %q", m.composer.text) + } + + m, _ = m.cancelDictation() + if m.composer.text != "hello wor" { + t.Fatalf("cancel did not restore the user's edited text: %q", m.composer.text) + } +} + +func TestDictationStreamingInvalidRegionBoundsReanchor(t *testing.T) { + tests := []struct { + name string + start int + end int + anchor string + want string + }{ + {name: "start past end", start: 40, end: 60, anchor: "a much longer previous composer", want: "ab partial text"}, + {name: "negative start", start: -3, end: 1, anchor: "", want: "ab partial text"}, + {name: "end past end", start: 1, end: 40, anchor: "a", want: "ab partial text"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := model{} + m.setComposerState(composerState{text: "ab", cursor: 2}) + m.dictation.regionActive = true + m.dictation.regionStart = tt.start + m.dictation.regionEnd = tt.end + m.dictation.regionAnchor = tt.anchor + + m.applyStreamingText("partial text") + if m.composer.text != tt.want { + t.Fatalf("composer = %q, want %q", m.composer.text, tt.want) + } + }) + } +} + 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