Skip to content

ci(security): retry the semgrep install fail-closed (transient ResolutionImpossible reded the queue) - #1889

Merged
wshallwshall merged 3 commits into
mainfrom
fix-semgrep-install
Oct 1, 2026
Merged

wshallwshall merged 3 commits into
mainfrom
fix-semgrep-install

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

What broke

Merge-group run 36832387843 (job 110271740736) failed the blocking semgrep (project SAST rules) gate at install time. pip reported ResolutionImpossible, with "no matching distributions available for your environment: pydantic-core" at every pydantic-core version from 2.33.0 to 2.46.5. The log shows a 37-second stall just before.

Root cause: a transient empty index read, not a new release

  • The same semgrep==1.172.0 pin resolved pydantic-core==2.46.5 six minutes later, on the same runner image (ubuntu24/20260927.320), in job 110273573584.
  • It also resolved an hour earlier on the same queue head (run 36826480823).
  • It installs cleanly in a fresh local Python 3.14 venv: semgrep 1.172.0, mcp 1.23.3, pydantic-core 2.46.5.
  • PyPI lists cp314 manylinux wheels for pydantic-core 2.46.5, and none are yanked.

So this was one bad read, and a re-run alone would have passed.

The fix

Both semgrep install sites (the standalone job and the repo-scan composite, still byte-identical) now retry up to three times, with a 15s and then 30s backoff, and annotate each retry with ::warning::. It stays fail-closed: after the third failure the step exits 1 and the scan never runs. semgrep==1.172.0 is unchanged, and so is the scan command.

tests/test_semgrep_install_retry.py runs the committed loop under bash -e with python and sleep stubbed. A recovered install reaches the scan. Three failures exit 1 without reaching it. Swapping exit 1 for break turns the fail-closed test red.

Why not a hash lock (the brief's first choice)

  • semgrep is kept out of [dependency-groups] on purpose. ADR 0034 section 3 records the [otel] conflict, and tests/test_ci_venv_pinning.py says the same. A ci/locks/ lock would mean reopening that decision.
  • A lock would not have stopped this failure. Even with --require-hashes, pip still reads the index page for every package, and that read is what came back empty.

Checks run

  • pytest over every test file that mentions security.yml, semgrep or ci/locks, plus the new file: 562 passed, 1 skipped.
  • Mutation arm (fall through instead of exit 1): 1 failed, 4 passed.
  • ruff check (All checks passed), ruff format --check, and mypy on the new test (no issues).
  • The pre-commit workflow lint ("Lint GitHub Actions workflow files"): Passed. A standalone actionlint is not installed here.
  • semgrep 1.172.0 over the repo, with the job's exact args and excludes: Ran 5 rules on 435 files: 0 findings.
  • code-review subagent at xhigh: three low findings, no blockers. Two are fixed in the second commit (the comment wording and the ::warning:: annotation). The third (no test pinned fail-closed) is fixed by the new test.

Legs to read on the PR: semgrep (project SAST rules) and repo-scan, which run the edited steps.

wshallwshall added 2 commits October 1, 2026 02:59
…utionImpossible reded the queue)

Merge-group run 36832387843 (job 110271740736) failed the blocking
"semgrep (project SAST rules)" gate at install time: pip reported
ResolutionImpossible with "no matching distributions" for pydantic-core
at every version from 2.33.0 to 2.46.5. That is an empty index read, not
a new release. The same pin resolved pydantic-core 2.46.5 on the same
runner image six minutes later (job 110273573584), an hour earlier on
the same queue head, and in a clean local Python 3.14 venv.

Both semgrep install sites (the standalone job and the repo-scan
composite, kept byte-identical) now retry up to three times with a
15s/30s backoff and exit 1 after the third failure. semgrep==1.172.0 is
unchanged, and the scan step is untouched.

No hash lock: semgrep is out of [dependency-groups] by recorded
decision (ADR 0034 section 3, the [otel] conflict), and a lock would not
have prevented this failure, since pip still reads the index per package.
Runs the committed retry loop under bash -e with python and sleep
stubbed: a recovered install reaches the scan, and three failures exit
1 without reaching it. A mutation that swaps `exit 1` for `break` reds
the fail-closed test. Also from code review: retries now annotate with
::warning::, and the comment no longer says the repo-scan copy failed
in run 36832387843 (only the standalone semgrep job did).
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA: code-review subagent xhigh, head 170651c, verdict 3 low findings, no blockers. All three are addressed in 67d8b2d (the current head), which was not re-reviewed: one round, as briefed.

@wshallwshall
wshallwshall enabled auto-merge October 1, 2026 08:04
The new test reads a CI workflow, not engine source, so it belongs in
tests/tooling_manifest.txt. test_tooling_partition.py reded on its
absence.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA addendum: head 2dbe7a6 adds tests/test_semgrep_install_retry.py to tests/tooling_manifest.txt (manifest-only, not re-reviewed); test_tooling_partition.py plus the new test: 43 passed.

@wshallwshall
wshallwshall added this pull request to the merge queue Oct 1, 2026
Merged via the queue into main with commit 57c4063 Oct 1, 2026
46 of 48 checks passed
@wshallwshall
wshallwshall deleted the fix-semgrep-install branch October 1, 2026 09:21
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.

1 participant