Claude/trusting nightingale 0d0743 - #176
Merged
Merged
Conversation
…pens `contends` is a list an author typed, and `can_overlap` compares two such lists to each other. Nothing in that loop had ever looked at a disk, so the invariant actually enforced was "the declarations agree with each other" -- and a step that did not mention what it touched satisfied every check by saying nothing. The writer was usually not the step at all, but a unit test three subprocesses below it. That gap cost two release runs. `capsem-admin` staged profile payloads with `fs::hard_link`, putting 48 checked-in `config/` files inside published release output sharing an inode, while a container held the same tree bind-mounted. The symptom was an intermittent `Permission denied` on a file that was `0644` before and `0644` after -- so no before/after comparison could ever have found it. Two sources, because neither alone is enough. `interception.Instrument` proxies the mutating `os`/`shutil` primitives: the caller is known exactly through a `ContextVar` the scheduler sets, and `chmod` is observed with the mode before *and* after, which no watcher can recover. A `watchdog` observer covers what subprocesses do, which no in-process proxy can see. Reported three ways, and the first two were what was missing: stderr the minute it happens, a fault log beside the run -- fsynced per line so a `kill -9` still leaves it, and rotated before writing so the cap is exact rather than `keep + 1` lines of lie -- and the run log. Rules: a hardlink between checked-in source and build output (decidable from one `stat`, no concurrency needed); a mode that returns to one it already had; source writable beyond its owner; an artifact that ends the run empty; identical bytes under two names; and two concurrent steps touching one path neither declared. Existing links are surveyed at start, because a link made by a previous run raises no event in this one and the defect is in the state. `capsem_core::auditfs` is the Rust half: one audited `stage()` that copies when the source is checked in, still hardlinks build output, and fails *closed* when it cannot classify -- the first version failed open on a relative path and 192 files were still linked on the next build. Rust cannot be monkeypatched, so `tests/test_rust_filesystem_chokepoint.py` is the equivalent, scoped to `hard_link` for the reason written into the file. Proven by planting the defect and watching a real `capsem-gate doctor` name it on stderr, in `errors.log`, and in `run.jsonl` -- not by the 22 tests, which are the ratchet. Staging produced 192 hardlinks into published output before `auditfs` and 0 after, artifacts byte-identical with distinct inodes. 1950 gate tests, 909 contract tests, capsem-core 1614.
…t behind `module_functional` asked `profiles.selected(config)` for its axis while the plan was being constructed, and that reads `target/config/profiles`. So the same commit produced one plan on a warm tree and a different one on a fresh clone. That is the CI failure. `just release-profile nightly code` passed the complete 57-minute gate locally, pushed, dispatched, and the run died with 94 tests all reporting `no materialized profiles found under target/config/profiles`. The local gate had been green partly on leftovers. `source.record`/`source.verify` never covered it -- they digest tracked source, and this input is not tracked source, so "this gate qualified exactly this revision" was structurally false: two runs at one HEAD could run different plans. No step ordering fixes it. Plan construction is deliberately pure -- built against a runner that refuses every invocation -- so a step's output cannot exist by the time the plan exists. The axis therefore comes from checked-in `config/profiles/`, via `imagebuild.profiles()` rather than a second reader of the same glob. The agreement check did not disappear, it moved to where it can run. `profiles.agree()` asserts materialized == source == manifest, and `functional.axis` performs it: first in the functional module, and after `prepare.materialize-config` in the complete gate. A materialized catalog that disagrees still fails the run; it now fails it from a step instead of preventing the plan from being built at all. Proven by cloning the repo to a directory with no `target/` and planning there: 14 steps, `functional.axis` first. Before this it raised. Mutation-checked by pointing the axis back at build output -- five of six guards red. Also registers `tests/test_gate_git_worktree_mount.py`, which arrived with the worktree-provenance fix (a9e5ae1) and was never added to the source-contract suites, so it ran nowhere. 1989 gate tests, 526 release/build-chain contracts.
"It builds on a cold tree" is the weaker claim. A release lane could plan *something* without build output and still plan a different something -- fewer profiles, a lane skipped -- and then publish on the strength of a proof that never ran. Verified once against reality by cloning to a directory with no `target/` and diffing the dry run of both lanes: zero lines. Asserted here on the described plan so it stays true without needing a clone, and mutation-checked by pointing the axis back at build output, which reds both. Also records what a search for the reported IronBank follow-up actually found: release-plan inspection is byte-identical cold and warm, so that class is closed. The one genuine non-hermetic read left is `scripts/docker-storage-policy.py:1183`, which globs gitignored `target/ironbank-assets/build-*.log` while collecting failure evidence -- and silently collects nothing when they are absent, which is a worse outcome than failing, because the gap shows up in a post-mortem rather than in the run.
`copy_small_file` returned quietly for three different outcomes -- absent, over the size cap, unreadable -- and the IronBank globs yielded nothing on a tree where those builds never ran. So a preserved bundle gave no way to tell "there were no IronBank logs" from "the collector never looked" from "the copy failed", and the gap only became visible during the post-mortem that needed it. Every bundle now carries `collected.json`: each source attempted, and what became of it. Globs are named even when they match nothing, because a build that never ran and a collector that never looked produce identical silence otherwise. That manifest paid for itself on the first real capture, which reported `build.log` and `docker-storage.jsonl` as `too-large`. Every bundle ever written had silently omitted the two files a post-mortem reaches for first -- and omitted them precisely on the long runs large enough to need them. Oversized files are tailed rather than dropped now: the end of a build log is where the failure is. Verified against a real capture, which produced a 25 MiB tail ending at the log's actual last line where it previously produced nothing. Mutation-checked. The first version of the guard asserted only that *something* was reported absent, which passed on `build.log` alone and would have survived deleting the empty-glob record entirely; it now names the IronBank globs.
…sion
Two flags, both previously invisible.
`--network` was never passed anywhere, so every container had outbound access
because nobody had said otherwise, and several lanes fetch dependencies
mid-run. It is now a required keyword with no default: a call site states what
it needs, and the two privileged install containers state `bridge` because
`dpkg -i || apt-get install -f` and a `pnpm install` genuinely need it until
those inputs are baked into the image.
`-v <repo_root>:/src` was the other. `Mount` refuses a source inside the
checkout now, because that flag let `rust-coverage` churn hardlinks on the
host while `linux-rust` read the same inodes through virtiofs -- a race no
`contends` declaration could constrain, since the two steps share nothing
except a filesystem nobody wrote down. It killed a release run with a
`Permission denied` on a file that was `0644` before and `0644` after.
Adds what copying instead of mounting needs: `build`, `create`, `start`,
`copy_out`, `run_once`, `image_exists`. `--rm` and `docker cp` are mutually
exclusive -- a removed container has nothing left to copy from -- which is why
extraction is create/start/cp rather than a flag on `run_once`.
Two checkout mounts remain and are not hidden. `Mount.unmigrated` is
deliberately ugly and greppable, and a test pins the exact set at
`{debproof.py: 1, installcontainer.py: 1}`. The alternative was disabling the
guard globally during the migration, which is how a temporary exemption
becomes the behaviour. A ratchet also pins the nine remaining hand-built
docker argv sites by module, so a tenth fails rather than passing unnoticed.
Named volumes are exempt by Docker's own rule -- a source with no separator is
a volume, not a path -- because resolving one relative to the cwd put it
inside the checkout and refused every legitimate cache.
1995 gate tests, 526 release/build-chain contracts.
The lane bind-mounted the live checkout, grafted two writable mounts back through it to retrieve coverage, inherited four named volumes that survive between runs, and ran with outbound network because nothing in the gate ever passed `--network`. A release died here on an intermittent `Permission denied` reading a file that was `0644` before and after, because `rust-coverage` was churning hardlinks in the tree this lane was reading. It now builds its source into an image, runs `--network none`, and returns coverage through `docker cp`. Nothing is shared, so nothing can be raced; nothing is inherited, so a cold machine and a warm one run the same thing. `cache-ownership`, `output-ownership` and `linux-rust-mountpoints` are gone -- they existed only to repair what root-owned shared state left behind. Sealing it found two fetches nobody had written down. The lane ran `pnpm install` mid-run whenever `frontend/dist` was missing, which it always is inside an image that excludes build output. And `ort` -- ONNX Runtime, pulled in by `magika` for file typing -- downloads a binary from `cdn.pyke.io` inside a build script on every cold build. Cache warming cannot fix the second: a build script re-runs on any fingerprint miss and coverage instrumentation changes every fingerprint. Both now come from the image, with ONNX Runtime taken from Microsoft's official release under `ORT_STRATEGY=system` and `ORT_PREFER_DYNAMIC_LINK=1` (it defaults to static and wants a single-file archive that release does not ship). Proof: with the base image built, `capsem-gate linux-rust` exits 0 in 106s and produces `codecov-linux.json` at 844182 bytes -- the same size as the warm bind-mount lane, over an identical set of 165 files, with 162 of them byte-identical in coverage. Three differ (`capsem-agent/src/main.rs`, `capsem-logger/src/reader.rs`, `capsem-logger/src/schema.rs`); that is consistent with run-to-run nondeterminism rather than a different proof, but it is not the byte-identical result this phase set out to get and is worth checking. `scripts/test-linux-rust.sh` is deliberately untouched: the concurrent test refactoring owns it, and `CAPSEM_LINUX_RUST_OUTPUT_DIR` already parameterizes where it writes. 2015 gate tests.
`just release-profile nightly code` fail-stopped in `contracts.release` on `test_just_test_owns_linux_rust_platform_coverage_through_docker`, which asserts `docker run --rm` is how the parity lane runs. It is not, since 630bb90: the lane creates a container, starts it, copies coverage out and removes it. Six assertions in that test pinned the mechanism rather than the property, and I had run only `tests/test_gate_*.py` before committing the seal -- this test lives in the release contracts, which is exactly the split that catches what the gate module set misses. The test was right to fail, and it found more than a stale string. `_LinuxRustSuite` was still here with nothing constructing it: the whole read-only source mount, the writable grafts, the host-uid `--user`, and the four named volumes, in an action no plan reaches. With it went nine `[hostimage]` settings that only ever existed to repair the consequences of sharing a developer's checkout and cache with a root-owned container -- `tmpfs`, `container_home`, `nextest_dir`, `nextest_mount`, `output_dir`, `container_output`, `alpine`, `writable_source_mounts`, `cached_volumes` -- and `NamedVolume`, whose last user they were. And one real regression, restored. The lane used to raise a named `GateError` on a host that is neither Linux nor macOS. Sealing it replaced that with a bare `else`, so a third platform fell through to the Docker path and would have failed somewhere inside a container instead of naming the host it will not run on. `assert "host.on_macos()" in hostimage` is what noticed, so the fix is the guard coming back, not the assertion being rewritten. Both stale tests are reimplemented, not deleted, and two claims got stronger: `/src:ro` asserted the container could not write the checkout, where the assertion is now that no mount exists at all; and `--network none` is pinned, which no earlier test could assert because it was not true. The removed `nextest` and volume assertions are replaced by the base image tag they moved into, and `--user` by the `USER 1000:1000` now baked into the lane image. Verified: `capsem-gate test-release-contracts` exits 0 with 3165 passed, 18 skipped -- the same module set that stopped the release.
The release run logged this 42 times, and every one was false:
[source-tree] profile.toml: unlink during the run; the gate qualifies
this tree (steps: no step in flight)
No path, no step, and no such event. `shutil.rmtree` deletes through a
directory descriptor -- `os.unlink('profile.toml', dir_fd=5)` -- so the proxy
recorded the bare entry name and passed `kwargs` straight through without ever
reading `dir_fd`. `Watch.is_source` then called `Path('profile.toml').
resolve()`, which anchors to the current working directory. That directory is
the checkout root, so deleting `target/config/profiles/code/profile.toml` --
an ordinary step, in build output -- was reported as a mutation of the tracked
`config/profiles` file that happens to share a basename.
Reproduced before fixing: `shutil.rmtree.avoids_symlink_attacks` is true, and
a spy on `os.unlink` records `('profile.toml', dir_fd=5)`.
This is the guard that exists to catch the `config/profiles` race that killed
a release run, and it was reporting 42 phantom mutations per run. A guard that
cries wolf every run is one nobody reads, and it would have buried the true
positive among them.
Three fixes, because any one alone would have hidden the others:
* `interception` resolves the subject where the call acted -- against
`dir_fd` when given, and through the descriptor itself when the subject is
an integer, as in `os.truncate(fd, n)`. Linux reads the symlink whose
template now comes from config; macOS asks `fcntl(F_GETPATH)`.
* `is_source` refuses to judge a path that is not absolute. That is the
structural half: the first fix repairs the one caller that was found, this
one makes any future loose spelling unjudgeable rather than misattributed.
* `dist`, `packages` and `assets` join `target` as build output. All are
gitignored roots the gate rewrites every run -- `assets/current` is
resynced per architecture and stale `.deb`s are cleared before each build
-- and with only `target` excluded those read as mutating the tree under
qualification.
When a path genuinely cannot be established the event is not judged: a fault
nobody can locate is not evidence, and inventing one is worse than missing it.
`fd_path_template` is in `config/gate.toml` because `test_gate_has_no_literal_
data` is right that a path spelled in a module is a second copy; the tests take
it from the same place rather than hardcoding their own. `_F_GETPATH` stays in
code -- it is a number in the platform ABI, not a path.
Each of the four new tests proves a different part: the relative-path test
drives `_judge` directly with cwd set to the source root, so it cannot pass by
landing outside the tree; the `dir_fd` test deletes a file under `config/`,
where widening the build-output set cannot help it.
Verified: 2018 passed, 18 skipped across tests/test_gate_*.py. The one
deselection, `test_a_live_run_is_never_rotated_away_by_another`, fails
identically at 7805bd8 with no changes applied -- it is a pre-existing
invocation artifact, not a regression, and is filed separately.
`functional.pytest.broad.code` failed two tests at once, and neither was at
fault:
FAILED tests/test_gate_candidate.py::
test_interrogating_the_gate_plan_leaves_the_checkout_alone
AssertionError: reading the plan rewrote the gate's own source state
ERROR tests/test_mock_server_launcher.py::
test_mock_server_replays_recorded_agy_code_assist_setup
rewrote gate-source-state.json ... It wrote
b'{"head": "sentinel", "digest": "sentinel"}'
One test wrote a sentinel the other was blamed for. Both symptoms are one
cause: `test_interrogating...` planted its sentinel in the *real*
`target/gate-source-state.json`, making it the only thing in the suite that
deliberately writes a file the whole suite shares. Under
`pytest -n 4 --dist=loadfile` the workers are separate processes watching one
path, so `conftest._the_running_gate_keeps_its_own_source_state` -- which
snapshots that file around every test -- saw the sentinel appear during an
unrelated test in another worker, blamed it, and restored the file underneath
the test that had written it on purpose.
It passed in `contracts.release`, where it runs single-process. That is the
tell: a guard whose result depends on how pytest was invoked.
The sentinel now goes to `<source_state_file>.probe-<pid>`, inside `target/`
so it is build output rather than tracked source, and per-process so four
workers cannot collide either. The plan runs against a config copied with that
one field changed, so nothing in the run can reach the shared path.
The claim is unchanged and one assertion stronger: reading the plan must leave
the probe at the sentinel, *and* must leave the real state file byte-identical
-- which is the property the racing version could not state, because it was
the one modifying it.
Why a sentinel at all, now written down in the docstring rather than lost: the
file holds the true HEAD during a real gate, so a broken `observing` would
rewrite identical bytes and a plain before/after comparison would pass.
`built_command` is public in `tests/helpers/gate.py` because a test that must
run a plan against a modified config cannot go through `gate_issued`, which
builds its `Context` from the real one.
Verified the race is gone at the root, not merely absent: running both files
together with `-n 4 --dist=loadfile` gives 41 passed, and the shared state
file's mtime is identical before and after.
`fast.audit.pnpm` failed the release:
error: frontend: fast-uri: high: fast-uri vulnerable to host confusion via
backslash authority introducer (>=4.0.0 <4.1.2) GHSA-7p8r-x3mc-p8w7
The override was already there, reading `fast-uri: ">=3.1.2"`, and it resolved
to 4.1.1 -- inside the advisory range. A lower bound below a vulnerable window
is not a bound; it just happens to exclude older versions while admitting the
one that matters. Now `>=4.1.2`, with the advisory written beside it so the
next person widening it can see what the number is for.
Dev-only: it arrives through `@astrojs/check` -> `@astrojs/language-server` ->
`yaml-language-server` -> `ajv`, none of which ships to a user. The audit is
still a release gate, and it is right to be.
Note for whoever hits this next: `pnpm install --lockfile-only` is not enough.
`scripts/audit-pnpm-bulk.py` reads `pnpm list --json --depth Infinity`, which
reports what is *installed*, so the audit kept failing against 4.1.1 in
`node_modules` while the lockfile already said 4.1.2.
Verified: `scripts/audit-pnpm-bulk.py` exits 0 with all four workspaces clean,
and `pnpm run check` is 0 errors, 0 warnings across 795 files.
The release stopped here, exactly as designed, and then handed back a command
that does not exist:
ERROR: release-profile failed -- linux-rust: no Linux parity base image for
capsem-linux-rust-base:03ebe122079926b2. Its dependencies changed; run
`just warm` to build it with network before the gate runs without.
`just warm` was never written. The refusal itself is right -- the lane's tag is
keyed by `Cargo.lock`, `rust-toolchain.toml` and `frontend/pnpm-lock.yaml`, and
building a 25 GB image mid-run would turn a `--network none` lane into a
network build at minute four -- but a refusal is only as good as the way out it
names.
Found by bumping `fast-uri` for GHSA-7p8r-x3mc-p8w7, which re-keyed the image.
`hostimage.py` already carried a note about the last time this happened:
`install-image` and `cross-compile` both dispatched `just _build-host-image`, a
recipe that has never existed, and neither test noticed "because both stopped
at the recipe boundary instead of crossing it". I read that docstring while
sealing the lane and then did the same thing.
So the fix is the class, not the instance:
* `warm-linux-rust-base` is a gate command; `_warm-linux-rust-base` is the
thin recipe that dispatches it, private because `config/public-surface.toml`
locks the public surface at 13 and this is developer machinery.
* the recipe name lives in `config/gate.toml` as `warm_recipe`, so the
refusal and the recipe cannot drift apart.
* `test_every_recipe_the_gate_tells_an_operator_to_run_exists` parses gate
source with `ast` and checks every ``just <recipe>`` named in a *string* --
docstrings excluded, since `hostimage.py`'s note about a recipe that never
existed is the point of that note.
One more false positive fixed on the way: `capsem-gate doctor` scanned every
justfile line containing `capsem-gate `, so the comment explaining this recipe
was parsed as a dispatch of ``linux-rust` ``, trailing backtick included. Three
doctor checks went red on a comment. A comment calls nothing, and a
commented-out dispatch is not a dispatch either.
Verified: `just _warm-linux-rust-base` exits 0 and builds
capsem-linux-rust-base:03ebe122079926b2; `capsem-gate test-release-contracts`
exits 0 with 3171 passed, 18 skipped.
The run-history contention test passed or failed on how pytest was typed. `multiprocessing`'s spawn start method re-imports its target's module in the child using nothing but a copy of the parent's `sys.path`, and `--import-mode=importlib` names test modules `tests.<basename>` without ever putting the repository root on that path. The name resolved under `python -m pytest` -- which contributes the working directory, and is how `pytestsuite` builds every gate invocation -- and not under the `pytest` console script, where the child died on `ModuleNotFoundError: No module named 'tests'` and the parent waited out a sixty-second queue timeout. The worker moves to `tests/helpers/runlog_worker.py`. `tests/` is on `sys.path` unconditionally: the root conftest puts it there before collection, under every invocation and in every xdist worker, which is the same wiring `config/gate.toml` already documents for `helpers.*`. Spawn is untouched -- the test is about cross-process rotation safety, and spawn is the stricter case as well as the macOS default.
`RecordingRunner` records commands and answers "" to anything without a canned reply, which is right for every probe whose output nothing reads. The git common-dir probe is not one of those: `docker_git_metadata_mount` reads the answer and builds a `-v` mount out of it, and refuses the build when the path does not resolve. An ordinary checkout never reaches that code -- its `.git` is a directory and the function returns early -- so the gap only opened from a linked worktree, where `.git` is a file and the probe runs. The plan then died at `package.<arch>.build`, `build-linux-package.sh` was never issued, and nine ordering contracts failed for want of a git answer rather than for anything they assert. Whether the contracts held depended on the shape of the checkout they ran from. The recorder answers that probe from the real repository. Truthfully rather than with a constant: the value becomes a mount argument the same contracts read back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.