Skip to content

fix(534): drop the unretried apt-get from a required check - #537

Merged
LukasWodka merged 1 commit into
developfrom
fix/534-drop-the-unretried-apt
Aug 20, 2026
Merged

fix(534): drop the unretried apt-get from a required check#537
LukasWodka merged 1 commit into
developfrom
fix/534-drop-the-unretried-apt

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Closes#534.

Installer (shell) is a required status check on develop, and its first action was an apt-get with no retry and no time bound of its own — so a slow package mirror consumed the whole 10-minute job budget before any shell was parsed, and blocked every PR in the repo while doing it.

Measured, not theorised

cli#533 is a workflow-only diff that cannot touch installer behaviour. It failed four consecutive times:

job 96126585157 Installer (shell) failure 10m16s
15:34 Set up job
15:34 Run actions/checkout
15:34 shellcheck + dash parse <- 10 minutes here, then killed
15:44 Post Run actions/checkout

Nothing after the apt-get line ever ran. And the annotation read The job has exceeded the maximum execution time of 10m0s on a job called Installer (shell) — so whoever reads it reasonably concludes the installer is hanging. Nothing points at apt.

Not repo-wide, which is worth saying because the ticket first implied it: #530 and #526 pass the same check. It reproduced on one head, four times.

The fix removes the dependency rather than hardening it

Both tools are already on ubuntu-latest:

  • shellcheck is preinstalledtracebloc/.github's own quality / shellcheck job, a required check in 16 repos, calls shellcheck --version with no install at all. The org has depended on that fleet-wide for as long as that job has existed.
  • dashis Ubuntu's /bin/sh, an essential package.

A retry-with-timeout around apt would have been the smaller diff and the worse fix: a step that installs nothing cannot stall on a mirror, and no wrapper can say that.

shellcheck --version | head -2 is kept as the first line, matching what the org's own shellcheck job does — the version in use lands in the log, and an absent binary fails on line one with an obvious message rather than somewhere further down.

This PR's own run is the proof

Deliberately: if either tool were missing, the step fails loudly here, before merge. That's a better check than any claim in the comment.

Verified locally as well — shellcheck --shell=sh --severity=error scripts/install.sh clean, dash -n scripts/install.sh clean.


Note

Cursor Bugbot is generating a summary for commit eea9e9d. Configure here.

`Installer (shell)` is a REQUIRED status check on develop, and its first action was
an `apt-get` with no retry and no time bound of its own. A slow package mirror
therefore consumed the whole 10-minute job budget before any shell was parsed, and
blocked every PR in the repo while doing it.
MEASURED, not theorised. cli#533 is a workflow-only diff that cannot touch installer
behaviour, and it failed FOUR consecutive times:
job 96126585157 Installer (shell) failure 10m16s
15:34 Set up job
15:34 Run actions/checkout
15:34 shellcheck + dash parse <- 10 minutes here, then killed
15:44 Post Run actions/checkout
Nothing after the `apt-get` line ever ran. And the annotation read `The job has
exceeded the maximum execution time of 10m0s` on a job called `Installer (shell)`,
so whoever sees it reasonably concludes the installer is hanging. Nothing points at
apt.
NOT REPO-WIDE, which is worth stating because the ticket first implied it: #530 and
#526 pass the same check. It reproduced on one head, four times.
THE FIX REMOVES THE DEPENDENCY RATHER THAN HARDENING IT. Both tools are already on
`ubuntu-latest`:
* shellcheck is preinstalled -- tracebloc/.github's own `quality / shellcheck`
job, a REQUIRED check in 16 repos, calls `shellcheck --version` with no install
at all. The org has depended on that fleet-wide for as long as that job existed.
* dash IS Ubuntu's /bin/sh, an essential package.
A retry-with-timeout around apt would have been the smaller diff and the worse fix:
a step that installs nothing cannot stall on a mirror, and no wrapper can say that.
`shellcheck --version | head -2` is kept as the first line, matching what the org's
own shellcheck job does -- so the version in use is in the log, and an absent binary
fails on line one with an obvious message instead of somewhere further down.
THIS PR'S OWN RUN IS THE PROOF, and that is deliberate: if either tool were missing
the step fails loudly here, before merge. Better than any claim in the comment.
Verified locally too: shellcheck --shell=sh --severity=error scripts/install.sh
clean, dash -n scripts/install.sh clean.
Closes#534.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 20, 2026

@saqlainsyed007saqlainsyed007 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed — this is correct and I'll approve as soon as the Test job goes green (it's the run that proves the tools are present, so approving ahead of it would defeat the point).

Verified the whole step, not just the diff context: after the apt-get line goes, the step still runs dash -n scripts/install.sh and bash -n … — both dash (Ubuntu's /bin/sh, essential) and bash are guaranteed on ubuntu-latest, and shellcheck is preinstalled on the runner image (corroborated by tracebloc/.github's required quality / shellcheck job calling it with no install). The shellcheck --version | head -2 preflight fails loudly on the first line if it's ever absent, so the "this PR's own run is the proof" claim holds. Removing an unbounded, un-retried apt-get from the first step of a required check is the right fix — nothing after it ran when a mirror stalled, and it mislabelled the failure as Installer (shell).

@saqlainsyed007saqlainsyed007 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving — CI is green now, which was the one thing I was holding for. As verified last pass: the step still exercises dash/bash/shellcheck (all guaranteed on ubuntu-latest), the shellcheck --version preflight fails loudly if the tool is ever absent, and removing the unbounded, un-retried apt-get from the first step of a required check is the right fix — the passing run is itself the proof the tools are present. No open threads, no conflict.

@LukasWodka
LukasWodka merged commit 749ea3a into developAug 20, 2026
28 checks passed
@LukasWodka
LukasWodka deleted the fix/534-drop-the-unretried-apt branch August 20, 2026 11:31
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

Sign up for freeto 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.

cli's required Installer (shell) check can be blocked by an unretried apt-get, and reports it as an installer timeout

2 participants

@LukasWodka@saqlainsyed007