Skip to content

fix(qa): a QA round tests the commit it was asked to - #373

Merged
sudoTomas merged 1 commit into
mainfrom
fix/qa-round-tests-requested-head
Nov 5, 2026
Merged

sudoTomas merged 1 commit into
mainfrom
fix/qa-round-tests-requested-head

Conversation

@sudoTomas

@sudoTomas sudoTomas commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

A QA round tests the commit it was asked to, qa["head_sha"]: every round the warmer checks the shared volume's tree out to that commit and removes every untracked and ignored file except the dependency directories, installing only when the lockfile changed; a sub-task that checks out its own workspace pins it to that commit; and a sub-task whose workspace holds another commit fails the round. Issue #371.

Stacked on #372.

Motivation — Intent + Problem

A QA verdict is about one commit. A shared cache volume is reused from round to round, so each round must start from the commit it tests and from a tree holding nothing an earlier round built, and every sub-task must be able to show which commit it ran.

How it works

  • The commit. init_orchestrator_node reads it from the bootstrap clone (qa["head_sha"]) and passes it to the warmer as requested_head (a required argument: a full commit id) and, through the fan-out, run_subtask and the sub-task state, to each sub-task as expected_head_sha.
  • The warmer, every round. After the mount check (fix(cache): collision-free QA volumes, trusted per-repo only #372), prep_qa_sandbox_clone(head_sha=…) clones into the volume or fetches the branch into the checkout already there, then checks the tree out to the commit, detached (pin_checkout: cat-file -e <sha>^{commit} || fetch origin <sha>, then checkout -f --detach <sha>). The warmer checks HEAD is that commit, then runs clean_tree: git clean -ffdxq with an --exclude for each kept pattern, and .qa/ created again. Only then does it install, and only when the recorded lockfile sha differs from the round's ("warmed"); otherwise it records the use and returns "refreshed". Every git step is built and run by sandbox_git.
  • Kept between rounds. DEFAULT_KEEP: node_modules at any depth (a workspace install writes one into each package, and nothing builds into a directory of that name), and, anchored at the top of the tree where their installs write them, /.pnpm-store, /.yarn, /.pnp.cjs, /.pnp.js, /.pnp.loader.mjs, /.pnp.data.json, /.venv, /__pypackages__ and /vendor, so a build output of the same name deeper down (a built public/vendor/) is removed. Plus the stored config's cache.shared_volume.keep: up to 32 gitignore-style patterns, for files an install writes elsewhere.
  • The keep rule. keep_pattern_problem requires a pattern's first path segment (after an optional leading /) to be a plain name: no *, ?, [ or backslash. A literal segment matches only that exact name, so every path a pattern keeps has a component of exactly that name (at any depth with no slash, at the top of the tree with one), and no pattern can keep every file however its wildcards are written (*, ?*, *?, [!.]*, **/x, /* are refused). It holds without re-implementing git's matcher; a check against sample paths would not, since a pattern can avoid the samples' names and still keep everything else ([!dn.]* keeps build/, out/ and every top-level file). Negated patterns, comments and ./.. segments are refused too, and every DEFAULT_KEEP entry passes the same rule (a test pins it).
  • The sub-task. On a shared volume it never moves the tree; it reads HEAD and compares. Without one (a clone, or a pre-built image's baked checkout) it passes head_sha to prep_qa_sandbox_clone, which pins its checkout the same way. Either way a workspace at another commit ends the sub-task as infra_failure, naming both commits.

Pattern provenance

N/A — originated in this PR.

Verification

Automated (on #372 at df565a90)

  • Full unit suite against a private Postgres database, -n 4 --fail-on-warnings, DOCKER_HOST=unix:///nonexistent.sock: 18183 passed, 21 skipped in 390.18s (0:06:30)
  • Integration suite: 51 passed, 20 skipped (the Docker-backed ones skip, with DOCKER_HOST pointed at no daemon)
  • ruff format --check ., ruff check .: clean. mypy embry0: Success: no issues found in 650 source files. test_sandbox_git_guard.py passes (the warmer's checkout and clean go through sandbox_git).
  • Through the graph. test_round_tests_requested_head.py runs the compiled QA workflow (init_orchestrator → orchestrate_qa → qa_report) with the real warmer, fan-out, sub-task graph and acquire node; only the daemon and the sandboxes are fakes. It checks that the warmer pins the shared volume to the commit the bootstrap read, and that a sub-task whose volume holds another commit fails the round naming it.
  • Fail first. The 18 new tests (56 cases), run inside a clean git archive of fix(cache): collision-free QA volumes, trusted per-repo only #372 (df565a90) with only the test files copied in: 56 failed, each on an AssertionError (or pytest.fail) with its own message. Against the previous head of this PR (67dc5803), the reviewer's forms ?*, *? and [!.]*, and every other wildcard-first form in the test, are accepted there and fail the test, and the real-git clean test fails there with apps/py/.venv, public/vendor/ and the nested .yarn/.pnp.cjs/.pnpm-store/__pypackages__ left behind.
  • Mutations. 33, all caught:
    • cutting the orchestrator → warmer link (requested_head), or any link of orchestrator → fan-out → run_subtask → sub-task state → acquire (expected_head_sha), or the acquire head check: each caught by the graph test;
    • the install skipped on a changed lockfile; no clean, or a clean only when installing; the warmer not pinned, not verifying the head, or accepting no commit;
    • node_modules dropped from the keep list, the config's keep ignored, the clean keeping ignored files (-x dropped);
    • the prep pin not run, or pinning any ref; the sub-task's own checkout not pinned; keep accepting a negated or match-everything pattern;
    • the keep rule ignoring ?, [ or a backslash, checking only whether the whole pattern is wildcards, not stripping the leading /, allowing a comment or a ./.. segment, or not applied at all;
    • vendor, .venv or .yarn unanchored, the .pnp files back to a .pnp.* glob, node_modules anchored.
  • The clean, with real git, in the unit suite. test_the_clean_removes_build_output_and_keeps_only_the_dependency_directories runs the warmer's own clean_tree script (prelude included) with bash and git on a real checkout: dist/, .next/, .turbo/, public/vendor/, nested .yarn/.pnp.cjs/.venv/__pypackages__/.pnpm-store and an untracked file go; every node_modules, the top-level dependency directories and the config's patterns (src/generated anchored, gen at any depth, tools/**) stay.
  • Real git (a qfix- container serving a bare repository over smart HTTP, embry0's own git configuration written by sandbox_git.configure, the warmer's own steps; 11/11): round 1 clones at commit 1 and leaves dist/, .next/, public/vendor/, an untracked file, an edited tracked file and node_modules/ (top level and in a workspace package); round 2 at commit 2 ends at commit 2 with the tracked file reset, dist/, .next/, public/vendor/ and the untracked file gone, both node_modules/, the top-level vendor/ and a keep path kept, and .qa/ in place; a commit behind the branch tip is checked out detached.
  • gitleaks (--no-git over the changed files): no leaks. Name denylist after staging: 0 matches.

Deploy notes

  • infra/deploy.sh with fix(cache): collision-free QA volumes, trusted per-repo only #372. No image rebuild: nothing baked into a sandbox image changes.
  • The first round on each shared volume after the deploy cleans its tree: build output and every other untracked or ignored file outside the kept directories go, and with an unchanged lockfile the install is skipped. A dependency directory other than node_modules below the top of the tree (a workspace package's own .venv/ or vendor/) is removed too; such a repository needs it in keep. A repository whose install writes files outside node_modules and the other kept directories (generated code from a postinstall, say) needs those paths in cache.shared_volume.keep; without them a round that skips the install fails to boot or build, which shows in that round's sub-task logs.
  • The log event qa_shared_volume_skipped is gone; a round with an unchanged lockfile logs qa_shared_volume_refreshed, and the warmer returns "refreshed".
  • A sub-task failure reading … is at <sha>, expected <sha>: this round was not run against the commit it was asked to test means its workspace held another commit (for example, a concurrent run moved a shared volume); rerunning the round warms the volume again.

@sudoTomas
sudoTomas force-pushed the fix/qa-volume-isolation branch from 635b3e6 to 7b0bd43 Compare November 5, 2026 03:44
@sudoTomas
sudoTomas force-pushed the fix/qa-round-tests-requested-head branch from 7cd2283 to 67dc580 Compare November 5, 2026 03:44
@sudoTomas
sudoTomas force-pushed the fix/qa-volume-isolation branch from 7b0bd43 to df565a9 Compare November 5, 2026 16:47
@sudoTomas
sudoTomas force-pushed the fix/qa-round-tests-requested-head branch from 67dc580 to cf79631 Compare November 5, 2026 16:52
@sudoTomas
sudoTomas deleted the branch fix/qa-volume-isolation November 5, 2026 17:07
@sudoTomas sudoTomas closed this Nov 5, 2026
@sudoTomas sudoTomas reopened this Nov 5, 2026
@sudoTomas
sudoTomas changed the base branch from fix/qa-volume-isolation to main November 5, 2026 17:07
A QA round verifies one commit: the head its bootstrap clone read,
qa["head_sha"]. Every workspace the round's sub-tasks run in holds that
commit, and a sub-task whose workspace holds another fails the round.

The warmer runs every round. It checks the shared volume's labels,
fetches the branch into the volume's checkout (or clones into an empty
volume), checks the tree out to the round's commit, and removes every
untracked and ignored file except the dependency directories: every
node_modules, and at the top of the tree only .pnpm-store, .yarn,
Yarn's .pnp files, .venv, __pypackages__ and vendor, plus the stored
config's cache.shared_volume.keep patterns. It runs the install only
when the recorded lockfile sha differs from the round's, and returns
"refreshed" when it skips it. Every git step goes through sandbox_git.

A sub-task on a shared volume reads its HEAD and fails the round, with
both commits in the failure, when it is not the round's commit. A
sub-task without a shared volume (a clone, or a pre-built image's
checkout) checks its workspace out to the round's commit first, with
the same check. The commit travels from init_orchestrator_node to the
warmer as requested_head and through the fan-out to each sub-task as
expected_head_sha.

cache.shared_volume.keep takes up to 32 gitignore-style patterns, for
files an install writes outside the default directories. Each must
start with a plain name (no *, ?, [ or backslash in its first path
segment), so every path it keeps has a component of exactly that name
and no pattern keeps every file; a negated pattern, a comment and a
. or .. segment are refused.

Fixes: #371
@sudoTomas
sudoTomas force-pushed the fix/qa-round-tests-requested-head branch from cf79631 to 141ffd2 Compare November 5, 2026 17:49
@sudoTomas
sudoTomas merged commit 1efcedb into main Nov 5, 2026
13 checks passed
@sudoTomas
sudoTomas deleted the fix/qa-round-tests-requested-head branch November 5, 2026 18:11
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