feat(lint): retry workflow downloads and flag the ones that skip it by rodboev · Pull Request #246 · kenn-io/kit

rodboev · 2026-10-27T19:48:10Z

Workflow authors get kit-download and kit-retry, plus kennlint workflow-retries, which flags run: steps that fetch without them. An apt mirror hiccup, a stalled toolchain curl, or a checksum-service error during go install now retries instead of turning main red.

Add ./.github/actions/kit-retry after checkout to put both commands on PATH:

kit-download URL DEST downloads to a temp file and moves it into place only on success. A download cut short on the first try ends with the complete file, and an exhausted one leaves an existing DEST unchanged.
kit-retry 3 5 go mod download retries any other command, waiting 5 then 10 seconds.
Kit's lint and test jobs download modules under kit-retry, and make lint runs the check.

The check flags direct curl and wget calls, and unwrapped APT, Go, npm, pnpm, yarn, bun, pip, Docker pull, apk, brew, cargo, pipx, and dnf installs, and reports each at the command's own line in run: | blocks. It parses Bash with mvdan.cc/sh/v3, a new dependency, so quoted text and echo curl aren't flagged. It doesn't follow launchers like timeout or bash -c, and # workflowretry: allow <reason> opts a command out.

roborev-ci · 2026-10-27T19:55:26Z

roborev: Combined Review (`aa75ecd`)

Verdict: Changes require fixes for 5 findings.

Medium

lint/workflowretry/workflowretry.go:620; lint/workflowretry/workflowretry.go:354: Process substitutions are accepted as download destinations in output options and shell redirects. Failed attempts can concatenate partial downloads with retries or feed partial code directly to a consumer.

Fix: Preserve destination AST information, classify process substitutions as streams, and reject unsafe retry output to them. Cover both output options and shell redirects.
lint/workflowretry/workflowretry.go:304: Inherited capture state overrides local stdout redirections, incorrectly rejecting safe downloads such as internally redirected curl commands with outer logging or pipelines.

Fix: Track effective stdout destinations so local file or null redirections override inherited capture state while preserving retry-boundary information and detection of actual captures.
lint/workflowretry/workflowretry.go:355: Nested retry helpers can incorrectly classify redirects as safe using an inherited retried flag. A redirect attached to the inner helper opens only once across its attempts, allowing partial and successful downloads to concatenate.

Fix: Track which retry boundary reopens each redirect, and treat redirects attached to an inner helper as captures across that helper's attempts.
lint/workflowretry/workflowretry.go:342: The scanner accepts exec inside kit-retry, but executing that builtin replaces the retry process with the download command, so transient failures exit without retrying.

Fix: Reject exec inside the helper and require it before kit-retry, or run each attempt in a subshell.
lint/workflowretry/workflowretry.go:338: Dynamic assignment words such as TOKEN="$TOKEN" become empty during literal extraction, causing prefix parsing to miss commands such as env TOKEN="$TOKEN" npm ci. Joined global options containing expansions can similarly hide downloads.

Fix: Preserve static assignment and option prefixes through structural extraction while keeping command names and nested scripts literal-only.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 18s | Total: 12m54s

roborev-ci · 2026-10-27T20:20:43Z

roborev: Combined Review (`a81b9ee`)

Verdict: Changes require fixes for 3 findings.

Medium

lint/workflowretry/workflowretry.go:25: Expression masking stops at }} inside quoted Actions strings. A valid step such as echo '${{ '}}' }}' becomes an unterminated Bash string after masking, causing a false lint failure.

Fix: Recognize expression delimiters while respecting quoted strings and escaped quotes; add a Scan regression test.
lint/workflowretry/workflowretry.go:452: Download recognition matches exact executable names, so ordinary invocations such as /usr/bin/curl URL and curl.exe URL bypass retry, timeout, and destination checks.

Fix: Normalize literal executable paths and supported executable suffixes consistently before recognizing downloads and applying policies.
lint/workflowretry/workflowretry.go:568: File destinations exclude only - and /dev/stdout. Commands such as kit-retry 3 5 curl -m60 -o /dev/fd/1 URL | sh pass while streaming partial failed attempts into the consumer.

Fix: Classify known file-descriptor destinations, including /dev/fd/1 and /proc/self/fd/1, as streams for both curl and wget.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 13s | Total: 8m26s

roborev-ci · 2026-10-27T21:27:34Z

