Skip to content

fix(wfctl): address download review feedback#861

Merged
intel352 merged 2 commits into
mainfrom
codex/wfctl-download-review-followup
Jun 6, 2026
Merged

fix(wfctl): address download review feedback#861
intel352 merged 2 commits into
mainfrom
codex/wfctl-download-review-followup

Conversation

@intel352
Copy link
Copy Markdown
Contributor

@intel352 intel352 commented Jun 6, 2026

Summary

  • clean up metadata cancellation flow in GitHub release asset downloads
  • capture stderr in the direct download deadline regression test
  • make download byte unit capping follow the prefix table length

Verification

  • GOWORK=off go test ./cmd/wfctl -run 'TestDownloadURL|TestDownloadGitHubReleaseAsset|TestFormatDownloadBytes' -count=1 -v
  • GOWORK=off go test ./cmd/wfctl -count=1
  • git diff --check HEAD~1..HEAD

Copilot AI review requested due to automatic review settings June 6, 2026 04:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines wfctl download behavior and related tests by tightening GitHub release asset metadata request cancellation, improving regression test robustness by capturing stderr, and making download byte-unit capping follow the configured prefix table.

Changes:

  • Adjust GitHub release asset metadata timeout cancellation flow to explicitly cancel on all early-return paths and after metadata decode.
  • Update the direct-download bounded-context regression test to capture stderr output during the download.
  • Make formatDownloadBytes cap units based on the prefix table length instead of a hard-coded limit.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
cmd/wfctl/plugin_install.go Refines metadata request cancellation handling in the GitHub release asset download flow.
cmd/wfctl/plugin_install_test.go Captures stderr during a direct-download regression test to avoid noisy output and better match runtime behavior.
cmd/wfctl/download_progress.go Removes a hard-coded unit cap by basing unit selection on the prefix table length.

Comment thread cmd/wfctl/plugin_install.go
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 6, 2026

Codecov Report

❌ Patch coverage is 38.46154% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/wfctl/plugin_install.go 40.00% 6 Missing ⚠️
cmd/wfctl/download_progress.go 33.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 6, 2026

⏱ Benchmark Results

No significant performance regressions detected.

benchstat comparison (baseline → PR)
## benchstat: baseline → PR
baseline-bench.txt:304: parsing iteration count: invalid syntax
baseline-bench.txt:383363: parsing iteration count: invalid syntax
baseline-bench.txt:815592: parsing iteration count: invalid syntax
baseline-bench.txt:1170445: parsing iteration count: invalid syntax
baseline-bench.txt:1735586: parsing iteration count: invalid syntax
baseline-bench.txt:2056791: parsing iteration count: invalid syntax
benchmark-results.txt:304: parsing iteration count: invalid syntax
benchmark-results.txt:376331: parsing iteration count: invalid syntax
benchmark-results.txt:662379: parsing iteration count: invalid syntax
benchmark-results.txt:993691: parsing iteration count: invalid syntax
benchmark-results.txt:1333580: parsing iteration count: invalid syntax
benchmark-results.txt:1640682: parsing iteration count: invalid syntax
goos: linux
goarch: amd64
pkg: github.com/GoCodeAlone/workflow/dynamic
cpu: AMD EPYC 9V74 80-Core Processor                
                            │ benchmark-results.txt │
                            │        sec/op         │
InterpreterCreation-4                  7.520m ± 60%
ComponentLoad-4                        3.450m ±  1%
ComponentExecute-4                     1.813µ ±  1%
PoolContention/workers-1-4             1.006µ ±  3%
PoolContention/workers-2-4             1.004µ ±  1%
PoolContention/workers-4-4             1.007µ ±  0%
PoolContention/workers-8-4             1.004µ ±  5%
PoolContention/workers-16-4            1.008µ ±  1%
ComponentLifecycle-4                   3.478m ±  1%
SourceValidation-4                     2.093µ ±  1%
RegistryConcurrent-4                   733.9n ±  2%
LoaderLoadFromString-4                 3.517m ±  0%
geomean                                17.64µ

                            │ benchmark-results.txt │
                            │         B/op          │
InterpreterCreation-4                  2.027Mi ± 0%
ComponentLoad-4                        2.180Mi ± 0%
ComponentExecute-4                     1.203Ki ± 0%
PoolContention/workers-1-4             1.203Ki ± 0%
PoolContention/workers-2-4             1.203Ki ± 0%
PoolContention/workers-4-4             1.203Ki ± 0%
PoolContention/workers-8-4             1.203Ki ± 0%
PoolContention/workers-16-4            1.203Ki ± 0%
ComponentLifecycle-4                   2.183Mi ± 0%
SourceValidation-4                     1.984Ki ± 0%
RegistryConcurrent-4                   1.133Ki ± 0%
LoaderLoadFromString-4                 2.182Mi ± 0%
geomean                                15.25Ki

                            │ benchmark-results.txt │
                            │       allocs/op       │
