Skip to content

ci: cache flash-attn wheels only after the ABI check, trust the workspace once - #1080

Open
curnane-lab wants to merge 1 commit into
sgl-project:mainfrom
curnane-lab:cursor/ci-fa-cache-hardening-6929
Open

curnane-lab wants to merge 1 commit into
sgl-project:mainfrom
curnane-lab:cursor/ci-fa-cache-hardening-6929

Conversation

@curnane-lab

@curnane-lab curnane-lab commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

Follow-up to #1078. Its unit-test fix left these problems in the CI dependency path, each reproduced locally before the fix:

  • A failed flash-attn wheel stays cached for good. ensure_flash_attention.sh built the patched wheel straight into the persistent SPECFORGE_CI_CACHE/flash-attn/<build_key>/wheels/, before installing it or running the ABI check. A wheel that fails the check, or a truncated one left when bdist_wheel is killed by the 30-minute step timeout, is reused by every later run with no rebuild, until someone deletes it on the runner. Reproduced with the existing stubs: after an ABI failure the wheel was still cached and the next run downloaded nothing.
  • The sdist is not checksummed. The script downloads the sdist from the URL in PyPI's JSON and patches it without checking the digests.sha256 from the same JSON. uv checked it when it fetched the sdist itself.
  • FLASH_ATTENTION_FORCE_BUILD is no longer covered by tests. Before ci: unbreak unit-test with the pinned SGLang toolchain #1078 a test asserted it on the uv command. Now it is only passed to setup.py and nothing checks it. Without it, flash-attn's setup.py downloads a prebuilt wheel built against another torch, and the tests still pass.
  • safe.directory grows every run. /github/home persists across jobs on this runner (that is what keeps SPECFORGE_CI_CACHE alive), so git config --global --add appends another identical entry on every unit-test run.

Modifications

  • ensure_flash_attention.sh
    • Build into the temporary source directory. Publish to the cache only after the wheel installs and passes check_flash_attention, by copying to a hidden staging file in the cache directory and renaming it atomically.
    • If a cached wheel fails to install or fails the check, delete it so the next run rebuilds.
    • Verify the sdist with sha256sum -c before extracting.
  • test.yaml: add the safe.directory entry only when git config --fixed-value --get does not already find it (the container has git 2.43).
  • test_ci_dependencies.py
    • The curl stub serves a byte-stable tarball plus its sha256, and its setup.py asserts FLASH_ATTENTION_FORCE_BUILD=TRUE.
    • New cases cover: no wheel cached after a failed ABI check; a cached wheel that fails is dropped and rebuilt; a checksum mismatch stops before any install; trusting the workspace twice leaves one entry.

This branch also tried running unit-test on pull_request: edited so that stacked PRs retargeted to main (#1065, #1066) would be tested, and then reverted it. GitHub does not evaluate a job's name expression when the job is skipped, so every description edit added checks named after the raw expression. Keeping the skipped jobs named unit-test instead would hide the real result and satisfy a required check. Making unit-test a required check in the repository settings covers that case without a workflow change: a retargeted PR with no unit-test run cannot merge until a push triggers one.

Related Issues

Follow-up to #1078.

Accuracy Test

Not applicable. CI scripts and tests only.

Benchmark & Profiling

Not applicable. CI scripts and tests only.

Testing

  • python -m unittest tests.test_scripts.test_ci_dependencies: 13 tests pass.
  • With the script and workflow reverted to ci: unbreak unit-test with the pinned SGLang toolchain #1078 and the new tests kept, the 4 new cases fail.
  • With FLASH_ATTENTION_FORCE_BUILD=TRUE removed from the script, 6 cases fail.
  • pre-commit run on the changed files passes.
  • unit-test passed on this PR. On the GPU runner, a passing wheel is already cached for the current build_key, so unit-test reuses it and does not exercise the new build path until torch or flash-attn changes.

Checklist

@curnane-lab curnane-lab changed the title ci fa cache hardening 6929 ci: cache flash-attn wheels only after the ABI check, trust the workspace once Nov 7, 2026
@curnane-lab curnane-lab changed the title ci: cache flash-attn wheels only after the ABI check, trust the workspace once ci: harden the flash-attn cache and run unit-test on retargeted PRs Nov 7, 2026
cursor Bot pushed a commit to curnane-lab/SpecForge that referenced this pull request Nov 7, 2026
A pull request shows the newest check run of each name on the head commit.
After a title or body edit, the skipped cleanup-runner and unit-test jobs of
that run replaced the real result on sgl-project#1080: unit-test passed in the
synchronize run but the PR kept showing it as skipped, and a skipped check
would also satisfy a required one.

Give the jobs a different name on description-edit runs so their skipped
checks no longer share a name with the real ones.

Co-authored-by: curnane-lab <curnane-lab@users.noreply.github.com>
cursor Bot pushed a commit to curnane-lab/SpecForge that referenced this pull request Nov 7, 2026
Reverts 5c1f892 and 3664971.

GitHub does not evaluate a job's name expression when the job is skipped,
so on sgl-project#1080 every description edit added two checks named after the raw
expression, and draft PRs would show the same. Keeping the skipped jobs
named unit-test instead would hide the real result and satisfy a required
check. There is no name that is readable when skipped and unit-test when
run.

Stacked PRs retargeted to main are better caught by making unit-test a
required check in the repository settings: a PR without a unit-test run
then cannot merge until a push triggers one, with no workflow change.

Co-authored-by: curnane-lab <curnane-lab@users.noreply.github.com>
@curnane-lab curnane-lab changed the title ci: harden the flash-attn cache and run unit-test on retargeted PRs ci: cache flash-attn wheels only after the ABI check, trust the workspace once Nov 7, 2026
…pace once

Follow-up to sgl-project#1078.

ensure_flash_attention.sh built the patched flash-attn wheel straight into
the persistent per-build_key cache, before installing it or running the
ABI check. A wheel that failed the check, or a truncated one left by a
timed-out or cancelled bdist_wheel, stayed cached, and every later run
skipped the build and reinstalled it.

- Build into the temporary source directory and publish to the cache by an
  atomic rename only after the wheel installs and passes the ABI check.
  Drop a cached wheel that fails to install or fails the check, so the next
  run rebuilds it.
- Verify the sdist against the sha256 in PyPI's JSON before extracting and
  patching it; uv used to do this when it fetched the sdist itself.
- Add the workspace to safe.directory only if it is not already there.
  HOME (/github/home) persists across jobs on the self-hosted runner, which
  is what keeps SPECFORGE_CI_CACHE alive, so --add appended another
  identical entry on every run.

Tests: the curl stub serves a byte-stable tarball with its sha256, and its
setup.py asserts FLASH_ATTENTION_FORCE_BUILD=TRUE, which no test covered
after the build moved out of uv. New cases cover the failed-check,
dropped-cache, checksum-mismatch and repeated safe.directory paths.

Co-authored-by: curnane-lab <curnane-lab@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/ci-fa-cache-hardening-6929 branch from 13fc02d to 040806b Compare November 7, 2026 08:02

This branch has not been deployed

No deployments
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.

3 participants