roborev: Combined Review (`5ae5f9c`)

Verdict: Changes require fixes for 5 findings.

Medium

lint/workflowretry/workflowretry.go:304: Nested shell scans lose the enclosing command's ignored or superseded failure context. For example, bash -c 'kit-retry 2 0 go mod download' || true passes despite exhausted retries leaving the step successful.

Fix: Propagate the enclosing shell invocation's failure context into recursive scans and add regression coverage.
lint/workflowretry/workflowretry.go:400: Shell-option changes are read only from preceding top-level statements. A block such as { set +e; kit-retry 2 0 go mod download; echo finished; } passes even though the final echo hides exhausted retries.

Fix: Track preceding option changes within enclosing statement lists, preserving appropriate scope for blocks, subshells, conditionals, and loops, and add regression coverage.
lint/workflowretry/workflowretry.go:218: Actions expression masking turns dynamic attempt counts into invalid literal strings. For example, kit-retry ${{ 3 }} 5 go mod download is rejected, as are input-based counts, despite support for runtime shell expansions.

Fix: Preserve dynamic argument metadata during expression masking so attempt-count validation handles Actions expressions consistently with shell expansions.
lint/workflowretry/workflowretry.go:499: Timeout context is lost across nested shells and explicit inner retry helpers, incorrectly requiring curl's --max-time when an enclosing timeout already bounds execution.

Fix: Combine inherited timeout bounds with locally recognized prefixes in both retryArgs return paths, and cover nested shells and helpers.
lint/workflowretry/workflowretry.go:498: Only one retry wrapper is removed. With kit-retry 2 0 kit-retry 2 0 curl URL, download detection sees the remaining helper instead of curl, bypassing required timeout, HTTP-error, and destination checks.

Fix: Inspect consecutive retry wrappers and validate the underlying download, or explicitly reject nested retry wrappers.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 15s | Total: 11m0s

roborev-ci · 2026-10-27T23:01:49Z

roborev: Combined Review (`7892de1`)

Verdict: Changes require fixes for 7 findings.

Medium

.github/workflows/ci.yml:368: On a Windows cache miss, ORT_ARCHIVE contains a backslash-separated Python path. Git Bash's basename leaves those separators intact, so the download URL contains the local path instead of the archive filename and the embedding job fails.

Fix: Export archive.name separately and use it in the URL, or export the archive path with forward slashes.
lint/workflowretry/workflowretry.go:116: The tokenizer does not respect quotes embedded in shell words. Literal assignments containing semicolons or command-substitution examples can produce false download findings, while concatenated quoted executable fragments such as 'cu''rl' escape detection.

Fix: Tokenize complete shell words with quote state, preserving literal assignments and concatenating adjacent quoted fragments. Add regression tests for these cases.
lint/workflowretry/workflowretry.go:248: Arithmetic shifts are mistaken for here-document declarations. For example, n=$((1 << 2)) registers 2 as a delimiter and masks a following npm ci command, silently missing the download.

Fix: Recognize here-document operators only in shell redirection contexts, excluding arithmetic expressions, and add a regression test.
lint/workflowretry/workflowretry.go:237; lint/workflowretry/workflowretry.go:225: Multiline command substitutions inside unquoted here-documents are discarded because substitution extraction processes each body line independently. Bash executes the download inside the substitution, but the linter misses it.

Fix: Collect the complete here-document body before extracting substitutions, or preserve substitution state across lines. Add a multiline substitution regression test.
lint/workflowretry/workflowretry.go:146: Leading redirections conceal downloads. For >download.log curl https://example.com, the scanner treats the redirection as the executable and misses the curl command.

Fix: Recognize redirection operators, descriptors, and targets separately before identifying the executable.
lint/workflowretry/workflowretry.go:141: An exemption leaks into subsequent command lines when its target ends with a semicolon. The semicolon flushes the target command, leaving the newline unable to clear allowed, so an unrelated following download is silently exempted.

Fix: Track whether the exemption's target command has been consumed and clear it before subsequent command lines, including after semicolon termination.
lint/workflowretry/workflowretry.go:194: The substitution boundary scanner counts parentheses inside shell comments. A closing parenthesis in a comment can prematurely terminate scanning and hide a subsequent download that Bash executes inside the substitution.

Fix: Skip shell comments while locating substitution boundaries, respecting quoting and comment-start rules.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 15s | Total: 9m50s