InterpreterCreation-4                   15.68k ± 0%
ComponentLoad-4                         18.02k ± 0%
ComponentExecute-4                       25.00 ± 0%
PoolContention/workers-1-4               25.00 ± 0%
PoolContention/workers-2-4               25.00 ± 0%
PoolContention/workers-4-4               25.00 ± 0%
PoolContention/workers-8-4               25.00 ± 0%
PoolContention/workers-16-4              25.00 ± 0%
ComponentLifecycle-4                    18.07k ± 0%
SourceValidation-4                       32.00 ± 0%
RegistryConcurrent-4                     2.000 ± 0%
LoaderLoadFromString-4                  18.06k ± 0%
geomean                                  183.3

cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                            │ baseline-bench.txt │
                            │       sec/op       │
InterpreterCreation-4               10.25m ± 71%
ComponentLoad-4                     3.431m ±  1%
ComponentExecute-4                  2.045µ ±  2%
PoolContention/workers-1-4          1.179µ ±  0%
PoolContention/workers-2-4          1.176µ ±  1%
PoolContention/workers-4-4          1.175µ ±  1%
PoolContention/workers-8-4          1.177µ ±  1%
PoolContention/workers-16-4         1.180µ ±  0%
ComponentLifecycle-4                3.455m ±  1%
SourceValidation-4                  2.221µ ±  4%
RegistryConcurrent-4                871.8n ±  3%
LoaderLoadFromString-4              3.468m ±  2%
geomean                             19.86µ

                            │ baseline-bench.txt │
                            │        B/op        │
InterpreterCreation-4               2.027Mi ± 0%
ComponentLoad-4                     2.180Mi ± 0%
ComponentExecute-4                  1.203Ki ± 0%
PoolContention/workers-1-4          1.203Ki ± 0%
PoolContention/workers-2-4          1.203Ki ± 0%
PoolContention/workers-4-4          1.203Ki ± 0%
PoolContention/workers-8-4          1.203Ki ± 0%
PoolContention/workers-16-4         1.203Ki ± 0%
ComponentLifecycle-4                2.183Mi ± 0%
SourceValidation-4                  1.984Ki ± 0%
RegistryConcurrent-4                1.133Ki ± 0%
LoaderLoadFromString-4              2.182Mi ± 0%
geomean                             15.25Ki

                            │ baseline-bench.txt │
                            │     allocs/op      │
InterpreterCreation-4                15.68k ± 0%
ComponentLoad-4                      18.02k ± 0%
ComponentExecute-4                    25.00 ± 0%
PoolContention/workers-1-4            25.00 ± 0%
PoolContention/workers-2-4            25.00 ± 0%
PoolContention/workers-4-4            25.00 ± 0%
PoolContention/workers-8-4            25.00 ± 0%
PoolContention/workers-16-4           25.00 ± 0%
ComponentLifecycle-4                 18.07k ± 0%
SourceValidation-4                    32.00 ± 0%
RegistryConcurrent-4                  2.000 ± 0%
LoaderLoadFromString-4               18.06k ± 0%
geomean                               183.3

pkg: github.com/GoCodeAlone/workflow/middleware
cpu: AMD EPYC 9V74 80-Core Processor                
                                  │ benchmark-results.txt │
                                  │        sec/op         │
CircuitBreakerDetection-4                    308.8n ± 10%
CircuitBreakerExecution_Success-4            22.70n ±  0%
CircuitBreakerExecution_Failure-4            70.94n ±  0%
geomean                                      79.22n

                                  │ benchmark-results.txt │
                                  │         B/op          │
CircuitBreakerDetection-4                    144.0 ± 0%
CircuitBreakerExecution_Success-4            0.000 ± 0%
CircuitBreakerExecution_Failure-4            0.000 ± 0%
geomean                                                 ¹
¹ summaries must be >0 to compute geomean

                                  │ benchmark-results.txt │
                                  │       allocs/op       │
CircuitBreakerDetection-4                    1.000 ± 0%
CircuitBreakerExecution_Success-4            0.000 ± 0%
CircuitBreakerExecution_Failure-4            0.000 ± 0%
geomean                                                 ¹
¹ summaries must be >0 to compute geomean

cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                                  │ baseline-bench.txt │
                                  │       sec/op       │
CircuitBreakerDetection-4                  458.8n ± 6%
CircuitBreakerExecution_Success-4          59.58n ± 0%
CircuitBreakerExecution_Failure-4          65.31n ± 0%
geomean                                    121.3n

                                  │ baseline-bench.txt │
                                  │        B/op        │
CircuitBreakerDetection-4                 144.0 ± 0%
CircuitBreakerExecution_Success-4         0.000 ± 0%
CircuitBreakerExecution_Failure-4         0.000 ± 0%
geomean                                              ¹
¹ summaries must be >0 to compute geomean

                                  │ baseline-bench.txt │
                                  │     allocs/op      │
CircuitBreakerDetection-4                 1.000 ± 0%
CircuitBreakerExecution_Success-4         0.000 ± 0%
CircuitBreakerExecution_Failure-4         0.000 ± 0%
geomean                                              ¹
¹ summaries must be >0 to compute geomean

pkg: github.com/GoCodeAlone/workflow/module
cpu: AMD EPYC 9V74 80-Core Processor                
                                 │ benchmark-results.txt │
                                 │        sec/op         │
IaCStateBackend_InProcess-4                 296.0n ±  1%
IaCStateBackend_GRPC-4                      10.15m ± 17%
JQTransform_Simple-4                        654.0n ± 32%
JQTransform_ObjectConstruction-4            1.467µ ±  1%
JQTransform_ArraySelect-4                   3.482µ ±  1%
JQTransform_Complex-4                       42.33µ ±  1%
JQTransform_Throughput-4                    1.792µ ±  1%
SSEPublishDelivery-4                        64.38n ±  1%
geomean                                     3.857µ

                                 │ benchmark-results.txt │
                                 │         B/op          │
IaCStateBackend_InProcess-4                416.0 ±  0%
IaCStateBackend_GRPC-4                   5.898Mi ± 11%
JQTransform_Simple-4                     1.273Ki ±  0%
JQTransform_ObjectConstruction-4         1.773Ki ±  0%
JQTransform_ArraySelect-4                2.625Ki ±  0%
JQTransform_Complex-4                    16.31Ki ±  0%
JQTransform_Throughput-4                 1.984Ki ±  0%
SSEPublishDelivery-4                       0.000 ±  0%
geomean                                                ¹
¹ summaries must be >0 to compute geomean

                                 │ benchmark-results.txt │
                                 │       allocs/op       │
IaCStateBackend_InProcess-4                 2.000 ± 0%
IaCStateBackend_GRPC-4                     6.861k ± 0%
JQTransform_Simple-4                        10.00 ± 0%
JQTransform_ObjectConstruction-4            15.00 ± 0%
JQTransform_ArraySelect-4                   30.00 ± 0%
JQTransform_Complex-4                       328.0 ± 0%
JQTransform_Throughput-4                    17.00 ± 0%
SSEPublishDelivery-4                        0.000 ± 0%
geomean                                                ¹
¹ summaries must be >0 to compute geomean

cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                                 │ baseline-bench.txt │
                                 │       sec/op       │
IaCStateBackend_InProcess-4              347.2n ± 27%
IaCStateBackend_GRPC-4                   10.10m ±  2%
JQTransform_Simple-4                     699.6n ± 30%
JQTransform_ObjectConstruction-4         1.515µ ±  1%
JQTransform_ArraySelect-4                3.342µ ±  1%
JQTransform_Complex-4                    37.36µ ±  1%
JQTransform_Throughput-4                 1.846µ ±  1%
SSEPublishDelivery-4                     75.70n ±  0%
geomean                                  3.995µ

                                 │ baseline-bench.txt │
                                 │        B/op        │
IaCStateBackend_InProcess-4              416.0 ± 0%
IaCStateBackend_GRPC-4                 5.861Mi ± 5%
JQTransform_Simple-4                   1.273Ki ± 0%
JQTransform_ObjectConstruction-4       1.773Ki ± 0%
JQTransform_ArraySelect-4              2.625Ki ± 0%
JQTransform_Complex-4                  16.31Ki ± 0%
JQTransform_Throughput-4               1.984Ki ± 0%
SSEPublishDelivery-4                     0.000 ± 0%
geomean                                             ¹
¹ summaries must be >0 to compute geomean

                                 │ baseline-bench.txt │
                                 │     allocs/op      │
