Reuse shared runtime for WebKit downloads
Agent-Logs-Url: https://github.com/bnema/dumber/sessions/1efb54e2-29d8-4b77-924a-ea1cf2cfa5ec Co-authored-by: bnema <5212137+bnema@users.noreply.github.com>
- commit
- ac8991785e2c20e716b9fd147d4b62cf44d4313b
- author
- copilot-swe-agent[bot] · · 2026-04-14 13:09 UTC
- committer
- brice · · 2026-04-19 19:06 UTC
- parent
- e7be50f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +15 -5
M internal/infrastructure/cef/download_handler.go +2 -2
| @@ -101,10 +101,10 @@ func (h *downloadHandler) onDownloadUpdated( | |||
| 101 | 101 | if !h.markFinished(id, state) { | |
| 102 | 102 | return | |
| 103 | 103 | } | |
| 104 | - | err := fmt.Errorf("download failed: %s", state.filename) | |
| 104 | + | var err error | |
| 105 | 105 | if downloadItem.IsInterrupted() { | |
| 106 | 106 | err = fmt.Errorf("download interrupted: %s (%v)", state.filename, downloadItem.GetInterruptReason()) | |
| 107 | - | } else if downloadItem.IsCanceled() { | |
| 107 | + | } else { | |
| 108 | 108 | err = fmt.Errorf("download canceled: %s", state.filename) | |
| 109 | 109 | } | |
| 110 | 110 | h.runtime.EmitFailed(ctx, state.filename, state.destination, err) | |
| @@ -101,10 +101,10 @@ func (h *downloadHandler) onDownloadUpdated( | |||||
| 101 | if !h.markFinished(id, state) { | 101 | if !h.markFinished(id, state) { | ||
| 102 | return | 102 | return | ||
| 103 | } | 103 | } | ||
| 104 | - | err := fmt.Errorf("download failed: %s", state.filename) | 104 | + | var err error |
| 105 | if downloadItem.IsInterrupted() { | 105 | if downloadItem.IsInterrupted() { | ||
| 106 | err = fmt.Errorf("download interrupted: %s (%v)", state.filename, downloadItem.GetInterruptReason()) | 106 | err = fmt.Errorf("download interrupted: %s (%v)", state.filename, downloadItem.GetInterruptReason()) | ||
| 107 | - | } else if downloadItem.IsCanceled() { | 107 | + | } else { |
| 108 | err = fmt.Errorf("download canceled: %s", state.filename) | 108 | err = fmt.Errorf("download canceled: %s", state.filename) | ||
| 109 | } | 109 | } | ||
| 110 | h.runtime.EmitFailed(ctx, state.filename, state.destination, err) | 110 | h.runtime.EmitFailed(ctx, state.filename, state.destination, err) | ||
M internal/infrastructure/cef/download_handler_test.go +1 -0
| @@ -86,6 +86,7 @@ func TestCEFDownloadHandlerLifecycle(t *testing.T) { | |||
| 86 | 86 | item.complete = true | |
| 87 | 87 | item.fullPath = callback.path | |
| 88 | 88 | handler.onDownloadUpdated(ctx, item, nil) | |
| 89 | + | // Repeated completion updates should not emit duplicate finished events. | |
| 89 | 90 | handler.onDownloadUpdated(ctx, item, nil) | |
| 90 | 91 | ||
| 91 | 92 | require.Len(t, events.events, 2) | |
| @@ -86,6 +86,7 @@ func TestCEFDownloadHandlerLifecycle(t *testing.T) { | |||||
| 86 | item.complete = true | 86 | item.complete = true | ||
| 87 | item.fullPath = callback.path | 87 | item.fullPath = callback.path | ||
| 88 | handler.onDownloadUpdated(ctx, item, nil) | 88 | handler.onDownloadUpdated(ctx, item, nil) | ||
| 89 | + | // Repeated completion updates should not emit duplicate finished events. | |||
| 89 | handler.onDownloadUpdated(ctx, item, nil) | 90 | handler.onDownloadUpdated(ctx, item, nil) | ||
| 90 | 91 | ||||
| 91 | require.Len(t, events.events, 2) | 92 | require.Len(t, events.events, 2) | ||
M internal/infrastructure/downloadruntime/runtime.go +7 -2
| @@ -52,11 +52,16 @@ func (r *Runtime) ResolveDestination( | |||
| 52 | 52 | return nil, fmt.Errorf("failed to create download directory %q: %w", downloadPath, err) | |
| 53 | 53 | } | |
| 54 | 54 | ||
| 55 | - | return preparer.Execute(ctx, port.DownloadPrepareInput{ | |
| 55 | + | output := preparer.Execute(ctx, port.DownloadPrepareInput{ | |
| 56 | 56 | SuggestedFilename: suggestedFilename, | |
| 57 | 57 | Response: response, | |
| 58 | 58 | DownloadDir: downloadPath, | |
| 59 | - | }), nil | |
| 59 | + | }) | |
| 60 | + | if output == nil { | |
| 61 | + | return nil, fmt.Errorf("failed to prepare download destination for %q", suggestedFilename) | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | return output, nil | |
| 60 | 65 | } | |
| 61 | 66 | ||
| 62 | 67 | func (r *Runtime) EmitStarted(ctx context.Context, output *port.DownloadPrepareOutput) { | |
| @@ -52,11 +52,16 @@ func (r *Runtime) ResolveDestination( | |||||
| 52 | return nil, fmt.Errorf("failed to create download directory %q: %w", downloadPath, err) | 52 | return nil, fmt.Errorf("failed to create download directory %q: %w", downloadPath, err) | ||
| 53 | } | 53 | } | ||
| 54 | 54 | ||||
| 55 | - | return preparer.Execute(ctx, port.DownloadPrepareInput{ | 55 | + | output := preparer.Execute(ctx, port.DownloadPrepareInput{ |
| 56 | SuggestedFilename: suggestedFilename, | 56 | SuggestedFilename: suggestedFilename, | ||
| 57 | Response: response, | 57 | Response: response, | ||
| 58 | DownloadDir: downloadPath, | 58 | DownloadDir: downloadPath, | ||
| 59 | - | }), nil | 59 | + | }) |
| 60 | + | if output == nil { | |||
| 61 | + | return nil, fmt.Errorf("failed to prepare download destination for %q", suggestedFilename) | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | return output, nil | |||
| 60 | } | 65 | } | ||
| 61 | 66 | ||||
| 62 | func (r *Runtime) EmitStarted(ctx context.Context, output *port.DownloadPrepareOutput) { | 67 | func (r *Runtime) EmitStarted(ctx context.Context, output *port.DownloadPrepareOutput) { | ||
M internal/infrastructure/webkit/download_handler_test.go +5 -1
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package webkit | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/dumber/internal/application/usecase" | |
| @@ -31,7 +32,10 @@ func TestDownloadHandler_SetDownloadPath(t *testing.T) { | |||
| 31 | 32 | ||
| 32 | 33 | handler.SetDownloadPath("/new/path") | |
| 33 | 34 | ||
| 34 | - | assert.NotNil(t, handler.runtime) | |
| 35 | + | output, err := handler.runtime.ResolveDestination(context.Background(), "artifact.pdf", nil) | |
| 36 | + | ||
| 37 | + | require.NoError(t, err) | |
| 38 | + | assert.Equal(t, "/new/path/artifact.pdf", output.DestinationPath) | |
| 35 | 39 | } | |
| 36 | 40 | ||
| 37 | 41 | func TestURIResponseAdapter(t *testing.T) { | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package webkit | 1 | package webkit | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/bnema/dumber/internal/application/usecase" | 7 | "github.com/bnema/dumber/internal/application/usecase" | ||
| @@ -31,7 +32,10 @@ func TestDownloadHandler_SetDownloadPath(t *testing.T) { | |||||
| 31 | 32 | ||||
| 32 | handler.SetDownloadPath("/new/path") | 33 | handler.SetDownloadPath("/new/path") | ||
| 33 | 34 | ||||
| 34 | - | assert.NotNil(t, handler.runtime) | 35 | + | output, err := handler.runtime.ResolveDestination(context.Background(), "artifact.pdf", nil) |
| 36 | + | ||||
| 37 | + | require.NoError(t, err) | |||
| 38 | + | assert.Equal(t, "/new/path/artifact.pdf", output.DestinationPath) | |||
| 35 | } | 39 | } | ||
| 36 | 40 | ||||
| 37 | func TestURIResponseAdapter(t *testing.T) { | 41 | func TestURIResponseAdapter(t *testing.T) { | ||