roborev-ci · 2026-10-28T00:20:56Z

roborev: Combined Review (`1ed919d`)

Verdict: Changes require fixes for 8 findings.

Medium

lint/workflowretry/workflowretry.go:148: Unquoted command substitutions lose the enclosing command's argument context. For example, echo $(date) curl https://example.com incorrectly reports an unwrapped download because arguments after the substitution are treated as a new command.

Fix: Scan substitution bodies separately while preserving the enclosing command and its arguments. Add a regression covering arguments after an unquoted substitution.
lint/workflowretry/workflowretry.go:116: Nested double quotes inside command substitutions can hide executed downloads. An inner quoted argument containing spaces causes tokenization to absorb a subsequent curl command as quoted data.

Fix: Track substitution nesting with an independent quoting context. Add a regression with an inner double-quoted argument containing spaces followed by a download.
lint/workflowretry/workflowretry.go:247: The heredoc detector treats arithmetic left shifts as heredoc operators. flags=$((1 << 2)) followed by npm ci records 2 as a delimiter and masks the remaining script, missing the download.

Fix: Recognize heredoc operators only in shell redirection context, excluding arithmetic expressions. Add a regression covering a left shift followed by a download.
lint/workflowretry/workflowretry.go:135: An allow comment before status="$(curl http://127.0.0.1/health)" also exempts a following npm ci. Removing assignment arguments leaves the exemption active after the substitution executes.

Fix: Consume the exemption when an assignment executes a command substitution, while preserving it across empty control tokens. Test that the following download is reported.
lint/workflowretry/workflowretry.go:365: command -v git curl incorrectly reports an unwrapped download because lookup operands are classified as executed commands. The same issue affects command -V.

Fix: Recognize command lookup mode and exclude its remaining operands from executable-command classification.
lint/workflowretry/workflowretry.go:152: Leading redirections hide executed downloads. For example, >deps.log npm ci produces no finding because the redirection becomes the executable name. Input and descriptor redirections have the same problem.

Fix: Parse redirections separately and exclude their operators and operands when identifying the executable.
lint/workflowretry/workflowretry.go:148: Case patterns and function declarations are incorrectly classified as command invocations. A curl) case arm or curl() { :; } function declaration reports a download without executing one.

Fix: Track case patterns and function declarations so their names are excluded from invocation checks.
lint/workflowretry/workflowretry_test.go:51: TestScan/prefix uses an already-compliant command, so ignoring the entire sudo/env chain still passes. The case does not establish that prefixes, options, assignments, or the underlying download were inspected.

Fix: Rewrite the case as sudo -u root env CI=1 npm ci and expect a kit-retry finding to verify traversal through both prefixes.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 20s | Total: 11m57s

roborev-ci · 2026-10-28T01:50:16Z

roborev: Combined Review (`b96cd8d`)

Verdict: Changes require fixes for 6 findings.

Medium

lint/workflowretry/workflowretry.go:151; lint/workflowretry/workflowretry.go:152: Leading redirections such as >deps.log npm ci are classified as the executable, hiding downloads. Trailing redirections can also become extra curl URL arguments and produce false findings.

Fix: Parse redirections separately, exclude their operators and operands from command arguments, and add regression coverage.
lint/workflowretry/workflowretry.go:285: Arithmetic left shifts such as flags=$((1 << 2)) are mistaken for heredocs with delimiter 2, masking subsequent commands and missing downloads.

Fix: Distinguish arithmetic expressions from heredoc redirections and test a left shift followed by an unwrapped download.
lint/workflowretry/workflowretry.go:147: Function declarations such as curl() { :; } and case patterns such as curl) are classified as executed downloads, causing valid workflows to fail linting.

Fix: Track function declarations and case-pattern context so structural names are excluded from invocation checks.
lint/workflowretry/workflowretry.go:229: A parenthesis inside a command-substitution comment prematurely closes the substitution, causing executed downloads inside it to be treated as quoted outer-command data and missed.

Fix: Skip shell comments when locating command-substitution boundaries and add a regression test.
lint/workflowretry/workflowretry.go:274: Multiline command substitutions inside unquoted heredocs lose their executable bodies because each heredoc line is processed independently, causing downloads inside those substitutions to be missed.