IaCStateBackend_InProcess-4              2.000 ± 0%
IaCStateBackend_GRPC-4                  6.875k ± 0%
JQTransform_Simple-4                     10.00 ± 0%
JQTransform_ObjectConstruction-4         15.00 ± 0%
JQTransform_ArraySelect-4                30.00 ± 0%
JQTransform_Complex-4                    328.0 ± 0%
JQTransform_Throughput-4                 17.00 ± 0%
SSEPublishDelivery-4                     0.000 ± 0%
geomean                                             ¹
¹ summaries must be >0 to compute geomean

pkg: github.com/GoCodeAlone/workflow/schema
cpu: AMD EPYC 9V74 80-Core Processor                
                                    │ benchmark-results.txt │
                                    │        sec/op         │
SchemaValidation_Simple-4                      1.091µ ± 10%
SchemaValidation_AllFields-4                   1.623µ ±  8%
SchemaValidation_FormatValidation-4            1.596µ ±  2%
SchemaValidation_ManySchemas-4                 1.588µ ±  1%
geomean                                        1.455µ

                                    │ benchmark-results.txt │
                                    │         B/op          │
SchemaValidation_Simple-4                      0.000 ± 0%
SchemaValidation_AllFields-4                   0.000 ± 0%
SchemaValidation_FormatValidation-4            0.000 ± 0%
SchemaValidation_ManySchemas-4                 0.000 ± 0%
geomean                                                   ¹
¹ summaries must be >0 to compute geomean

                                    │ benchmark-results.txt │
                                    │       allocs/op       │
SchemaValidation_Simple-4                      0.000 ± 0%
SchemaValidation_AllFields-4                   0.000 ± 0%
SchemaValidation_FormatValidation-4            0.000 ± 0%
SchemaValidation_ManySchemas-4                 0.000 ± 0%
geomean                                                   ¹
¹ summaries must be >0 to compute geomean

cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                                    │ baseline-bench.txt │
                                    │       sec/op       │
SchemaValidation_Simple-4                    1.010µ ± 9%
SchemaValidation_AllFields-4                 1.539µ ± 2%
SchemaValidation_FormatValidation-4          1.501µ ± 1%
SchemaValidation_ManySchemas-4               1.513µ ± 3%
geomean                                      1.370µ

                                    │ baseline-bench.txt │
                                    │        B/op        │
SchemaValidation_Simple-4                   0.000 ± 0%
SchemaValidation_AllFields-4                0.000 ± 0%
SchemaValidation_FormatValidation-4         0.000 ± 0%
SchemaValidation_ManySchemas-4              0.000 ± 0%
geomean                                                ¹
¹ summaries must be >0 to compute geomean

                                    │ baseline-bench.txt │
                                    │     allocs/op      │
SchemaValidation_Simple-4                   0.000 ± 0%
SchemaValidation_AllFields-4                0.000 ± 0%
SchemaValidation_FormatValidation-4         0.000 ± 0%
SchemaValidation_ManySchemas-4              0.000 ± 0%
geomean                                                ¹
¹ summaries must be >0 to compute geomean

pkg: github.com/GoCodeAlone/workflow/store
cpu: AMD EPYC 9V74 80-Core Processor                
                                   │ benchmark-results.txt │
                                   │        sec/op         │
EventStoreAppend_InMemory-4                   1.070µ ± 14%
EventStoreAppend_SQLite-4                     1.063m ±  6%
GetTimeline_InMemory/events-10-4              14.16µ ±  3%
GetTimeline_InMemory/events-50-4              78.78µ ±  3%
GetTimeline_InMemory/events-100-4             119.2µ ± 32%
GetTimeline_InMemory/events-500-4             613.2µ ±  7%
GetTimeline_InMemory/events-1000-4            1.241m ±  1%
GetTimeline_SQLite/events-10-4                61.27µ ±  1%
GetTimeline_SQLite/events-50-4                202.0µ ±  1%
GetTimeline_SQLite/events-100-4               374.1µ ±  1%
GetTimeline_SQLite/events-500-4               1.728m ±  1%
GetTimeline_SQLite/events-1000-4              3.412m ±  1%
geomean                                       195.6µ

                                   │ benchmark-results.txt │
                                   │         B/op          │
EventStoreAppend_InMemory-4                    830.5 ± 10%
EventStoreAppend_SQLite-4                    1.984Ki ±  3%
GetTimeline_InMemory/events-10-4             7.953Ki ±  0%
GetTimeline_InMemory/events-50-4             46.62Ki ±  0%
GetTimeline_InMemory/events-100-4            94.48Ki ±  0%
GetTimeline_InMemory/events-500-4            472.8Ki ±  0%
GetTimeline_InMemory/events-1000-4           944.3Ki ±  0%
GetTimeline_SQLite/events-10-4               16.74Ki ±  0%
GetTimeline_SQLite/events-50-4               87.14Ki ±  0%
GetTimeline_SQLite/events-100-4              175.4Ki ±  0%
GetTimeline_SQLite/events-500-4              846.1Ki ±  0%
GetTimeline_SQLite/events-1000-4             1.639Mi ±  0%
geomean                                      67.62Ki

                                   │ benchmark-results.txt │
                                   │       allocs/op       │
