fix(navigation): limit external launch prompts to one per pane
A pane keeps at most one pending external-launch confirmation, and a confirmation answered after its pane was released or rebound no longer launches anything.
- commit
- 65f79a87552fdbb86d2127e28070577e9c74968f
- author
- brice · · 2026-10-15 06:09 UTC
- parent
- f6fdf4d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +63 -26
M internal/ui/coordinator/content/coordinator.go +3 -4
| @@ -138,11 +138,10 @@ type Coordinator struct { | |||
| 138 | 138 | // respond must be called exactly once on the GTK thread. | |
| 139 | 139 | onConfirmExternalLaunch func(paneID entity.PaneID, uri string, respond func(allowed bool)) | |
| 140 | 140 | ||
| 141 | - | // externalPromptMu guards externalPrompts, the external URI currently | |
| 142 | - | // awaiting confirmation per pane. It suppresses duplicate prompts when the | |
| 143 | - | // engine reports the same address change more than once. | |
| 141 | + | // externalPromptMu guards externalPrompts: panes with an outstanding | |
| 142 | + | // external-launch prompt, mapped to the WebView that asked. | |
| 144 | 143 | externalPromptMu sync.Mutex | |
| 145 | - | externalPrompts map[entity.PaneID]string | |
| 144 | + | externalPrompts map[entity.PaneID]port.WebView | |
| 146 | 145 | } | |
| 147 | 146 | ||
| 148 | 147 | type pendingThemeUpdate struct { | |
| @@ -138,11 +138,10 @@ type Coordinator struct { | |||||
| 138 | // respond must be called exactly once on the GTK thread. | 138 | // respond must be called exactly once on the GTK thread. | ||
| 139 | onConfirmExternalLaunch func(paneID entity.PaneID, uri string, respond func(allowed bool)) | 139 | onConfirmExternalLaunch func(paneID entity.PaneID, uri string, respond func(allowed bool)) | ||
| 140 | 140 | ||||
| 141 | - | // externalPromptMu guards externalPrompts, the external URI currently | 141 | + | // externalPromptMu guards externalPrompts: panes with an outstanding |
| 142 | - | // awaiting confirmation per pane. It suppresses duplicate prompts when the | 142 | + | // external-launch prompt, mapped to the WebView that asked. |
| 143 | - | // engine reports the same address change more than once. | |||
| 144 | externalPromptMu sync.Mutex | 143 | externalPromptMu sync.Mutex | ||
| 145 | - | externalPrompts map[entity.PaneID]string | 144 | + | externalPrompts map[entity.PaneID]port.WebView |
| 146 | } | 145 | } | ||
| 147 | 146 | ||||
| 148 | type pendingThemeUpdate struct { | 147 | type pendingThemeUpdate struct { | ||
M internal/ui/coordinator/content/lifecycle.go +1 -0
| @@ -49,6 +49,7 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||
| 49 | 49 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | |
| 50 | 50 | log := logging.FromContext(ctx) | |
| 51 | 51 | c.ensurePopupManager().cancelDeferredPopupsForParent(paneID) | |
| 52 | + | c.clearExternalPrompt(paneID) | |
| 52 | 53 | ||
| 53 | 54 | // deleteWebViewLocked clears reveal state before returning, even when a | |
| 54 | 55 | // previous cleanup already removed this pane's mapping. | |
| @@ -49,6 +49,7 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||||
| 49 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | 49 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | ||
| 50 | log := logging.FromContext(ctx) | 50 | log := logging.FromContext(ctx) | ||
| 51 | c.ensurePopupManager().cancelDeferredPopupsForParent(paneID) | 51 | c.ensurePopupManager().cancelDeferredPopupsForParent(paneID) | ||
| 52 | + | c.clearExternalPrompt(paneID) | |||
| 52 | 53 | ||||
| 53 | // deleteWebViewLocked clears reveal state before returning, even when a | 54 | // deleteWebViewLocked clears reveal state before returning, even when a | ||
| 54 | // previous cleanup already removed this pane's mapping. | 55 | // previous cleanup already removed this pane's mapping. | ||
M internal/ui/coordinator/content/navigation.go +25 -10
| @@ -387,9 +387,11 @@ func (c *Coordinator) onLinkHover(paneID entity.PaneID, uri string) { | |||
| 387 | 387 | } | |
| 388 | 388 | } | |
| 389 | 389 | ||
| 390 | - | // promptExternalLaunch asks the user once per pane and URI whether the external | |
| 391 | - | // URI may be handed to the system handler. | |
| 392 | - | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, uri string) { | |
| 390 | + | // promptExternalLaunch asks the user whether the external URI may be handed to | |
| 391 | + | // the system handler. A pane has at most one outstanding prompt: further | |
| 392 | + | // external URIs from that pane are dropped until it is answered, so a page | |
| 393 | + | // cannot flood the window's prompt queue. | |
| 394 | + | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | |
| 393 | 395 | log := logging.FromContext(ctx) | |
| 394 | 396 | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | |
| 395 | 397 | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | |
| @@ -397,32 +399,45 @@ func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.Pa | |||
| 397 | 399 | } | |
| 398 | 400 | ||
| 399 | 401 | c.externalPromptMu.Lock() | |
| 400 | - | if c.externalPrompts[paneID] == uri { | |
| 402 | + | if _, pending := c.externalPrompts[paneID]; pending { | |
| 401 | 403 | c.externalPromptMu.Unlock() | |
| 402 | 404 | return | |
| 403 | 405 | } | |
| 404 | 406 | if c.externalPrompts == nil { | |
| 405 | - | c.externalPrompts = make(map[entity.PaneID]string) | |
| 407 | + | c.externalPrompts = make(map[entity.PaneID]port.WebView) | |
| 406 | 408 | } | |
| 407 | - | c.externalPrompts[paneID] = uri | |
| 409 | + | c.externalPrompts[paneID] = wv | |
| 408 | 410 | c.externalPromptMu.Unlock() | |
| 409 | 411 | ||
| 410 | 412 | c.onConfirmExternalLaunch(paneID, uri, func(allowed bool) { | |
| 411 | 413 | c.externalPromptMu.Lock() | |
| 412 | - | if c.externalPrompts[paneID] == uri { | |
| 414 | + | owner, pending := c.externalPrompts[paneID] | |
| 415 | + | // The prompt is stale when its pane was released (or rebound to | |
| 416 | + | // another WebView) while it was queued: never launch for it. | |
| 417 | + | current := pending && owner == wv | |
| 418 | + | if current { | |
| 413 | 419 | delete(c.externalPrompts, paneID) | |
| 414 | 420 | } | |
| 415 | 421 | c.externalPromptMu.Unlock() | |
| 422 | + | current = current && c.GetWebView(paneID) == wv | |
| 416 | 423 | ||
| 417 | - | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed). | |
| 424 | + | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). | |
| 418 | 425 | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | |
| 419 | 426 | Msg("external scheme launch decision") | |
| 420 | - | if allowed { | |
| 427 | + | if allowed && current { | |
| 421 | 428 | c.onLaunchExternalURL(uri) | |
| 422 | 429 | } | |
| 423 | 430 | }) | |
| 424 | 431 | } | |
| 425 | 432 | ||
| 433 | + | // clearExternalPrompt invalidates the pane's outstanding external-launch | |
| 434 | + | // prompt so a late answer cannot launch anything. | |
| 435 | + | func (c *Coordinator) clearExternalPrompt(paneID entity.PaneID) { | |
| 436 | + | c.externalPromptMu.Lock() | |
| 437 | + | delete(c.externalPrompts, paneID) | |
| 438 | + | c.externalPromptMu.Unlock() | |
| 439 | + | } | |
| 440 | + | ||
| 426 | 441 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | |
| 427 | 442 | // and SPA navigation tracking. | |
| 428 | 443 | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | |
| @@ -448,7 +463,7 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||
| 448 | 463 | // with a redirect to claude://, vscode://, ...), so the launch is never | |
| 449 | 464 | // automatic: the user must confirm it. Do not navigate back: going back | |
| 450 | 465 | // to an OAuth endpoint re-issues the redirect and loops. | |
| 451 | - | c.promptExternalLaunch(ctx, paneID, uri) | |
| 466 | + | c.promptExternalLaunch(ctx, paneID, wv, uri) | |
| 452 | 467 | return | |
| 453 | 468 | } | |
| 454 | 469 | ||
| @@ -387,9 +387,11 @@ func (c *Coordinator) onLinkHover(paneID entity.PaneID, uri string) { | |||||
| 387 | } | 387 | } | ||
| 388 | } | 388 | } | ||
| 389 | 389 | ||||
| 390 | - | // promptExternalLaunch asks the user once per pane and URI whether the external | 390 | + | // promptExternalLaunch asks the user whether the external URI may be handed to |
| 391 | - | // URI may be handed to the system handler. | 391 | + | // the system handler. A pane has at most one outstanding prompt: further |
| 392 | - | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, uri string) { | 392 | + | // external URIs from that pane are dropped until it is answered, so a page |
| 393 | + | // cannot flood the window's prompt queue. | |||
| 394 | + | func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | |||
| 393 | log := logging.FromContext(ctx) | 395 | log := logging.FromContext(ctx) | ||
| 394 | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | 396 | if c.onConfirmExternalLaunch == nil || c.onLaunchExternalURL == nil { | ||
| 395 | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | 397 | log.Info().Str("pane_id", string(paneID)).Msg("external scheme launch unavailable; blocked") | ||
| @@ -397,32 +399,45 @@ func (c *Coordinator) promptExternalLaunch(ctx context.Context, paneID entity.Pa | |||||
| 397 | } | 399 | } | ||
| 398 | 400 | ||||
| 399 | c.externalPromptMu.Lock() | 401 | c.externalPromptMu.Lock() | ||
| 400 | - | if c.externalPrompts[paneID] == uri { | 402 | + | if _, pending := c.externalPrompts[paneID]; pending { |
| 401 | c.externalPromptMu.Unlock() | 403 | c.externalPromptMu.Unlock() | ||
| 402 | return | 404 | return | ||
| 403 | } | 405 | } | ||
| 404 | if c.externalPrompts == nil { | 406 | if c.externalPrompts == nil { | ||
| 405 | - | c.externalPrompts = make(map[entity.PaneID]string) | 407 | + | c.externalPrompts = make(map[entity.PaneID]port.WebView) |
| 406 | } | 408 | } | ||
| 407 | - | c.externalPrompts[paneID] = uri | 409 | + | c.externalPrompts[paneID] = wv |
| 408 | c.externalPromptMu.Unlock() | 410 | c.externalPromptMu.Unlock() | ||
| 409 | 411 | ||||
| 410 | c.onConfirmExternalLaunch(paneID, uri, func(allowed bool) { | 412 | c.onConfirmExternalLaunch(paneID, uri, func(allowed bool) { | ||
| 411 | c.externalPromptMu.Lock() | 413 | c.externalPromptMu.Lock() | ||
| 412 | - | if c.externalPrompts[paneID] == uri { | 414 | + | owner, pending := c.externalPrompts[paneID] |
| 415 | + | // The prompt is stale when its pane was released (or rebound to | |||
| 416 | + | // another WebView) while it was queued: never launch for it. | |||
| 417 | + | current := pending && owner == wv | |||
| 418 | + | if current { | |||
| 413 | delete(c.externalPrompts, paneID) | 419 | delete(c.externalPrompts, paneID) | ||
| 414 | } | 420 | } | ||
| 415 | c.externalPromptMu.Unlock() | 421 | c.externalPromptMu.Unlock() | ||
| 422 | + | current = current && c.GetWebView(paneID) == wv | |||
| 416 | 423 | ||||
| 417 | - | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed). | 424 | + | log.Info().Str("pane_id", string(paneID)).Bool("allowed", allowed).Bool("stale", !current). |
| 418 | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | 425 | Str("uri", logging.TruncateURL(uri, logging.PermissionLogURLMaxLen)). | ||
| 419 | Msg("external scheme launch decision") | 426 | Msg("external scheme launch decision") | ||
| 420 | - | if allowed { | 427 | + | if allowed && current { |
| 421 | c.onLaunchExternalURL(uri) | 428 | c.onLaunchExternalURL(uri) | ||
| 422 | } | 429 | } | ||
| 423 | }) | 430 | }) | ||
| 424 | } | 431 | } | ||
| 425 | 432 | ||||
| 433 | + | // clearExternalPrompt invalidates the pane's outstanding external-launch | |||
| 434 | + | // prompt so a late answer cannot launch anything. | |||
| 435 | + | func (c *Coordinator) clearExternalPrompt(paneID entity.PaneID) { | |||
| 436 | + | c.externalPromptMu.Lock() | |||
| 437 | + | delete(c.externalPrompts, paneID) | |||
| 438 | + | c.externalPromptMu.Unlock() | |||
| 439 | + | } | |||
| 440 | + | ||||
| 426 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | 441 | // handleURIChanged handles URI changes from WebKit, including external scheme detection | ||
| 427 | // and SPA navigation tracking. | 442 | // and SPA navigation tracking. | ||
| 428 | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | 443 | func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri string) { | ||
| @@ -448,7 +463,7 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||||
| 448 | // with a redirect to claude://, vscode://, ...), so the launch is never | 463 | // with a redirect to claude://, vscode://, ...), so the launch is never | ||
| 449 | // automatic: the user must confirm it. Do not navigate back: going back | 464 | // automatic: the user must confirm it. Do not navigate back: going back | ||
| 450 | // to an OAuth endpoint re-issues the redirect and loops. | 465 | // to an OAuth endpoint re-issues the redirect and loops. | ||
| 451 | - | c.promptExternalLaunch(ctx, paneID, uri) | 466 | + | c.promptExternalLaunch(ctx, paneID, wv, uri) |
| 452 | return | 467 | return | ||
| 453 | } | 468 | } | ||
| 454 | 469 | ||||
M internal/ui/coordinator/content/navigation_security_test.go +34 -12
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | ||
| 8 | 8 | "github.com/stretchr/testify/assert" | |
| 9 | 9 | ||
| 10 | + | "github.com/bnema/dumber/internal/application/port" | |
| 10 | 11 | "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | 12 | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | 13 | ) | |
| @@ -52,7 +53,7 @@ func TestHandleURIChanged_ExternalSchemeLaunchesOnlyAfterConfirmation(t *testing | |||
| 52 | 53 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 53 | 54 | ||
| 54 | 55 | rec := &externalLaunchRecorder{} | |
| 55 | - | c := &Coordinator{} | |
| 56 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 56 | 57 | rec.install(c) | |
| 57 | 58 | ||
| 58 | 59 | uri := "claude://login/google-auth?code=abc" | |
| @@ -74,7 +75,7 @@ func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T | |||
| 74 | 75 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 75 | 76 | ||
| 76 | 77 | rec := &externalLaunchRecorder{} | |
| 77 | - | c := &Coordinator{} | |
| 78 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 78 | 79 | rec.install(c) | |
| 79 | 80 | ||
| 80 | 81 | c.handleURIChanged(ctx, "pane-1", wv, "spotify://track/123") | |
| @@ -83,29 +84,50 @@ func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T | |||
| 83 | 84 | assert.Empty(t, rec.launched) | |
| 84 | 85 | } | |
| 85 | 86 | ||
| 86 | - | func TestHandleURIChanged_DuplicateExternalURIPromptsOnce(t *testing.T) { | |
| 87 | + | func TestHandleURIChanged_OneOutstandingPromptPerPane(t *testing.T) { | |
| 87 | 88 | t.Parallel() | |
| 88 | 89 | ||
| 89 | 90 | ctx := context.Background() | |
| 90 | 91 | wv := mocks.NewMockWebView(t) | |
| 91 | - | wv.EXPECT().Stop(ctx).Return(nil).Twice() | |
| 92 | + | wv.EXPECT().Stop(ctx).Return(nil).Times(3) | |
| 92 | 93 | ||
| 93 | 94 | rec := &externalLaunchRecorder{} | |
| 94 | - | c := &Coordinator{} | |
| 95 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 95 | 96 | rec.install(c) | |
| 96 | 97 | ||
| 97 | - | uri := "claude://login/google-auth?code=abc" | |
| 98 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | |
| 99 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | |
| 100 | - | assert.Len(t, rec.prompts, 1) | |
| 98 | + | // Alternating URIs must not queue more prompts while one is pending. | |
| 99 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") | |
| 100 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") | |
| 101 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") | |
| 102 | + | assert.Equal(t, []string{"claude://a"}, rec.prompts) | |
| 101 | 103 | ||
| 102 | - | // Once answered, a later identical request may prompt again. | |
| 104 | + | // Once answered, the pane may prompt again. | |
| 103 | 105 | rec.respond(false) | |
| 104 | 106 | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 105 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | |
| 107 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") | |
| 106 | 108 | assert.Len(t, rec.prompts, 2) | |
| 107 | 109 | } | |
| 108 | 110 | ||
| 111 | + | func TestHandleURIChanged_StalePromptAfterReleaseDoesNotLaunch(t *testing.T) { | |
| 112 | + | t.Parallel() | |
| 113 | + | ||
| 114 | + | ctx := context.Background() | |
| 115 | + | wv := mocks.NewMockWebView(t) | |
| 116 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |
| 117 | + | ||
| 118 | + | rec := &externalLaunchRecorder{} | |
| 119 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 120 | + | rec.install(c) | |
| 121 | + | ||
| 122 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://login?code=x") | |
| 123 | + | // Pane closed while the prompt was queued. | |
| 124 | + | c.clearExternalPrompt("pane-1") | |
| 125 | + | delete(c.webViews, "pane-1") | |
| 126 | + | ||
| 127 | + | rec.respond(true) | |
| 128 | + | assert.Empty(t, rec.launched) | |
| 129 | + | } | |
| 130 | + | ||
| 109 | 131 | func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | |
| 110 | 132 | t.Parallel() | |
| 111 | 133 | ||
| @@ -137,7 +159,7 @@ func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) { | |||
| 137 | 159 | wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once() | |
| 138 | 160 | ||
| 139 | 161 | rec := &externalLaunchRecorder{} | |
| 140 | - | c := &Coordinator{} | |
| 162 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |
| 141 | 163 | rec.install(c) | |
| 142 | 164 | ||
| 143 | 165 | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | 7 | ||||
| 8 | "github.com/stretchr/testify/assert" | 8 | "github.com/stretchr/testify/assert" | ||
| 9 | 9 | ||||
| 10 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 10 | "github.com/bnema/dumber/internal/application/port/mocks" | 11 | "github.com/bnema/dumber/internal/application/port/mocks" | ||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 12 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 12 | ) | 13 | ) | ||
| @@ -52,7 +53,7 @@ func TestHandleURIChanged_ExternalSchemeLaunchesOnlyAfterConfirmation(t *testing | |||||
| 52 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 53 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 53 | 54 | ||||
| 54 | rec := &externalLaunchRecorder{} | 55 | rec := &externalLaunchRecorder{} | ||
| 55 | - | c := &Coordinator{} | 56 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 56 | rec.install(c) | 57 | rec.install(c) | ||
| 57 | 58 | ||||
| 58 | uri := "claude://login/google-auth?code=abc" | 59 | uri := "claude://login/google-auth?code=abc" | ||
| @@ -74,7 +75,7 @@ func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T | |||||
| 74 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 75 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 75 | 76 | ||||
| 76 | rec := &externalLaunchRecorder{} | 77 | rec := &externalLaunchRecorder{} | ||
| 77 | - | c := &Coordinator{} | 78 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 78 | rec.install(c) | 79 | rec.install(c) | ||
| 79 | 80 | ||||
| 80 | c.handleURIChanged(ctx, "pane-1", wv, "spotify://track/123") | 81 | c.handleURIChanged(ctx, "pane-1", wv, "spotify://track/123") | ||
| @@ -83,29 +84,50 @@ func TestHandleURIChanged_ExternalSchemeDeniedDoesNotLaunchOrGoBack(t *testing.T | |||||
| 83 | assert.Empty(t, rec.launched) | 84 | assert.Empty(t, rec.launched) | ||
| 84 | } | 85 | } | ||
| 85 | 86 | ||||
| 86 | - | func TestHandleURIChanged_DuplicateExternalURIPromptsOnce(t *testing.T) { | 87 | + | func TestHandleURIChanged_OneOutstandingPromptPerPane(t *testing.T) { |
| 87 | t.Parallel() | 88 | t.Parallel() | ||
| 88 | 89 | ||||
| 89 | ctx := context.Background() | 90 | ctx := context.Background() | ||
| 90 | wv := mocks.NewMockWebView(t) | 91 | wv := mocks.NewMockWebView(t) | ||
| 91 | - | wv.EXPECT().Stop(ctx).Return(nil).Twice() | 92 | + | wv.EXPECT().Stop(ctx).Return(nil).Times(3) |
| 92 | 93 | ||||
| 93 | rec := &externalLaunchRecorder{} | 94 | rec := &externalLaunchRecorder{} | ||
| 94 | - | c := &Coordinator{} | 95 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 95 | rec.install(c) | 96 | rec.install(c) | ||
| 96 | 97 | ||||
| 97 | - | uri := "claude://login/google-auth?code=abc" | 98 | + | // Alternating URIs must not queue more prompts while one is pending. |
| 98 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | 99 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") |
| 99 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | 100 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") |
| 100 | - | assert.Len(t, rec.prompts, 1) | 101 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://a") |
| 102 | + | assert.Equal(t, []string{"claude://a"}, rec.prompts) | |||
| 101 | 103 | ||||
| 102 | - | // Once answered, a later identical request may prompt again. | 104 | + | // Once answered, the pane may prompt again. |
| 103 | rec.respond(false) | 105 | rec.respond(false) | ||
| 104 | wv.EXPECT().Stop(ctx).Return(nil).Once() | 106 | wv.EXPECT().Stop(ctx).Return(nil).Once() | ||
| 105 | - | c.handleURIChanged(ctx, "pane-1", wv, uri) | 107 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://b") |
| 106 | assert.Len(t, rec.prompts, 2) | 108 | assert.Len(t, rec.prompts, 2) | ||
| 107 | } | 109 | } | ||
| 108 | 110 | ||||
| 111 | + | func TestHandleURIChanged_StalePromptAfterReleaseDoesNotLaunch(t *testing.T) { | |||
| 112 | + | t.Parallel() | |||
| 113 | + | ||||
| 114 | + | ctx := context.Background() | |||
| 115 | + | wv := mocks.NewMockWebView(t) | |||
| 116 | + | wv.EXPECT().Stop(ctx).Return(nil).Once() | |||
| 117 | + | ||||
| 118 | + | rec := &externalLaunchRecorder{} | |||
| 119 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} | |||
| 120 | + | rec.install(c) | |||
| 121 | + | ||||
| 122 | + | c.handleURIChanged(ctx, "pane-1", wv, "claude://login?code=x") | |||
| 123 | + | // Pane closed while the prompt was queued. | |||
| 124 | + | c.clearExternalPrompt("pane-1") | |||
| 125 | + | delete(c.webViews, "pane-1") | |||
| 126 | + | ||||
| 127 | + | rec.respond(true) | |||
| 128 | + | assert.Empty(t, rec.launched) | |||
| 129 | + | } | |||
| 130 | + | ||||
| 109 | func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | 131 | func TestHandleURIChanged_HTTPURIStillRecordsSPANavigation(t *testing.T) { | ||
| 110 | t.Parallel() | 132 | t.Parallel() | ||
| 111 | 133 | ||||
| @@ -137,7 +159,7 @@ func TestHandleURIChanged_StopErrorDoesNotPanic(t *testing.T) { | |||||
| 137 | wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once() | 159 | wv.EXPECT().Stop(ctx).Return(errors.New("stop failed")).Once() | ||
| 138 | 160 | ||||
| 139 | rec := &externalLaunchRecorder{} | 161 | rec := &externalLaunchRecorder{} | ||
| 140 | - | c := &Coordinator{} | 162 | + | c := &Coordinator{webViews: map[entity.PaneID]port.WebView{"pane-1": wv}} |
| 141 | rec.install(c) | 163 | rec.install(c) | ||
| 142 | 164 | ||||
| 143 | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | 165 | c.handleURIChanged(ctx, "pane-1", wv, "vscode://file/etc/passwd") | ||