Fix: Preserve substitution state across heredoc lines or collect the complete body before extracting substitutions, and add regression coverage.
lint/workflowretry/workflowretry.go:222: The substitution-boundary scanner treats escaped double quotes as closing quotes. This can consume outer-command arguments into a substitution and report curl even when it is only an echo argument.

Fix: Track quote and escape state while locating substitution boundaries and test an escaped quote inside a substitution.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 20s | Total: 11m59s

rodboev · 2026-10-28T02:19:20Z

Reworking this around a real bash parser and a download helper that only replaces the file after a full download, so a failed attempt can't leave a broken file behind.

roborev-ci · 2026-10-28T04:01:39Z

roborev: Combined Review (`4fbac6a`)

Verdict: No findings at or above medium severity.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 5m25s

roborev-ci · 2026-10-28T04:58:44Z

roborev: Combined Review (`b710ee6`)

Verdict: No findings at or above medium severity.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 4m41s

rodboev · 2026-10-29T14:28:06Z

Back to draft while I drop the Windows runner guess, since reading "windows" out of the runner text could hide real findings on Linux jobs. The docs now say to set the shell explicitly on Windows.

roborev-ci · 2026-10-29T14:45:40Z

roborev: Combined Review (`4bfaf26`)

Verdict: No findings at or above medium severity.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 7m34s

Transient network failures can turn healthy workflow runs red. Retrying a download also needs to preserve the destination until the full response arrives, so consumers don't receive partial files. Shared helpers give repositories one retry policy. Checking executable shell calls keeps raw downloads and package installers from bypassing that policy.

…ot name Remove option-value tables for sudo, env, apt, and go, option skipping before installer subcommands, path and .exe normalization of command names, and the expanded installer row builder in favor of one table regex. Shorten the kit-download test to its two named cases.

Workflow downloads can bypass retry checks when installers place options before their subcommands or callers use executable paths and shell prefixes. Keep those spellings subject to the same retry policy. Implicit Windows shells can contain valid PowerShell that Bash cannot parse. Skip those scripts so the workflow check remains usable across runners.

Bash runs c\url as curl, so escaped command names must receive the same retry guidance as ordinary spellings. Document the check's launcher and installer limits so callers can judge its coverage.

Repeated guidance at one run key leaves users unable to identify which command needs retries. Each message needs enough context to find the command within the script. Clarify where allow comments apply and which installers the check covers so users can judge its coverage.

Failed downloads must preserve the destination and remove partial files. HTTP errors leave no partial file, so truncated responses are needed to exercise cleanup when every attempt fails. Keep scanner cases together so command-specific diagnostics share the same assertions.

CI rejects the installer condition under staticcheck QF1001. Preserve its behavior while meeting the lint rule.

Windows jobs use PowerShell by default, so treating their commands as Bash produces false download warnings. Keep explicit shell choices authoritative and continue checking runners selected through expressions conservatively.

Runner metadata can hide download findings when unrelated labels or comments contain Windows text. Shell declarations provide the choice this check needs without interpreting runner labels and expressions. Keep runner inference out of the scanner and document how Windows workflows select PowerShell explicitly.

Windows users need explicit shell guidance because the workflow checker and runner use different defaults. Keep that guidance concise so callers can choose the right shell for retry helpers and PowerShell steps.

roborev-ci · 2026-10-29T14:49:51Z

roborev: Combined Review (`8e9b96c`)

Verdict: No findings at or above medium severity.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 6m27s

Findings pointed at the run key and gave a script line number, so in a long step a reader had to count lines to find the command. Literal blocks and one-line plain values now report the command's own file line and column. Two commands on one line now get separate positions. Other scalar styles fold or escape text, so their script positions do not map to the file. Those findings still point at the run key. Plain `yarn` and `yarn --frozen-lockfile` are the usual yarn v1 CI install commands, and the check missed both. Yarn with no subcommand now counts as an install. Version and help flags are excluded. kit-download failed with exit 126 when run as `bash kit-download` from its own directory, because the script path had no directory part. It now uses dirname, which returns "." in that case. The lint job ran `go mod tidy` as its first module fetch, without a retry, so a short proxy failure there still turned CI red. The job now downloads modules under kit-retry first. Generated with Claude Code (claude-opus-5-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

roborev-ci · 2026-10-29T15:50:34Z

roborev: Combined Review (`e84e67e`)

Verdict: No findings at or above medium severity.

Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 7m37s

rodboev force-pushed the pr/shared-download-retry branch from 5ae5f9c to 7892de1 Compare October 27, 2026 22:56

rodboev force-pushed the pr/shared-download-retry branch from 7892de1 to 1ed919d Compare October 28, 2026 00:16

rodboev force-pushed the pr/shared-download-retry branch from b96cd8d to 89eefdb Compare October 28, 2026 03:54

rodboev changed the title ~~feat(ci): retry shared downloads~~ feat(lint): retry workflow downloads and flag the ones that skip it Oct 28, 2026

rodboev marked this pull request as ready for review October 28, 2026 04:13

rodboev marked this pull request as draft October 28, 2026 04:54

rodboev marked this pull request as ready for review October 28, 2026 05:08

wesm self-assigned this Oct 29, 2026

rodboev marked this pull request as draft October 29, 2026 14:28

rodboev added 10 commits October 29, 2026 09:46

fix(workflowretry): catch escaped download commands

eea973f

Bash runs c\url as curl, so escaped command names must receive the same retry guidance as ordinary spellings. Document the check's launcher and installer limits so callers can judge its coverage.

fix(workflowretry): satisfy Go hygiene checks

c398374

CI rejects the installer condition under staticcheck QF1001. Preserve its behavior while meeting the lint rule.

fix(workflowretry): respect Windows runner default shells

e8756e1

Windows jobs use PowerShell by default, so treating their commands as Bash produces false download warnings. Keep explicit shell choices authoritative and continue checking runners selected through expressions conservatively.

docs: clarify Windows workflow shell requirements

8e9b96c

Windows users need explicit shell guidance because the workflow checker and runner use different defaults. Keep that guidance concise so callers can choose the right shell for retry helpers and PowerShell steps.

rodboev force-pushed the pr/shared-download-retry branch from 4bfaf26 to 8e9b96c Compare October 29, 2026 14:46

rodboev marked this pull request as ready for review October 29, 2026 15:00

rodboev requested a review from wesm October 29, 2026 15:00

rodboev removed the request for review from wesm October 29, 2026 18:45

wesm merged commit cf014ce into main Oct 30, 2026
19 checks passed

wesm deleted the pr/shared-download-retry branch October 30, 2026 02:36

Conversation

rodboev commented Oct 27, 2026 • edited by wesm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roborev-ci Bot commented Oct 27, 2026

roborev: Combined Review (aa75ecd)

Medium

Uh oh!

roborev-ci Bot commented Oct 27, 2026

roborev: Combined Review (a81b9ee)

Medium

Uh oh!

roborev-ci Bot commented Oct 27, 2026

roborev: Combined Review (5ae5f9c)

Medium

Uh oh!

roborev-ci Bot commented Oct 27, 2026

roborev: Combined Review (7892de1)

Medium

Uh oh!

roborev-ci Bot commented Oct 28, 2026

roborev: Combined Review (1ed919d)

Medium

Uh oh!

roborev-ci Bot commented Oct 28, 2026

roborev: Combined Review (b96cd8d)

Medium

Uh oh!

rodboev commented Oct 28, 2026

Uh oh!

roborev-ci Bot commented Oct 28, 2026

roborev: Combined Review (4fbac6a)

Uh oh!

roborev-ci Bot commented Oct 28, 2026

roborev: Combined Review (b710ee6)

Uh oh!

rodboev commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roborev-ci Bot commented Oct 29, 2026

roborev: Combined Review (4bfaf26)

Uh oh!

roborev-ci Bot commented Oct 29, 2026

roborev: Combined Review (8e9b96c)

Uh oh!

roborev-ci Bot commented Oct 29, 2026

roborev: Combined Review (e84e67e)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

rodboev commented Oct 27, 2026 •

edited by wesm

Loading

roborev: Combined Review (`aa75ecd`)

roborev: Combined Review (`a81b9ee`)

roborev: Combined Review (`5ae5f9c`)

roborev: Combined Review (`7892de1`)

roborev: Combined Review (`1ed919d`)

roborev: Combined Review (`b96cd8d`)

roborev: Combined Review (`4fbac6a`)

roborev: Combined Review (`b710ee6`)

rodboev commented Oct 29, 2026 •

edited

Loading

roborev: Combined Review (`4bfaf26`)

roborev: Combined Review (`8e9b96c`)

roborev: Combined Review (`e84e67e`)