EventStoreAppend_InMemory-4                     7.000 ± 0%
EventStoreAppend_SQLite-4                       53.00 ± 0%
GetTimeline_InMemory/events-10-4                125.0 ± 0%
GetTimeline_InMemory/events-50-4                653.0 ± 0%
GetTimeline_InMemory/events-100-4              1.306k ± 0%
GetTimeline_InMemory/events-500-4              6.514k ± 0%
GetTimeline_InMemory/events-1000-4             13.02k ± 0%
GetTimeline_SQLite/events-10-4                  382.0 ± 0%
GetTimeline_SQLite/events-50-4                 1.852k ± 0%
GetTimeline_SQLite/events-100-4                3.681k ± 0%
GetTimeline_SQLite/events-500-4                18.54k ± 0%
GetTimeline_SQLite/events-1000-4               37.29k ± 0%
geomean                                        1.162k

cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                                   │ baseline-bench.txt │
                                   │       sec/op       │
EventStoreAppend_InMemory-4                1.106µ ± 32%
EventStoreAppend_SQLite-4                  1.355m ±  4%
GetTimeline_InMemory/events-10-4           15.17µ ±  3%
GetTimeline_InMemory/events-50-4           83.82µ ±  4%
GetTimeline_InMemory/events-100-4          165.5µ ±  2%
GetTimeline_InMemory/events-500-4          838.3µ ±  3%
GetTimeline_InMemory/events-1000-4         1.595m ± 13%
GetTimeline_SQLite/events-10-4             64.83µ ±  2%
GetTimeline_SQLite/events-50-4             227.1µ ±  1%
GetTimeline_SQLite/events-100-4            426.0µ ±  1%
GetTimeline_SQLite/events-500-4            2.019m ±  1%
GetTimeline_SQLite/events-1000-4           3.994m ±  1%
geomean                                    229.5µ

                                   │ baseline-bench.txt │
                                   │        B/op        │
EventStoreAppend_InMemory-4                  820.0 ± 9%
EventStoreAppend_SQLite-4                  1.985Ki ± 1%
GetTimeline_InMemory/events-10-4           7.953Ki ± 0%
GetTimeline_InMemory/events-50-4           46.62Ki ± 0%
GetTimeline_InMemory/events-100-4          94.48Ki ± 0%
GetTimeline_InMemory/events-500-4          472.8Ki ± 0%
GetTimeline_InMemory/events-1000-4         944.3Ki ± 0%
GetTimeline_SQLite/events-10-4             16.74Ki ± 0%
GetTimeline_SQLite/events-50-4             87.14Ki ± 0%
GetTimeline_SQLite/events-100-4            175.4Ki ± 0%
GetTimeline_SQLite/events-500-4            846.1Ki ± 0%
GetTimeline_SQLite/events-1000-4           1.639Mi ± 0%
geomean                                    67.56Ki

                                   │ baseline-bench.txt │
                                   │     allocs/op      │
EventStoreAppend_InMemory-4                  7.000 ± 0%
EventStoreAppend_SQLite-4                    53.00 ± 0%
GetTimeline_InMemory/events-10-4             125.0 ± 0%
GetTimeline_InMemory/events-50-4             653.0 ± 0%
GetTimeline_InMemory/events-100-4           1.306k ± 0%
GetTimeline_InMemory/events-500-4           6.514k ± 0%
GetTimeline_InMemory/events-1000-4          13.02k ± 0%
GetTimeline_SQLite/events-10-4               382.0 ± 0%
GetTimeline_SQLite/events-50-4              1.852k ± 0%
GetTimeline_SQLite/events-100-4             3.681k ± 0%
GetTimeline_SQLite/events-500-4             18.54k ± 0%
GetTimeline_SQLite/events-1000-4            37.29k ± 0%
geomean                                     1.162k

Benchmarks run with go test -bench=. -benchmem -count=6.
Regressions ≥ 20% are flagged. Results compared via benchstat.

@intel352 intel352 merged commit 079619e into main Jun 6, 2026
22 checks passed
@intel352 intel352 deleted the codex/wfctl-download-review-followup branch June 6, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants