Repository navigation
Keep git's background maintenance out of test repositories (engine-free flake) - #1266
Merged
Merged
Conversation
main's Tests run at 74f5e61 failed once in engine-free (3.14): test_a_clean_checkout_at_the_declared_commit_is_accepted raised FileNotFoundError on .git/objects/maintenance.lock inside rulespec_tree_digest. git commit starts `git maintenance run --auto`, which takes that lock, detaches and removes it after the commit has returned. The checkout reference hashes .git: the digest lists every file and then reads each, so it can list the lock and find it gone. Every test helper that commits to a throwaway repository now passes test_support.git_repositories.NO_BACKGROUND_MAINTENANCE, so no maintenance process starts. rulespec_tree_digest still refuses a tree that loses a file while it is hashed, now with a ValueError naming the file instead of an unhandled FileNotFoundError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both independent reviews approved; this takes their non-blocking points. - The modal stage plan test runs _verify_branch, whose own git fetch started background maintenance in its scratch clone. Git run by the code under test reads os.environ, so test_support.git_repositories now also gives the settings as GIT_CONFIG_* variables (NO_BACKGROUND_MAINTENANCE_ENV), and that test sets them. New tests: the branch check starts no maintenance (trace2), and the variables set exactly what the -c options set. - Use maintenance.autoDetach=false, not its fallback gc.autoDetach. - The digest's refusal says what to do for any root, then for a git checkout; the docstrings name the refusal where compiles re-hash. - docs/agent-guide.md: tests that commit or fetch in a throwaway repository pass these settings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Wording from the second review, and one skip guard; no behaviour change. - Changelog and guide: tests that commit or fetch apply the settings as -c options or, where the code under test runs git, as variables; a helper whose environment carries the variables needs no options; code that strips git's local variables drops them. - NO_BACKGROUND_MAINTENANCE_ENV overrides GIT_CONFIG_COUNT entries, not every GIT_CONFIG_* variable. - The digest's refusal says a git process "can" remove the file. - The variables-versus-options test skips without git, like the rest of its section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Fixes a flake in the
engine-freejob. No issue tracked it; the NZ hub reported it on 2026-10-13.The failure
microcosm
main's Tests run at 74f5e61 (the #1254 merge) failed inengine-free (3.14)(run 38405652457):The runs either side passed. It is a flake, not #1254's content. Since the checkout tests merged (#1118, 2026-10-07), 356 Tests runs have started: 193 passed, 49 failed, 108 were cancelled and the rest are still running. 43 of the failed runs have a failed
engine-freejob. I read the 42 whose logs GitHub still serves, and this is the only one that namesmaintenance.lock(the unreadable one, run 37653545314, failed in bothengine-freejobs at once).Cause (confirmed)
git commitstarts a maintenance process. In git 2.55.0 (the version on the CI runner and on the host I measured on),builtin/commit.ccallsrun_auto_maintenance, andprepare_auto_maintenance(run-command.c) startsgit maintenance run --auto --detachunlessmaintenance.autois false, ormaintenance.autois unset andgc.autois 0 or less. The test helper_gitset neither. A trace2 event file shows that child in 200 of 200 commits made with the helper's options.maintenance_run_tasks(builtin/gc.c) takes<objects>/maintenance.lock, runs its foreground tasks, detaches, runs its background tasks, then removes the lock. Checked the instantgit commitreturned, the lock was still there in 22, 94 and 32 of 200 commits in three runs, and stayed for up to 34 ms..git, in two steps.rulespec_tree_digestlists every file under the root (rglobandis_file), then reads each. A lock that is listed and then removed raisesFileNotFoundErrorat the read. That is the frame in the CI traceback.So the test does not fail on what it asserts. It fails when the detached maintenance process removes its lock between the digest's listing and its read.
The fix
Tests: no test repository starts background maintenance
New
test_support/git_repositories.pydefinesNO_BACKGROUND_MAINTENANCE:-c maintenance.auto=false -c gc.auto=0 -c maintenance.autoDetach=false. Beside it,NO_BACKGROUND_MAINTENANCE_ENVgives the same three settings asGIT_CONFIG_COUNTenvironment variables, for git that the code under test runs itself fromos.environ. Every test that commits or fetches in a throwaway repository now uses one of them:.gititself?packages/microcosm-frame/tests/engine_free/shared/test_axiom_graph_adapter.py(_git, used for every repository in the file)packages/microcosm-build/tests/engine_free/us/test_us_chronicle_feed.pypackages/microcosm-build/tests/engine_free/us/test_us_modal_stage_plan_tool.py_GIT_ENV, which reach both its helper and thegit fetchthat_verify_branchrunspackages/microcosm-build/tests/engine_workflow/us/test_us_annual_static_aging.pytest_support/microcosm_graph/graph_acceptance_burndown.pyOnly the first can hit this failure. The code the other four exercise never reads
.gitas files: it asks git (status,ls-files,ls-tree,rev-parse,show,fetch). They get the same settings so that no test repository has a process still writing to it after the command returns.git fetchstarts the same maintenance process asgit commit(a trace2 file shows it in the branch check's fetch into its scratch clone, and none with the variables set). The remaining test-side git calls (test_route_a_driver.py,test_us_modal_stage_puf_support_base.py,test_uk_staging_integration.py,test_logbook_adoption.py) only read this repository or mock git, and are unchanged.docs/agent-guide.mdnow tells new tests to do the same.Each option closes the race on its own (table below), so the three together do not depend on which one a given git honours.
Production:
rulespec_tree_digestrefuses by nameThe same race exists on a real checkout. If
axiom_engine_refis given a git checkout root while git's background maintenance (or any other git process) is still working in it, the digest can list a lock and find it gone, or hash a lock that is then removed, which moves the reference..git. That is deliberate and documented. The digest must equal the graph's directory source key (microcosm.graph.keys._directory_identity, held bytest_differential_digest_equals_the_graph_directory_identity), and a checkout reference is meant to be clone-specific (test_a_checkout_reference_is_clone_specific_and_an_export_is_not). Leaving.gitout would change what a checkout reference names. That is a design change, not a flake fix.ValueError, naming the file and the remedy (wait for git, or reference agit archiveexport), where it was an unhandledFileNotFoundError.axiom_engine_refdocumentsValueErrorfor every other refusal. No digest value changes.The docstrings of
rulespec_tree_digestandaxiom_engine_refnow say this. The graph's own_directory_identityis unchanged: it still raisesFileNotFoundErrorin the same case.Invariants
rulespec_tree_digestreturns what it returned before, equal to_directory_identity(existing Hypothesis differential, 60 examples; unchanged and passing).ValueErrornaming it, and the settled tree then hashes to the graph's identity (new Hypothesis property, 40 examples).git maintenanceorgit gcprocess and leaves nomaintenance.lock; nor does the branch check's fetch (two new tests; they read a trace2 event file, so they do not depend on timing).git config --list --show-scopelists the same command-scope entries under both (new test, the consistency checkdocs/shared-constants.mdasks for of a derived view).New tests
test_a_commit_in_a_test_repository_starts_no_git_maintenancegit maintenance run --auto --quiet --detachtest_a_lock_git_removes_while_the_checkout_is_hashed_is_refused(the CI failure, reproduced without timing)FileNotFoundErroron.git/objects/maintenance.locktest_property_a_file_removed_after_the_listing_is_refused_by_nameFileNotFoundErrortest_the_branch_check_starts_no_git_maintenance(plan tool)git maintenance run --auto … --detachtest_the_maintenance_variables_set_what_the_options_set(plan tool)I measured the middle column in this checkout:
NO_BACKGROUND_MAINTENANCEemptied for the first row,axiom.pyrestored frommainfor the next two, andNO_BACKGROUND_MAINTENANCE_ENVleft out of the plan tool's_GIT_ENVfor the fourth.Before and after
One host throughout: macOS 26.6.2, 18 CPUs at a load average of 50 to 100, git 2.55.0, CPython 3.14.7 (free-threading build). Never more than 4 pytest workers in total;
OMP_NUM_THREADS=4. "Before" is a separate worktree oforigin/main4b33e3c with its own environment. The host's load control paused some runs for a while and then resumed them; a pause changes timing only, and no run failed.main)maintenance.lockstill present when the helper'sgit commitreturns (200 commits, three runs)rulespec_tree_digestcalled twice straight after the fixture's commit (3,000 repositories): a listed file was gone at the read.git/objects/maintenance.lock-n 4-n 2: 15 side by side with the PR's first commit 1123be8, then 15 alternating with 219e0b4The exact test does not fail on this host, before or after. In that test the digest starts 134 to 242 ms after the commit returns (median 175 ms, 100 runs), because the fixture and the clean-checkout checks run eight git commands first. Here the lock lasted at most 34 ms in my runs (the first independent reviewer saw up to 149 ms with empty commits). The CI failure shows it outlived that gap on the runner at least once; I did not measure on a runner. On CI, the 193 passing runs each ran two
engine-freejobs against this one failure, about 1 in 400 jobs, so 30 local runs could not be expected to show it either way. The second and third rows are the same race with that delay removed: the fixture's owngit commitand the production digest, nothing mocked.Lock observation by option, 200 commits each:
git maintenancemain)gc.auto=0maintenance.auto=falsemaintenance.autoDetach=falsegc.autoDetach=falseThe second column comes from a trace2 event file per commit; the third is checked the instant
git commitreturns.Review
Three commits, each reviewed independently; every review approved with no blocking findings.
review, hard tier)_verify_branchruns its owngit fetch, which starts the same maintenance process. Covered in 219e0b4.maintenance.autoDetachrather than its fallbackgc.autoDetach; hedge the error message for non-git roots; a guide note for new tests; changelog wording. All in 219e0b4.Left as is, on purpose:
tools/modal_us_stage_plan.py) still lets its fetch start maintenance in its scratch clone, which_verify_branchthen deletes withrmtree(..., ignore_errors=True). That races only a temporary directory, and nothing has failed there.git fetch --no-auto-maintenancewould close it if it ever matters.microcosm.graph.keys._directory_identityhas the same list-then-read loop and still raisesFileNotFoundError. Digest values agree (the differential test); only the error type differs._require_clean_checkoutcan still raiseFileNotFoundErrorif a git command rewrites the work tree during the check. Background maintenance cannot cause that:_checkout_filesskips the top-level.git.axiom: n/a (test infrastructure and an error type; no policy change)
🤖 Generated with Claude Code