Merge branch 'main' into fix/editproperly
4 files · +128 -6
M packages/provider/github_copilot.go +1 -0
| @@ -175,6 +175,7 @@ func (t *copilotRefreshTransport) RoundTrip(req *http.Request) (*http.Response, | |||
| 175 | 175 | if u, err := url.Parse(tok.baseURL); err == nil && u.Host != clone.URL.Host { | |
| 176 | 176 | clone.URL.Scheme = u.Scheme | |
| 177 | 177 | clone.URL.Host = u.Host | |
| 178 | + | clone.Host = u.Host | |
| 178 | 179 | } | |
| 179 | 180 | } | |
| 180 | 181 | return t.inner.RoundTrip(clone) | |
| @@ -175,6 +175,7 @@ func (t *copilotRefreshTransport) RoundTrip(req *http.Request) (*http.Response, | |||||
| 175 | if u, err := url.Parse(tok.baseURL); err == nil && u.Host != clone.URL.Host { | 175 | if u, err := url.Parse(tok.baseURL); err == nil && u.Host != clone.URL.Host { | ||
| 176 | clone.URL.Scheme = u.Scheme | 176 | clone.URL.Scheme = u.Scheme | ||
| 177 | clone.URL.Host = u.Host | 177 | clone.URL.Host = u.Host | ||
| 178 | + | clone.Host = u.Host | |||
| 178 | } | 179 | } | ||
| 179 | } | 180 | } | ||
| 180 | return t.inner.RoundTrip(clone) | 181 | return t.inner.RoundTrip(clone) | ||
A packages/provider/github_copilot_test.go +70 -0
| @@ -0,0 +1,70 @@ | |||
| 1 | + | package provider | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "net/http" | |
| 6 | + | "net/url" | |
| 7 | + | "testing" | |
| 8 | + | "time" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | type mockRoundTripper struct { | |
| 12 | + | lastReq *http.Request | |
| 13 | + | } | |
| 14 | + | ||
| 15 | + | func (m *mockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { | |
| 16 | + | m.lastReq = req | |
| 17 | + | return &http.Response{ | |
| 18 | + | StatusCode: http.StatusOK, | |
| 19 | + | Body: http.NoBody, | |
| 20 | + | }, nil | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func TestCopilotHostRewrite(t *testing.T) { | |
| 24 | + | // Seed the cache with a mock token that carries a non-default proxy endpoint | |
| 25 | + | // Ensure the expiresAt time is far in the future so that it bypasses the exchange check | |
| 26 | + | const pat = "ghp_mock_token_for_test" | |
| 27 | + | copilotCache.mu.Lock() | |
| 28 | + | copilotCache.tokens[pat] = copilotToken{ | |
| 29 | + | value: "mock-jwt-with;proxy-ep=enterprise.copilot.proxy", | |
| 30 | + | expiresAt: time.Now().Add(1 * time.Hour), | |
| 31 | + | baseURL: "https://api.enterprise.copilot.proxy", | |
| 32 | + | } | |
| 33 | + | copilotCache.mu.Unlock() | |
| 34 | + | ||
| 35 | + | mockRT := &mockRoundTripper{} | |
| 36 | + | transport := &copilotRefreshTransport{ | |
| 37 | + | inner: mockRT, | |
| 38 | + | pat: pat, | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | reqUrl, err := url.Parse("https://api.individual.githubcopilot.com/chat/completions") | |
| 42 | + | if err != nil { | |
| 43 | + | t.Fatalf("failed to parse test URL: %v", err) | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | req := &http.Request{ | |
| 47 | + | Method: "POST", | |
| 48 | + | URL: reqUrl, | |
| 49 | + | Header: make(http.Header), | |
| 50 | + | } | |
| 51 | + | req = req.WithContext(context.Background()) | |
| 52 | + | ||
| 53 | + | _, err = transport.RoundTrip(req) | |
| 54 | + | if err != nil { | |
| 55 | + | t.Fatalf("RoundTrip failed: %v", err) | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | if mockRT.lastReq == nil { | |
| 59 | + | t.Fatal("RoundTrip did not forward the request") | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | // Verify both URL host and Host header are rewritten correctly to prevent 421 Misdirected Request | |
| 63 | + | wantHost := "api.enterprise.copilot.proxy" | |
| 64 | + | if mockRT.lastReq.URL.Host != wantHost { | |
| 65 | + | t.Errorf("expected URL.Host to be %q, got %q", wantHost, mockRT.lastReq.URL.Host) | |
| 66 | + | } | |
| 67 | + | if mockRT.lastReq.Host != wantHost { | |
| 68 | + | t.Errorf("expected Host header (Request.Host) to be %q, got %q", wantHost, mockRT.lastReq.Host) | |
| 69 | + | } | |
| 70 | + | } | |
| @@ -0,0 +1,70 @@ | |||||
| 1 | + | package provider | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "net/http" | |||
| 6 | + | "net/url" | |||
| 7 | + | "testing" | |||
| 8 | + | "time" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | type mockRoundTripper struct { | |||
| 12 | + | lastReq *http.Request | |||
| 13 | + | } | |||
| 14 | + | ||||
| 15 | + | func (m *mockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { | |||
| 16 | + | m.lastReq = req | |||
| 17 | + | return &http.Response{ | |||
| 18 | + | StatusCode: http.StatusOK, | |||
| 19 | + | Body: http.NoBody, | |||
| 20 | + | }, nil | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | func TestCopilotHostRewrite(t *testing.T) { | |||
| 24 | + | // Seed the cache with a mock token that carries a non-default proxy endpoint | |||
| 25 | + | // Ensure the expiresAt time is far in the future so that it bypasses the exchange check | |||
| 26 | + | const pat = "ghp_mock_token_for_test" | |||
| 27 | + | copilotCache.mu.Lock() | |||
| 28 | + | copilotCache.tokens[pat] = copilotToken{ | |||
| 29 | + | value: "mock-jwt-with;proxy-ep=enterprise.copilot.proxy", | |||
| 30 | + | expiresAt: time.Now().Add(1 * time.Hour), | |||
| 31 | + | baseURL: "https://api.enterprise.copilot.proxy", | |||
| 32 | + | } | |||
| 33 | + | copilotCache.mu.Unlock() | |||
| 34 | + | ||||
| 35 | + | mockRT := &mockRoundTripper{} | |||
| 36 | + | transport := &copilotRefreshTransport{ | |||
| 37 | + | inner: mockRT, | |||
| 38 | + | pat: pat, | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | reqUrl, err := url.Parse("https://api.individual.githubcopilot.com/chat/completions") | |||
| 42 | + | if err != nil { | |||
| 43 | + | t.Fatalf("failed to parse test URL: %v", err) | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | req := &http.Request{ | |||
| 47 | + | Method: "POST", | |||
| 48 | + | URL: reqUrl, | |||
| 49 | + | Header: make(http.Header), | |||
| 50 | + | } | |||
| 51 | + | req = req.WithContext(context.Background()) | |||
| 52 | + | ||||
| 53 | + | _, err = transport.RoundTrip(req) | |||
| 54 | + | if err != nil { | |||
| 55 | + | t.Fatalf("RoundTrip failed: %v", err) | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | if mockRT.lastReq == nil { | |||
| 59 | + | t.Fatal("RoundTrip did not forward the request") | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | // Verify both URL host and Host header are rewritten correctly to prevent 421 Misdirected Request | |||
| 63 | + | wantHost := "api.enterprise.copilot.proxy" | |||
| 64 | + | if mockRT.lastReq.URL.Host != wantHost { | |||
| 65 | + | t.Errorf("expected URL.Host to be %q, got %q", wantHost, mockRT.lastReq.URL.Host) | |||
| 66 | + | } | |||
| 67 | + | if mockRT.lastReq.Host != wantHost { | |||
| 68 | + | t.Errorf("expected Host header (Request.Host) to be %q, got %q", wantHost, mockRT.lastReq.Host) | |||
| 69 | + | } | |||
| 70 | + | } | |||
M packages/tui/render.go +15 -6
| @@ -483,7 +483,7 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||
| 483 | 483 | w.WriteString(SeqSynchronizedOn) | |
| 484 | 484 | w.WriteString(SeqHideCursor) | |
| 485 | 485 | ||
| 486 | - | writeFull := func(clear bool) { | |
| 486 | + | writeFull := func(clear, purgeScrollback bool) { | |
| 487 | 487 | if clear { | |
| 488 | 488 | w.WriteString(SeqDeleteKittyImages) | |
| 489 | 489 | if r.keepScrollback { | |
| @@ -500,7 +500,13 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||
| 500 | 500 | w.WriteString(SeqClearToEnd) | |
| 501 | 501 | } else { | |
| 502 | 502 | w.WriteString(SeqClearScreenNoHome) | |
| 503 | - | w.WriteString(r.clearScrollbackSeq()) | |
| 503 | + | // Purging scrollback invalidates native terminal selections. | |
| 504 | + | // It is safe on the first paint because no zot output can be | |
| 505 | + | // selected yet, but implicit recovery repaints happen while an | |
| 506 | + | // agent is streaming and must preserve the user's selection. | |
| 507 | + | if purgeScrollback { | |
| 508 | + | w.WriteString(r.clearScrollbackSeq()) | |
| 509 | + | } | |
| 504 | 510 | w.WriteString(MoveTo(1, 1)) | |
| 505 | 511 | } | |
| 506 | 512 | } | |
| @@ -576,9 +582,10 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||
| 576 | 582 | // because the cached entry was the \x00 sentinel rather than the | |
| 577 | 583 | // real previous bg-colored row. | |
| 578 | 584 | ||
| 579 | - | full := !r.logInit || len(r.logLines) == 0 | |
| 585 | + | wasInitialized := r.logInit | |
| 586 | + | full := !wasInitialized || len(r.logLines) == 0 | |
| 580 | 587 | if full { | |
| 581 | - | writeFull(true) | |
| 588 | + | writeFull(true, !wasInitialized) | |
| 582 | 589 | r.logInit = true | |
| 583 | 590 | } else { | |
| 584 | 591 | firstChanged := -1 | |
| @@ -623,7 +630,9 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||
| 623 | 630 | // move the hardware cursor if the editor cursor changed. | |
| 624 | 631 | } else if firstChanged < r.logViewportTop { | |
| 625 | 632 | // Changes above the visible viewport cannot be patched safely. | |
| 626 | - | writeFull(true) | |
| 633 | + | // Repaint without erasing scrollback: the terminal may have a | |
| 634 | + | // native text selection anchored there while output streams. | |
| 635 | + | writeFull(true, false) | |
| 627 | 636 | } else { | |
| 628 | 637 | prevViewportTop := r.logViewportTop | |
| 629 | 638 | viewportTop := prevViewportTop | |
| @@ -686,7 +695,7 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||
| 686 | 695 | if len(r.logLines) > len(lines) { | |
| 687 | 696 | extra := len(r.logLines) - len(lines) | |
| 688 | 697 | if extra > r.rows { | |
| 689 | - | writeFull(true) | |
| 698 | + | writeFull(true, false) | |
| 690 | 699 | } else { | |
| 691 | 700 | for e := 0; e < extra; e++ { | |
| 692 | 701 | w.WriteString("\x1b[1B") | |
| @@ -483,7 +483,7 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||||
| 483 | w.WriteString(SeqSynchronizedOn) | 483 | w.WriteString(SeqSynchronizedOn) | ||
| 484 | w.WriteString(SeqHideCursor) | 484 | w.WriteString(SeqHideCursor) | ||
| 485 | 485 | ||||
| 486 | - | writeFull := func(clear bool) { | 486 | + | writeFull := func(clear, purgeScrollback bool) { |
| 487 | if clear { | 487 | if clear { | ||
| 488 | w.WriteString(SeqDeleteKittyImages) | 488 | w.WriteString(SeqDeleteKittyImages) | ||
| 489 | if r.keepScrollback { | 489 | if r.keepScrollback { | ||
| @@ -500,7 +500,13 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||||
| 500 | w.WriteString(SeqClearToEnd) | 500 | w.WriteString(SeqClearToEnd) | ||
| 501 | } else { | 501 | } else { | ||
| 502 | w.WriteString(SeqClearScreenNoHome) | 502 | w.WriteString(SeqClearScreenNoHome) | ||
| 503 | - | w.WriteString(r.clearScrollbackSeq()) | 503 | + | // Purging scrollback invalidates native terminal selections. |
| 504 | + | // It is safe on the first paint because no zot output can be | |||
| 505 | + | // selected yet, but implicit recovery repaints happen while an | |||
| 506 | + | // agent is streaming and must preserve the user's selection. | |||
| 507 | + | if purgeScrollback { | |||
| 508 | + | w.WriteString(r.clearScrollbackSeq()) | |||
| 509 | + | } | |||
| 504 | w.WriteString(MoveTo(1, 1)) | 510 | w.WriteString(MoveTo(1, 1)) | ||
| 505 | } | 511 | } | ||
| 506 | } | 512 | } | ||
| @@ -576,9 +582,10 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||||
| 576 | // because the cached entry was the \x00 sentinel rather than the | 582 | // because the cached entry was the \x00 sentinel rather than the | ||
| 577 | // real previous bg-colored row. | 583 | // real previous bg-colored row. | ||
| 578 | 584 | ||||
| 579 | - | full := !r.logInit || len(r.logLines) == 0 | 585 | + | wasInitialized := r.logInit |
| 586 | + | full := !wasInitialized || len(r.logLines) == 0 | |||
| 580 | if full { | 587 | if full { | ||
| 581 | - | writeFull(true) | 588 | + | writeFull(true, !wasInitialized) |
| 582 | r.logInit = true | 589 | r.logInit = true | ||
| 583 | } else { | 590 | } else { | ||
| 584 | firstChanged := -1 | 591 | firstChanged := -1 | ||
| @@ -623,7 +630,9 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||||
| 623 | // move the hardware cursor if the editor cursor changed. | 630 | // move the hardware cursor if the editor cursor changed. | ||
| 624 | } else if firstChanged < r.logViewportTop { | 631 | } else if firstChanged < r.logViewportTop { | ||
| 625 | // Changes above the visible viewport cannot be patched safely. | 632 | // Changes above the visible viewport cannot be patched safely. | ||
| 626 | - | writeFull(true) | 633 | + | // Repaint without erasing scrollback: the terminal may have a |
| 634 | + | // native text selection anchored there while output streams. | |||
| 635 | + | writeFull(true, false) | |||
| 627 | } else { | 636 | } else { | ||
| 628 | prevViewportTop := r.logViewportTop | 637 | prevViewportTop := r.logViewportTop | ||
| 629 | viewportTop := prevViewportTop | 638 | viewportTop := prevViewportTop | ||
| @@ -686,7 +695,7 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||||
| 686 | if len(r.logLines) > len(lines) { | 695 | if len(r.logLines) > len(lines) { | ||
| 687 | extra := len(r.logLines) - len(lines) | 696 | extra := len(r.logLines) - len(lines) | ||
| 688 | if extra > r.rows { | 697 | if extra > r.rows { | ||
| 689 | - | writeFull(true) | 698 | + | writeFull(true, false) |
| 690 | } else { | 699 | } else { | ||
| 691 | for e := 0; e < extra; e++ { | 700 | for e := 0; e < extra; e++ { | ||
| 692 | w.WriteString("\x1b[1B") | 701 | w.WriteString("\x1b[1B") | ||
M packages/tui/render_test.go +42 -0
| @@ -101,3 +101,45 @@ func TestDrawLogResizeForcesFullRedraw(t *testing.T) { | |||
| 101 | 101 | t.Fatal("post-resize redraw skipped; the new frame would never reach the terminal") | |
| 102 | 102 | } | |
| 103 | 103 | } | |
| 104 | + | ||
| 105 | + | // TestDrawLogRecoveryPreservesScrollbackSelection covers long streaming | |
| 106 | + | // output whose changing first row has already scrolled above the viewport. | |
| 107 | + | // DrawLog must repaint in that case, but erasing terminal scrollback also | |
| 108 | + | // erases the text backing any native mouse selection the user is making. | |
| 109 | + | func TestDrawLogRecoveryPreservesScrollbackSelection(t *testing.T) { | |
| 110 | + | t.Setenv("TERM_PROGRAM", "") | |
| 111 | + | var buf bytes.Buffer | |
| 112 | + | r := NewRenderer(&buf) | |
| 113 | + | r.Resize(80, 3) | |
| 114 | + | r.DrawLog([]string{"partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 115 | + | buf.Reset() | |
| 116 | + | ||
| 117 | + | // The changed row is above logViewportTop, forcing the recovery repaint | |
| 118 | + | // used while a long streamed response continues to be reformatted. | |
| 119 | + | r.DrawLog([]string{"partial response", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 120 | + | got := buf.String() | |
| 121 | + | if !strings.Contains(got, SeqClearScreenNoHome) { | |
| 122 | + | t.Fatalf("recovery path did not repaint the screen: %q", got) | |
| 123 | + | } | |
| 124 | + | if strings.Contains(got, SeqClearScrollback) { | |
| 125 | + | t.Fatalf("recovery repaint erased scrollback and native selection: %q", got) | |
| 126 | + | } | |
| 127 | + | } | |
| 128 | + | ||
| 129 | + | // TestDrawLogInvalidationPreservesScrollbackSelection pins the same rule for | |
| 130 | + | // cache invalidations, which can happen during an active turn independently | |
| 131 | + | // of an inaccessible changed row. | |
| 132 | + | func TestDrawLogInvalidationPreservesScrollbackSelection(t *testing.T) { | |
| 133 | + | t.Setenv("TERM_PROGRAM", "") | |
| 134 | + | var buf bytes.Buffer | |
| 135 | + | r := NewRenderer(&buf) | |
| 136 | + | r.Resize(80, 3) | |
| 137 | + | r.DrawLog([]string{"one", "two", "three", "four"}, []string{"input"}, 0, 0) | |
| 138 | + | buf.Reset() | |
| 139 | + | ||
| 140 | + | r.Invalidate() | |
| 141 | + | r.DrawLog([]string{"one", "two", "three", "four"}, []string{"input"}, 0, 0) | |
| 142 | + | if got := buf.String(); strings.Contains(got, SeqClearScrollback) { | |
| 143 | + | t.Fatalf("invalidation repaint erased scrollback and native selection: %q", got) | |
| 144 | + | } | |
| 145 | + | } | |
| @@ -101,3 +101,45 @@ func TestDrawLogResizeForcesFullRedraw(t *testing.T) { | |||||
| 101 | t.Fatal("post-resize redraw skipped; the new frame would never reach the terminal") | 101 | t.Fatal("post-resize redraw skipped; the new frame would never reach the terminal") | ||
| 102 | } | 102 | } | ||
| 103 | } | 103 | } | ||
| 104 | + | ||||
| 105 | + | // TestDrawLogRecoveryPreservesScrollbackSelection covers long streaming | |||
| 106 | + | // output whose changing first row has already scrolled above the viewport. | |||
| 107 | + | // DrawLog must repaint in that case, but erasing terminal scrollback also | |||
| 108 | + | // erases the text backing any native mouse selection the user is making. | |||
| 109 | + | func TestDrawLogRecoveryPreservesScrollbackSelection(t *testing.T) { | |||
| 110 | + | t.Setenv("TERM_PROGRAM", "") | |||
| 111 | + | var buf bytes.Buffer | |||
| 112 | + | r := NewRenderer(&buf) | |||
| 113 | + | r.Resize(80, 3) | |||
| 114 | + | r.DrawLog([]string{"partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |||
| 115 | + | buf.Reset() | |||
| 116 | + | ||||
| 117 | + | // The changed row is above logViewportTop, forcing the recovery repaint | |||
| 118 | + | // used while a long streamed response continues to be reformatted. | |||
| 119 | + | r.DrawLog([]string{"partial response", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |||
| 120 | + | got := buf.String() | |||
| 121 | + | if !strings.Contains(got, SeqClearScreenNoHome) { | |||
| 122 | + | t.Fatalf("recovery path did not repaint the screen: %q", got) | |||
| 123 | + | } | |||
| 124 | + | if strings.Contains(got, SeqClearScrollback) { | |||
| 125 | + | t.Fatalf("recovery repaint erased scrollback and native selection: %q", got) | |||
| 126 | + | } | |||
| 127 | + | } | |||
| 128 | + | ||||
| 129 | + | // TestDrawLogInvalidationPreservesScrollbackSelection pins the same rule for | |||
| 130 | + | // cache invalidations, which can happen during an active turn independently | |||
| 131 | + | // of an inaccessible changed row. | |||
| 132 | + | func TestDrawLogInvalidationPreservesScrollbackSelection(t *testing.T) { | |||
| 133 | + | t.Setenv("TERM_PROGRAM", "") | |||
| 134 | + | var buf bytes.Buffer | |||
| 135 | + | r := NewRenderer(&buf) | |||
| 136 | + | r.Resize(80, 3) | |||
| 137 | + | r.DrawLog([]string{"one", "two", "three", "four"}, []string{"input"}, 0, 0) | |||
| 138 | + | buf.Reset() | |||
| 139 | + | ||||
| 140 | + | r.Invalidate() | |||
| 141 | + | r.DrawLog([]string{"one", "two", "three", "four"}, []string{"input"}, 0, 0) | |||
| 142 | + | if got := buf.String(); strings.Contains(got, SeqClearScrollback) { | |||
| 143 | + | t.Fatalf("invalidation repaint erased scrollback and native selection: %q", got) | |||
| 144 | + | } | |||
| 145 | + | } | |||