Repository navigation
fix(deploy,update): controller-only manifest paths, and a provenance marker that bricks pip (#17331, #17332, #17246) - #17338
Merged
Merged
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 24 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (21)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
mrveiss
changed the title
This was referenced Sep 23, 2026
mrveiss
changed the title
mrveiss
force-pushed
the
issue-17331-manifest-paths
branch
from
September 23, 2026 22:14
e70493c to
adb46b4
Compare
mrveiss
added a commit
that referenced
this pull request
Sep 23, 2026
…3049, #17142) Rebased onto origin/main 3f0d4b3. Two declarations were red, both because main sits at EXACTLY its allowance and this branch adds counting files. hooks-path-override 6536 -> 6800 population 6938 (6907 glob hits for *.sh/*.py/*.yml/*.yaml, plus 31 extensionless files whose shebang says shell), completed 6937 (`read` counts opened files, minus the one EXEMPT). At 6536: slack 402 against growth 400 + skips 1 = 401. One over. Window [6537, 6937]. prompt-injection-detector-strict-mode 2956 -> 3100 population 3257 non-test .py under autobot-backend/ (origin/main: 3256, slack exactly 300 = the whole growth allowance, skips defaults to 0). This branch adds accelerate_loader.py, so 301. Window [2957, 3257]; no completed() call on this declaration, so verify_floor is the only bound. audio-extension-allowlist needed nothing: already pinned to 5700 on this branch, population 6152 against window [5452, 5766]. NEITHER IS PINNED TO THE MINIMUM, and that is the point. hooks-path-override has been re-pinned SIXTEEN times, and its own comment block records why each time: every re-pin took the bottom of the window, so the next file main added was another red, and the branch paying for it was never the branch that caused it. 6800 and 3100 are mid-window -- slack 138 and 157 against allowances of 401 and 300 -- which buys ~260 and ~143 files of headroom instead of zero. The reasoning is autobot-ai-58's, from #17338. 6800 is deliberately 58's value rather than a fourth number for one measurement: if #17338 lands first this line is identical and there is nothing to resolve. autobot-ai-f8 proposed 6736 for the post-#17318 tree, also inside the window; 6800 leaves more headroom. Rule agreed between the three sessions: first to land wins, the others adopt rather than re-derive. 3100 is new -- no peer has a re-pin of that declaration in flight. Verified with the declaring module explicitly in the session: pytest autobot-backend/security/prompt_injection_detector_strict_mode_test.py \ repo_tests/reach_declarations_test.py -> 192 passed That invocation is not incidental. The declaration's own docstring warns it reaches the sweep only when something else has already imported its module: running repo_tests/ alone drops the parameter count from 36 to 35 and reports a clean pass over a floor it never checked. #17142 remains the real fix: a 400-file growth allowance on a ~6900-file tree is 5.8%, and sixteen re-pins is the evidence that the allowance is what is wrong. Refs #13049, #17142, #17338
3 tasks
mrveiss
added a commit
that referenced
this pull request
Sep 23, 2026
…, re-pin two floors (#14881, #14631) Three failures, diagnosed from the run log rather than inferred. The log only became readable once the WHOLE run finished -- a job's log is not served while any shard of its run is still going, which is #17351. shard 10 one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks 34 direct `git ls-files` invocations in repo_tests/, MAX says 33 shard 12 reach_declarations_test[hooks-path-override] floor 6536 sits 403 below population 6939, allowance 401 shard 12 reach_declarations_test[import-hermeticity] floor 602 sits 61 below population 663, allowance 60 THE FIRST ONE IS MINE, and it is the one worth recording. enum_hand_copy_guard shelled out to `git ls-files` for itself. MAX_DIRECT_INVOCATIONS only ever shrinks, so that is a regression even though the guard worked -- a new fork of the repo's enumeration helper, added in a PR whose subject is hand-copied definitions. Routed through tools.lint._scan_helpers.tracked_paths, which also gets the exclusion right: `exclude` becomes a git `:(exclude)` pathspec so git does the matching, rather than a second matcher in Python that can disagree with the first (#15510). subprocess and scrubbed_git_env are no longer imported. THE OTHER TWO ARE MAIN'S, and this branch pays for them: hooks-path-override 6536 -> 6800. Population 6939, completed 6938, window [6537, 6938]. Main had spent 401 of its 401 allowance before this branch existed. Same 6800 as autobot-ai-58's #17338 and this session's #13049 branch -- one measurement should not produce three numbers, and if any two land the line is identical. import-hermeticity 602 -> 633. A FOURTH floor at zero headroom on main that nobody had named: 662 non-test modules against floor 602 is slack 60, exactly the allowance (skips=0). This branch adds autobot_shared/code_review_enums.py -- the shared home the four hand-copied enums moved to -- so 663, one over. 633 is mid-window of [603, 663]; the declaration's own comment records this population growing ~52 modules per 30 days, so the minimum 603 would be red again within three weeks of ordinary work. Measured through the declaration's own discover (repo_tests._import_hermeticity_scope.entries), not a re-derived glob. NEITHER FLOOR IS PINNED TO ITS MINIMUM. hooks-path-override has been re-pinned sixteen times and its own comment block records why every time: pinned at the bottom of the window, the next file main adds is another red, and the branch that pays is never the branch that caused it. #17142 is the real fix -- a 400-file growth allowance on a ~6900-file tree is 5.8%. Verified on this tree, rebased onto origin/main 3f0d4b3: one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency 50 passed prompt_injection_detector_strict_mode_test + reach_declarations_test 192 passed The second invocation is not incidental: reach_declarations_test only sees the prompt-injection declaration when something else in the session has imported its module, so running repo_tests/ alone drops the sweep from 36 parameters to 35 and reports a clean pass over a floor it never checked. Refs #14881, #14631, #17142, #17351
mrveiss
added a commit
that referenced
this pull request
Sep 23, 2026
…re-empt a zero-headroom floor (#16688) Both changes are pre-emptive: this branch was GREEN when they were made. It was green against a base that has since moved, which is the whole point. THE CENSUS GUARD WAS A SECOND FORK OF THE ENUMERATION. redaction_concept_census_test shelled out to `git ls-files` for itself, exactly as enum_hand_copy_guard did on the #14881 branch. one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks caught that one in CI; rebasing this branch onto origin/main 3f0d4b3 reproduces it here -- 34 invocations against a MAX of 33 -- because main reached 33 after this branch's last run. Found by predicting it and checking, not by a red. A census guard forking the repo's enumeration, in a PR whose subject is forks, is the joke writing itself. Routed through tools.lint._scan_helpers.tracked_paths, whose `exclude` becomes a git `:(exclude)` pathspec so git does the matching -- rather than the second Python-side matcher this had, which is how #15510 fired an exclusion against absolute paths and matched every file. subprocess and scrubbed_git_env are no longer imported. HOOKS-PATH-OVERRIDE 6536 -> 6800 WHILE PASSING. Measured on this rebased tree: population 6937 (glob hits for *.sh/*.py/*.yml/*.yaml plus 31 extensionless files whose shebang says shell), completed 6936, slack 401 against growth 400 + skips 1 = exactly 401. That is not "passing", it is "passing until the next file anyone lands", and four sessions moved main in the hour this branch sat in review. The cost of pre-empting is one line on a hub file three other PRs already change to the same value; the cost of not is an hour of CI and a round trip on a predictable red. 6800 is mid-window of [6536, 6936], not the minimum -- fifteen previous re-pins took the bottom, which is why there were fifteen. It is deliberately the same value as autobot-ai-58's #17338 and this session's #17342 and #17346: one measurement should not produce four numbers, and identical lines cannot conflict. First to land wins, the rest adopt on rebase. The other three floors were measured here and left alone, because this branch does not move their populations: audio-extension-allowlist 6151/5764 against the 5700 pinned earlier on this branch (margin 64), prompt-injection-detector-strict-mode 3256 (no new backend non-test module), import-hermeticity 662 (no new module under api/ or autobot_shared/). Two of those sit at exactly zero headroom on main and will need re-pinning by whichever branch next adds to their roots -- see #17142. Verified on this tree: one_git_enumeration + redaction_concept_census + threat_model_anchors + prompt_injection_detector_strict_mode + reach_declarations ... 274 passed reach_declarations_test is run with the prompt-injection module in the same session on purpose: its declaration reaches the sweep only when something else has imported it, so repo_tests/ alone silently checks 35 of 36 floors. Refs #16688, #15926, #17142
…he controller's checkout (#17331, #17246) #17242 moved the generation of the filtered backend manifest onto the controller. It did not move the path that generation writes INTO the file, so provisioning still aborted on every non-manager host -- one task later, inside pip: ERROR: Could not open constraint file: [Errno 2] No such file or directory: '<base_dir>/code_source/constraints/shared.txt' Confirmed from a two-host run, not inferred. pip resolves a nested `-c` against the directory of the file containing it, so the path the rewrite bakes in is the path pip opens on the target -- where code_source does not exist. - #17331 build-filtered-requirements.sh takes the rewrite root as its own argument, separate from the source root. Omitting it keeps both equal, which is correct for services/role_registry.py's post_sync_cmd (reads and installs on the manager). The five delegated ansible generations pass the node's base_dir; the three GPU manifests had no relative includes to rewrite, so that was latent rather than failing. - #17246 six of the seven baselined pip sites now read a staged constraints/shared.txt. The seventh (npu-worker) reads its whole manifest from the checkout and that manifest editable-installs autobot_shared, which nothing else puts in that venv; routing it through the filter script would strip that line and silently drop the dependency. Filed as #17334 with the decision it needs, and it stays as the single remaining baseline entry. - roles/_shared/tasks/stage_shared_manifests.yml stages constraints/shared.txt and the root requirements.txt onto the node, guarded by a per-host fact and tagged `always` for the reason update-all-nodes.yml gives at its own staging task. Each role includes it, so a role carries its own precondition whatever playbook ran it. - A new guard covers the generated-CONTENT case the #17243 guard structurally cannot see: that one reads task payloads, and this path only exists inside a file a shell script writes at run time. Both mutations named in its docstring were executed and both went red. - #17242's false invariant comment ("code_source ... always exists as the git checkout") is corrected -- it is the sentence that hid this half. Verified: 32 ansible guards and 15 rewrite tests pass locally; reach_declarations_test 185 passed. Local passes are not CI (#17144).
repo_tests/glob_declared_reads_15900_test.py keeps a bidirectional record of which guards declare each uncovered glob. A new guard that reads *.yml must appear in it, or the record is wrong in a way a maintainer would trust.
…ir scan (#17331) repo_tests/ansible_shared_tasks_code_source_dir_default_16750_test.py scans every _shared/tasks/ file for a templated code_source_dir with no | default(...), and it reads comments as well as tasks. The prose describing the defect being fixed quoted the bad pattern verbatim, which the scan cannot distinguish from a real template.
…nd clear the husks it left (#17332) The #15067 marker is written inside a distribution's dist-info and was not listed in that distribution's RECORD. pip deletes the paths RECORD names and then the directory if nothing is left, so every upgrade of a stamped package left a dist-info holding one file. Python then reports a second distribution for that name at version None and the next pip run that must uninstall it aborts: error: uninstall-no-record-file x Cannot uninstall cachetools None -- no RECORD file was found Twelve packages on one host were already in that state, found in tonight's provisioning run. pip stops at the first, so each failed deploy reveals one. - write_provenance_marker now lists the marker in RECORD, which makes the module's central claim -- "a reinstall erases the marker" -- true instead of assumed. Empty hash and size columns, which is what pip writes for RECORD itself. Idempotent, because the reconciler re-stamps every declared package on every run. - clear_provenance_husks repairs a whole venv in one pass before pip runs, so a host with twelve does not need twelve failed deploys. On #17038: what it removes is tool-authored debris, admitted only when the directory describes no installed distribution AND its sole content is this module's own marker. It cannot reach a real installation, an operator's files, or another tool's, and each removal is logged. - has_tool_provenance refuses a husk. It used to answer "ours" for a distribution that is not installed, which would let a removal be authorised by the debris of an earlier one. - The module docstring's false invariant is corrected at the sentence that states it. Three existing fixtures built a dist-info with mkdir() alone -- shaped exactly like the husk -- so they are now realistic installs via one shared helper. _StubPath grows `parents` modelled on pathlib's sequence rather than a single plausible value. Verified: 48 venv tests and 98 code-sync tests pass; the mutation (drop the RECORD listing) turns three of the new tests red, including the upgrade sequence itself. The new suite carries a contrast test asserting the modelled uninstall DOES produce a husk without the fix, so the passing tests cannot be satisfied by a simulation that never removes anything. pip's behaviour is read from its source, not assumed: req_uninstall.uninstallation_paths yields dist.iter_declared_entries() and raises UninstallMissingRecord when RECORD is absent. Local passes are not CI (#17144). Refs #17332
…ater files (#17332) The sys.modules leak guard blocked the push, and the leak was not in the new file it was reporting against: test_worker_component_resolve_12450.py installed `multipart` and `multipart.multipart` stubs at module import and never removed them, so they outlived it and were whatever the next test file to import happened to inherit. The guard names the file that SEES the leak, which is why this surfaced only once the venv tests ran in the same selection. The stubs are needed while `api.code_sync` is imported and are dead weight afterwards, since the module object is cached. `_load_code_sync()` installs them, imports, and removes them in one try/finally, restoring whatever was there before. The ten call sites now go through it. Verified: 117 tests pass across test_worker_component_resolve_12450.py, test_code_sync_deploy_bugs.py and test_venv_provenance.py. Nothing was added to repo_tests/sys_modules_leak_baseline.txt -- that list only shrinks. Refs #17332
… one (#17332) The count was measured in autobot-backend/venv alone and written as "twelve packages on one host". A one-venv measurement reported with host-wide language, which is the shape of error this repo treats as serious: a reader greps that venv, confirms twelve, and stops. Re-measured across every venv under the install tree, read-only: autobot-backend/venv 12 husks autobot-slm-backend/venv 1 husk psycopg2_binary-2.9.12.dist-info the other five venvs 0 The thirteenth matters beyond arithmetic. It is in the SLM's OWN venv -- the component that runs the builtin updater, which is the only path clear_provenance_husks is wired into (#17339). Its live sibling psycopg2_binary-2.9.13.dist-info carries a 46-line RECORD while the husk holds one file, so it is the same upgrade-leak mechanism and not a second defect. Found by autobot-ai-f8, who swept every venv rather than the one the issue named, and verified independently here before changing the text. Refs #17332
… leave (#17331, #17332) #17318 landed hooks-path-override 6736 and audio-extension-allowlist 5752 while this branch carried 6800 and 5700. Both of main's values are inside the windows measured here, so this branch takes them rather than re-deriving competing numbers for one measurement -- first to land wins, the others adopt. Re-measured after the rebase to confirm they hold WITH this branch's files, which is the half that adopting a number normally skips: audio : population 6154 completed 5767 floor 5752 gap 402/700 headroom 15 hooks : population 6941 completed 6940 floor 6736 gap 205/401 headroom 204 Both pass. Audio's 15 is thin and is recorded at the site: that declaration's loop skips `repo_tests` outright, so its `completed` ceiling FALLS as guards are added while population rises -- the two bounds close from opposite directions, and only the gap one is ever measured. Not re-pinning it here; one branch widening a floor another just set is how two correct numbers become a conflict. Carried to #17142 instead. Refs #17142
mrveiss
added a commit
that referenced
this pull request
Sep 23, 2026
…3049, #17142) Rebased onto origin/main 3f0d4b3. Two declarations were red, both because main sits at EXACTLY its allowance and this branch adds counting files. hooks-path-override 6536 -> 6800 population 6938 (6907 glob hits for *.sh/*.py/*.yml/*.yaml, plus 31 extensionless files whose shebang says shell), completed 6937 (`read` counts opened files, minus the one EXEMPT). At 6536: slack 402 against growth 400 + skips 1 = 401. One over. Window [6537, 6937]. prompt-injection-detector-strict-mode 2956 -> 3100 population 3257 non-test .py under autobot-backend/ (origin/main: 3256, slack exactly 300 = the whole growth allowance, skips defaults to 0). This branch adds accelerate_loader.py, so 301. Window [2957, 3257]; no completed() call on this declaration, so verify_floor is the only bound. audio-extension-allowlist needed nothing: already pinned to 5700 on this branch, population 6152 against window [5452, 5766]. NEITHER IS PINNED TO THE MINIMUM, and that is the point. hooks-path-override has been re-pinned SIXTEEN times, and its own comment block records why each time: every re-pin took the bottom of the window, so the next file main added was another red, and the branch paying for it was never the branch that caused it. 6800 and 3100 are mid-window -- slack 138 and 157 against allowances of 401 and 300 -- which buys ~260 and ~143 files of headroom instead of zero. The reasoning is autobot-ai-58's, from #17338. 6800 is deliberately 58's value rather than a fourth number for one measurement: if #17338 lands first this line is identical and there is nothing to resolve. autobot-ai-f8 proposed 6736 for the post-#17318 tree, also inside the window; 6800 leaves more headroom. Rule agreed between the three sessions: first to land wins, the others adopt rather than re-derive. 3100 is new -- no peer has a re-pin of that declaration in flight. Verified with the declaring module explicitly in the session: pytest autobot-backend/security/prompt_injection_detector_strict_mode_test.py \ repo_tests/reach_declarations_test.py -> 192 passed That invocation is not incidental. The declaration's own docstring warns it reaches the sweep only when something else has already imported its module: running repo_tests/ alone drops the parameter count from 36 to 35 and reports a clean pass over a floor it never checked. 5.8%, and sixteen re-pins is the evidence that the allowance is what is wrong. Refs #13049, #17142, #17338
mrveiss
force-pushed
the
issue-17331-manifest-paths
branch
from
September 23, 2026 23:20
adb46b4 to
da45434
Compare
mrveiss
added a commit
that referenced
this pull request
Sep 23, 2026
…, re-pin two floors (#14881, #14631) Three failures, diagnosed from the run log rather than inferred. The log only became readable once the WHOLE run finished -- a job's log is not served while any shard of its run is still going, which is #17351. shard 10 one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks 34 direct `git ls-files` invocations in repo_tests/, MAX says 33 shard 12 reach_declarations_test[hooks-path-override] floor 6536 sits 403 below population 6939, allowance 401 shard 12 reach_declarations_test[import-hermeticity] floor 602 sits 61 below population 663, allowance 60 THE FIRST ONE IS MINE, and it is the one worth recording. enum_hand_copy_guard shelled out to `git ls-files` for itself. MAX_DIRECT_INVOCATIONS only ever shrinks, so that is a regression even though the guard worked -- a new fork of the repo's enumeration helper, added in a PR whose subject is hand-copied definitions. Routed through tools.lint._scan_helpers.tracked_paths, which also gets the exclusion right: `exclude` becomes a git `:(exclude)` pathspec so git does the matching, rather than a second matcher in Python that can disagree with the first (#15510). subprocess and scrubbed_git_env are no longer imported. THE OTHER TWO ARE MAIN'S, and this branch pays for them: hooks-path-override 6536 -> 6800. Population 6939, completed 6938, window [6537, 6938]. Main had spent 401 of its 401 allowance before this branch existed. Same 6800 as autobot-ai-58's #17338 and this session's #13049 branch -- one measurement should not produce three numbers, and if any two land the line is identical. import-hermeticity 602 -> 633. A FOURTH floor at zero headroom on main that nobody had named: 662 non-test modules against floor 602 is slack 60, exactly the allowance (skips=0). This branch adds autobot_shared/code_review_enums.py -- the shared home the four hand-copied enums moved to -- so 663, one over. 633 is mid-window of [603, 663]; the declaration's own comment records this population growing ~52 modules per 30 days, so the minimum 603 would be red again within three weeks of ordinary work. Measured through the declaration's own discover (repo_tests._import_hermeticity_scope.entries), not a re-derived glob. NEITHER FLOOR IS PINNED TO ITS MINIMUM. hooks-path-override has been re-pinned sixteen times and its own comment block records why every time: pinned at the bottom of the window, the next file main adds is another red, and the branch that pays is never the branch that caused it. #17142 is the real fix -- a 400-file growth allowance on a ~6900-file tree is 5.8%. Verified on this tree, rebased onto origin/main 3f0d4b3: one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency 50 passed prompt_injection_detector_strict_mode_test + reach_declarations_test 192 passed The second invocation is not incidental: reach_declarations_test only sees the prompt-injection declaration when something else in the session has imported its module, so running repo_tests/ alone drops the sweep from 36 parameters to 35 and reports a clean pass over a floor it never checked. Refs #14881, #14631, #17142, #17351
mrveiss
added a commit
that referenced
this pull request
Sep 23, 2026
…re-empt a zero-headroom floor (#16688) Both changes are pre-emptive: this branch was GREEN when they were made. It was green against a base that has since moved, which is the whole point. THE CENSUS GUARD WAS A SECOND FORK OF THE ENUMERATION. redaction_concept_census_test shelled out to `git ls-files` for itself, exactly as enum_hand_copy_guard did on the #14881 branch. one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks caught that one in CI; rebasing this branch onto origin/main 3f0d4b3 reproduces it here -- 34 invocations against a MAX of 33 -- because main reached 33 after this branch's last run. Found by predicting it and checking, not by a red. A census guard forking the repo's enumeration, in a PR whose subject is forks, is the joke writing itself. Routed through tools.lint._scan_helpers.tracked_paths, whose `exclude` becomes a git `:(exclude)` pathspec so git does the matching -- rather than the second Python-side matcher this had, which is how #15510 fired an exclusion against absolute paths and matched every file. subprocess and scrubbed_git_env are no longer imported. HOOKS-PATH-OVERRIDE 6536 -> 6800 WHILE PASSING. Measured on this rebased tree: population 6937 (glob hits for *.sh/*.py/*.yml/*.yaml plus 31 extensionless files whose shebang says shell), completed 6936, slack 401 against growth 400 + skips 1 = exactly 401. That is not "passing", it is "passing until the next file anyone lands", and four sessions moved main in the hour this branch sat in review. The cost of pre-empting is one line on a hub file three other PRs already change to the same value; the cost of not is an hour of CI and a round trip on a predictable red. 6800 is mid-window of [6536, 6936], not the minimum -- fifteen previous re-pins took the bottom, which is why there were fifteen. It is deliberately the same value as autobot-ai-58's #17338 and this session's #17342 and #17346: one measurement should not produce four numbers, and identical lines cannot conflict. First to land wins, the rest adopt on rebase. The other three floors were measured here and left alone, because this branch does not move their populations: audio-extension-allowlist 6151/5764 against the 5700 pinned earlier on this branch (margin 64), prompt-injection-detector-strict-mode 3256 (no new backend non-test module), import-hermeticity 662 (no new module under api/ or autobot_shared/). Two of those sit at exactly zero headroom on main and will need re-pinning by whichever branch next adds to their roots -- see #17142. Verified on this tree: one_git_enumeration + redaction_concept_census + threat_model_anchors + prompt_injection_detector_strict_mode + reach_declarations ... 274 passed reach_declarations_test is run with the prompt-injection module in the same session on purpose: its declaration reaches the sweep only when something else has imported it, so repo_tests/ alone silently checks 35 of 36 floors. Refs #16688, #15926, #17142
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…3049, #17142) Rebased onto origin/main 3f0d4b3. Two declarations were red, both because main sits at EXACTLY its allowance and this branch adds counting files. hooks-path-override 6536 -> 6800 population 6938 (6907 glob hits for *.sh/*.py/*.yml/*.yaml, plus 31 extensionless files whose shebang says shell), completed 6937 (`read` counts opened files, minus the one EXEMPT). At 6536: slack 402 against growth 400 + skips 1 = 401. One over. Window [6537, 6937]. prompt-injection-detector-strict-mode 2956 -> 3100 population 3257 non-test .py under autobot-backend/ (origin/main: 3256, slack exactly 300 = the whole growth allowance, skips defaults to 0). This branch adds accelerate_loader.py, so 301. Window [2957, 3257]; no completed() call on this declaration, so verify_floor is the only bound. audio-extension-allowlist needed nothing: already pinned to 5700 on this branch, population 6152 against window [5452, 5766]. NEITHER IS PINNED TO THE MINIMUM, and that is the point. hooks-path-override has been re-pinned SIXTEEN times, and its own comment block records why each time: every re-pin took the bottom of the window, so the next file main added was another red, and the branch paying for it was never the branch that caused it. 6800 and 3100 are mid-window -- slack 138 and 157 against allowances of 401 and 300 -- which buys ~260 and ~143 files of headroom instead of zero. The reasoning is autobot-ai-58's, from #17338. 6800 is deliberately 58's value rather than a fourth number for one measurement: if #17338 lands first this line is identical and there is nothing to resolve. autobot-ai-f8 proposed 6736 for the post-#17318 tree, also inside the window; 6800 leaves more headroom. Rule agreed between the three sessions: first to land wins, the others adopt rather than re-derive. 3100 is new -- no peer has a re-pin of that declaration in flight. Verified with the declaring module explicitly in the session: pytest autobot-backend/security/prompt_injection_detector_strict_mode_test.py \ repo_tests/reach_declarations_test.py -> 192 passed That invocation is not incidental. The declaration's own docstring warns it reaches the sweep only when something else has already imported its module: running repo_tests/ alone drops the parameter count from 36 to 35 and reports a clean pass over a floor it never checked. 5.8%, and sixteen re-pins is the evidence that the allowance is what is wrong. Refs #13049, #17142, #17338
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…, re-pin two floors (#14881, #14631) Three failures, diagnosed from the run log rather than inferred. The log only became readable once the WHOLE run finished -- a job's log is not served while any shard of its run is still going, which is #17351. shard 10 one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks 34 direct `git ls-files` invocations in repo_tests/, MAX says 33 shard 12 reach_declarations_test[hooks-path-override] floor 6536 sits 403 below population 6939, allowance 401 shard 12 reach_declarations_test[import-hermeticity] floor 602 sits 61 below population 663, allowance 60 THE FIRST ONE IS MINE, and it is the one worth recording. enum_hand_copy_guard shelled out to `git ls-files` for itself. MAX_DIRECT_INVOCATIONS only ever shrinks, so that is a regression even though the guard worked -- a new fork of the repo's enumeration helper, added in a PR whose subject is hand-copied definitions. Routed through tools.lint._scan_helpers.tracked_paths, which also gets the exclusion right: `exclude` becomes a git `:(exclude)` pathspec so git does the matching, rather than a second matcher in Python that can disagree with the first (#15510). subprocess and scrubbed_git_env are no longer imported. THE OTHER TWO ARE MAIN'S, and this branch pays for them: hooks-path-override 6536 -> 6800. Population 6939, completed 6938, window [6537, 6938]. Main had spent 401 of its 401 allowance before this branch existed. Same 6800 as autobot-ai-58's #17338 and this session's #13049 branch -- one measurement should not produce three numbers, and if any two land the line is identical. import-hermeticity 602 -> 633. A FOURTH floor at zero headroom on main that nobody had named: 662 non-test modules against floor 602 is slack 60, exactly the allowance (skips=0). This branch adds autobot_shared/code_review_enums.py -- the shared home the four hand-copied enums moved to -- so 663, one over. 633 is mid-window of [603, 663]; the declaration's own comment records this population growing ~52 modules per 30 days, so the minimum 603 would be red again within three weeks of ordinary work. Measured through the declaration's own discover (repo_tests._import_hermeticity_scope.entries), not a re-derived glob. NEITHER FLOOR IS PINNED TO ITS MINIMUM. hooks-path-override has been re-pinned sixteen times and its own comment block records why every time: pinned at the bottom of the window, the next file main adds is another red, and the branch that pays is never the branch that caused it. #17142 is the real fix -- a 400-file growth allowance on a ~6900-file tree is 5.8%. Verified on this tree, rebased onto origin/main 3f0d4b3: one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency 50 passed prompt_injection_detector_strict_mode_test + reach_declarations_test 192 passed The second invocation is not incidental: reach_declarations_test only sees the prompt-injection declaration when something else in the session has imported its module, so running repo_tests/ alone drops the sweep from 36 parameters to 35 and reports a clean pass over a floor it never checked. Refs #14881, #14631, #17142, #17351
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…re-empt a zero-headroom floor (#16688) Both changes are pre-emptive: this branch was GREEN when they were made. It was green against a base that has since moved, which is the whole point. THE CENSUS GUARD WAS A SECOND FORK OF THE ENUMERATION. redaction_concept_census_test shelled out to `git ls-files` for itself, exactly as enum_hand_copy_guard did on the #14881 branch. one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks caught that one in CI; rebasing this branch onto origin/main 3f0d4b3 reproduces it here -- 34 invocations against a MAX of 33 -- because main reached 33 after this branch's last run. Found by predicting it and checking, not by a red. A census guard forking the repo's enumeration, in a PR whose subject is forks, is the joke writing itself. Routed through tools.lint._scan_helpers.tracked_paths, whose `exclude` becomes a git `:(exclude)` pathspec so git does the matching -- rather than the second Python-side matcher this had, which is how #15510 fired an exclusion against absolute paths and matched every file. subprocess and scrubbed_git_env are no longer imported. HOOKS-PATH-OVERRIDE 6536 -> 6800 WHILE PASSING. Measured on this rebased tree: population 6937 (glob hits for *.sh/*.py/*.yml/*.yaml plus 31 extensionless files whose shebang says shell), completed 6936, slack 401 against growth 400 + skips 1 = exactly 401. That is not "passing", it is "passing until the next file anyone lands", and four sessions moved main in the hour this branch sat in review. The cost of pre-empting is one line on a hub file three other PRs already change to the same value; the cost of not is an hour of CI and a round trip on a predictable red. 6800 is mid-window of [6536, 6936], not the minimum -- fifteen previous re-pins took the bottom, which is why there were fifteen. It is deliberately the same value as autobot-ai-58's #17338 and this session's #17342 and #17346: one measurement should not produce four numbers, and identical lines cannot conflict. First to land wins, the rest adopt on rebase. The other three floors were measured here and left alone, because this branch does not move their populations: audio-extension-allowlist 6151/5764 against the 5700 pinned earlier on this branch (margin 64), prompt-injection-detector-strict-mode 3256 (no new backend non-test module), import-hermeticity 662 (no new module under api/ or autobot_shared/). Two of those sit at exactly zero headroom on main and will need re-pinning by whichever branch next adds to their roots -- see #17142. Verified on this tree: one_git_enumeration + redaction_concept_census + threat_model_anchors + prompt_injection_detector_strict_mode + reach_declarations ... 274 passed reach_declarations_test is run with the prompt-injection module in the same session on purpose: its declaration reaches the sweep only when something else has imported it, so repo_tests/ alone silently checks 35 of 36 floors. Refs #16688, #15926, #17142
4 tasks
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…3049, #17142) Rebased onto origin/main 3f0d4b3. Two declarations were red, both because main sits at EXACTLY its allowance and this branch adds counting files. hooks-path-override 6536 -> 6800 population 6938 (6907 glob hits for *.sh/*.py/*.yml/*.yaml, plus 31 extensionless files whose shebang says shell), completed 6937 (`read` counts opened files, minus the one EXEMPT). At 6536: slack 402 against growth 400 + skips 1 = 401. One over. Window [6537, 6937]. prompt-injection-detector-strict-mode 2956 -> 3100 population 3257 non-test .py under autobot-backend/ (origin/main: 3256, slack exactly 300 = the whole growth allowance, skips defaults to 0). This branch adds accelerate_loader.py, so 301. Window [2957, 3257]; no completed() call on this declaration, so verify_floor is the only bound. audio-extension-allowlist needed nothing: already pinned to 5700 on this branch, population 6152 against window [5452, 5766]. NEITHER IS PINNED TO THE MINIMUM, and that is the point. hooks-path-override has been re-pinned SIXTEEN times, and its own comment block records why each time: every re-pin took the bottom of the window, so the next file main added was another red, and the branch paying for it was never the branch that caused it. 6800 and 3100 are mid-window -- slack 138 and 157 against allowances of 401 and 300 -- which buys ~260 and ~143 files of headroom instead of zero. The reasoning is autobot-ai-58's, from #17338. 6800 is deliberately 58's value rather than a fourth number for one measurement: if #17338 lands first this line is identical and there is nothing to resolve. autobot-ai-f8 proposed 6736 for the post-#17318 tree, also inside the window; 6800 leaves more headroom. Rule agreed between the three sessions: first to land wins, the others adopt rather than re-derive. 3100 is new -- no peer has a re-pin of that declaration in flight. Verified with the declaring module explicitly in the session: pytest autobot-backend/security/prompt_injection_detector_strict_mode_test.py \ repo_tests/reach_declarations_test.py -> 192 passed That invocation is not incidental. The declaration's own docstring warns it reaches the sweep only when something else has already imported its module: running repo_tests/ alone drops the parameter count from 36 to 35 and reports a clean pass over a floor it never checked. 5.8%, and sixteen re-pins is the evidence that the allowance is what is wrong. Refs #13049, #17142, #17338
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…3049, #17142) Rebased onto origin/main 3f0d4b3. Two declarations were red, both because main sits at EXACTLY its allowance and this branch adds counting files. hooks-path-override 6536 -> 6800 population 6938 (6907 glob hits for *.sh/*.py/*.yml/*.yaml, plus 31 extensionless files whose shebang says shell), completed 6937 (`read` counts opened files, minus the one EXEMPT). At 6536: slack 402 against growth 400 + skips 1 = 401. One over. Window [6537, 6937]. prompt-injection-detector-strict-mode 2956 -> 3100 population 3257 non-test .py under autobot-backend/ (origin/main: 3256, slack exactly 300 = the whole growth allowance, skips defaults to 0). This branch adds accelerate_loader.py, so 301. Window [2957, 3257]; no completed() call on this declaration, so verify_floor is the only bound. audio-extension-allowlist needed nothing: already pinned to 5700 on this branch, population 6152 against window [5452, 5766]. NEITHER IS PINNED TO THE MINIMUM, and that is the point. hooks-path-override has been re-pinned SIXTEEN times, and its own comment block records why each time: every re-pin took the bottom of the window, so the next file main added was another red, and the branch paying for it was never the branch that caused it. 6800 and 3100 are mid-window -- slack 138 and 157 against allowances of 401 and 300 -- which buys ~260 and ~143 files of headroom instead of zero. The reasoning is autobot-ai-58's, from #17338. 6800 is deliberately 58's value rather than a fourth number for one measurement: if #17338 lands first this line is identical and there is nothing to resolve. autobot-ai-f8 proposed 6736 for the post-#17318 tree, also inside the window; 6800 leaves more headroom. Rule agreed between the three sessions: first to land wins, the others adopt rather than re-derive. 3100 is new -- no peer has a re-pin of that declaration in flight. Verified with the declaring module explicitly in the session: pytest autobot-backend/security/prompt_injection_detector_strict_mode_test.py \ repo_tests/reach_declarations_test.py -> 192 passed That invocation is not incidental. The declaration's own docstring warns it reaches the sweep only when something else has already imported its module: running repo_tests/ alone drops the parameter count from 36 to 35 and reports a clean pass over a floor it never checked. 5.8%, and sixteen re-pins is the evidence that the allowance is what is wrong. Refs #13049, #17142, #17338
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…, re-pin two floors (#14881, #14631) Three failures, diagnosed from the run log rather than inferred. The log only became readable once the WHOLE run finished -- a job's log is not served while any shard of its run is still going, which is #17351. shard 10 one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks 34 direct `git ls-files` invocations in repo_tests/, MAX says 33 shard 12 reach_declarations_test[hooks-path-override] floor 6536 sits 403 below population 6939, allowance 401 shard 12 reach_declarations_test[import-hermeticity] floor 602 sits 61 below population 663, allowance 60 THE FIRST ONE IS MINE, and it is the one worth recording. enum_hand_copy_guard shelled out to `git ls-files` for itself. MAX_DIRECT_INVOCATIONS only ever shrinks, so that is a regression even though the guard worked -- a new fork of the repo's enumeration helper, added in a PR whose subject is hand-copied definitions. Routed through tools.lint._scan_helpers.tracked_paths, which also gets the exclusion right: `exclude` becomes a git `:(exclude)` pathspec so git does the matching, rather than a second matcher in Python that can disagree with the first (#15510). subprocess and scrubbed_git_env are no longer imported. THE OTHER TWO ARE MAIN'S, and this branch pays for them: hooks-path-override 6536 -> 6800. Population 6939, completed 6938, window [6537, 6938]. Main had spent 401 of its 401 allowance before this branch existed. Same 6800 as autobot-ai-58's #17338 and this session's #13049 branch -- one measurement should not produce three numbers, and if any two land the line is identical. import-hermeticity 602 -> 633. A FOURTH floor at zero headroom on main that nobody had named: 662 non-test modules against floor 602 is slack 60, exactly the allowance (skips=0). This branch adds autobot_shared/code_review_enums.py -- the shared home the four hand-copied enums moved to -- so 663, one over. 633 is mid-window of [603, 663]; the declaration's own comment records this population growing ~52 modules per 30 days, so the minimum 603 would be red again within three weeks of ordinary work. Measured through the declaration's own discover (repo_tests._import_hermeticity_scope.entries), not a re-derived glob. NEITHER FLOOR IS PINNED TO ITS MINIMUM. hooks-path-override has been re-pinned sixteen times and its own comment block records why every time: pinned at the bottom of the window, the next file main adds is another red, and the branch that pays is never the branch that caused it. #17142 is the real fix -- a 400-file growth allowance on a ~6900-file tree is 5.8%. Verified on this tree, rebased onto origin/main 3f0d4b3: one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency 50 passed prompt_injection_detector_strict_mode_test + reach_declarations_test 192 passed The second invocation is not incidental: reach_declarations_test only sees the prompt-injection declaration when something else in the session has imported its module, so running repo_tests/ alone drops the sweep from 36 parameters to 35 and reports a clean pass over a floor it never checked. Refs #14881, #14631, #17142, #17351
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…C4 only) (#17346) * refactor(optimization): one lazy-import primitive, and the accelerate fork closed (#13049) C4 ONLY, AND THE ISSUE SAYS SO. #13049's Ordering section reads: "Land them after the #13030 P0 defects, so the enum work is done against a module whose behaviour is verifiable. C4 is independent and can go any time." All eight #13030 children are still open, including the P0 defects #13031/#13032/#13033, so C1 (architecture-family taxonomies), C2 (dtype vocabularies) and C3 (compression validation) are not touched here. Those blockers are now recorded as native blocked_by edges on #13049 rather than left as prose in an Ordering paragraph nobody queries. C4: _import_accelerate() was defined twice in llm_shared/optimization/ -- meta_eviction.py and model_inspector.py -- same package, same optional dependency. The issue calls them "character-for-character in intent"; they had in fact already drifted on the part that matters. meta_eviction caught (ImportError, RuntimeError) and re-raised BOTH as ImportError; model_inspector caught ImportError alone and let RuntimeError through. That divergence decided the design rather than being papered over. torch_loader had already ruled on exactly this question (#12714): a custom error_message applies only when the dependency is genuinely ABSENT, while a RuntimeError -- installed but failed to initialise -- is re-raised unchanged, because rewording it as "Install with: pip install X" sends a reader to fix something that is already there. The rule is inherited, not invented. It changes one behaviour, narrowly: meta_eviction's RuntimeError-to-ImportError conversion is gone. Its public entry point evict_layer_to_meta documents "ImportError: If quantizer is provided but accelerate is not installed" -- the not-installed case, which still raises ImportError. The only call site, _evict_quantized_layer, is unguarded, and nothing upstream catches it, so no handler changes meaning. NOT a third copy of the machinery. Rather than clone torch_loader's caching and double-checked locking into an accelerate_loader beside it, the primitive moved to llm_shared/lazy_import.py and both loaders are thin wrappers over it. lazy_torch keeps its exact signature and behaviour; its 18 call sites are untouched, and nothing outside the module ever referenced the _torch/_torch_error globals it used to own (checked before moving them). Call-site impact, which the symbol rename made load-bearing: 9 existing tests patched "...meta_eviction._import_accelerate" / "...model_inspector. _import_accelerate" by name. Retargeted to lazy_accelerate in the same module namespaces, which is where the imported symbol lives. Grepping the behaviour rather than the symbol is rule 7 for exactly this reason. lazy_import_test.py pins the asymmetry that had no test while the rule lived in torch_loader -- and which the two forks had already drifted apart on: an ImportError takes the call site's wording, a RuntimeError is re-raised unchanged and ignores error_message. Both driven through the real import path rather than by seeding the cache, so they also prove RuntimeError is among the exceptions caught. Also pinned: the failure is cached rather than retried, and eight threads racing first use all receive one module object -- the race #12714 found in 8 of its 9 copies. Refs #13049, #12714, #13030 * fix(optimization): put the accelerate loader where the harness can see it (#13049) The first attempt added llm_shared/lazy_import.py and llm_shared/accelerate_loader.py at the package root and made torch_loader delegate to the shared primitive. The pre-push suite refused it, and the reason is worth recording rather than routing around. A module at the llm_shared ROOT is invisible to the test harness until it is named in autobot-backend/conftest.py. That package's __init__.py eagerly imports .adapters (reaching autobot_shared and the provider stack), so conftest installs a stub whose __path__ is [] and real-loads only the submodules tests need -- torch_loader among them, added by #12714 when it created that module. Two more entries were the obvious fix. conftest.py cannot take them. It sits at EXACTLY its recorded 1505-line ceiling on origin/main (measured: git show origin/main:autobot-backend/conftest.py | wc -l = 1505, baseline 1505 in both python_file_size_known_large.py and python_file_size_ratchet_baseline.py). A grandfathered file may not grow, and raising a ceiling is not allowed -- so this is a second zero-headroom ceiling on main, alongside the audio-extension-allowlist reach floor, and it blocks ANY new llm_shared root module for everyone, not just this change. WHAT CHANGED INSTEAD. llm_shared.optimization is given a REAL __path__ by that same conftest, and both callers of the loader live there. So the loader is llm_shared/optimization/accelerate_loader.py, imported as `from .accelerate_loader import lazy_accelerate`, and needs no registration at all. conftest.py and torch_loader.py are now byte-identical to origin/main -- verified with `git diff origin/main` on both, empty. That is not free, and the cost is stated rather than hidden: the locking-and-caching machinery now exists twice, here and in torch_loader. Extracting it into one primitive requires a conftest entry, so it is blocked on that ceiling and filed separately. The alternative -- hiding a generic LazyModule class inside a module named torch_loader purely to dodge a size limit -- would be a workaround shaped by the constraint rather than by the design, so it was not taken. The consolidation #13049 C4 asks for is delivered either way: two _import_accelerate() copies become one lazy_accelerate(), and the raise policy they had drifted apart on is inherited from torch_loader's recorded ruling rather than re-decided -- error_message applies to a genuine ImportError, a RuntimeError is re-raised unchanged. Verified on this tree: accelerate_loader_test + meta_eviction_test + model_inspector_test . 56 passed llm_shared/ (excluding pricing/) ................. 683 passed, 128 skipped llm_shared/pricing/sync_cache_scheduler_test.py fails to collect here and is NOT caused by this change: "llc.scheduler.base requires asyncio.Task.cancelling() (Python 3.11+)", identical on the untouched origin/main checkout. This machine runs 3.10; CI runs 3.14. Refs #13049, #12714 * chore(ratchets): re-pin the two floors main left at zero headroom (#13049, #17142) Rebased onto origin/main 3f0d4b3. Two declarations were red, both because main sits at EXACTLY its allowance and this branch adds counting files. hooks-path-override 6536 -> 6800 population 6938 (6907 glob hits for *.sh/*.py/*.yml/*.yaml, plus 31 extensionless files whose shebang says shell), completed 6937 (`read` counts opened files, minus the one EXEMPT). At 6536: slack 402 against growth 400 + skips 1 = 401. One over. Window [6537, 6937]. prompt-injection-detector-strict-mode 2956 -> 3100 population 3257 non-test .py under autobot-backend/ (origin/main: 3256, slack exactly 300 = the whole growth allowance, skips defaults to 0). This branch adds accelerate_loader.py, so 301. Window [2957, 3257]; no completed() call on this declaration, so verify_floor is the only bound. audio-extension-allowlist needed nothing: already pinned to 5700 on this branch, population 6152 against window [5452, 5766]. NEITHER IS PINNED TO THE MINIMUM, and that is the point. hooks-path-override has been re-pinned SIXTEEN times, and its own comment block records why each time: every re-pin took the bottom of the window, so the next file main added was another red, and the branch paying for it was never the branch that caused it. 6800 and 3100 are mid-window -- slack 138 and 157 against allowances of 401 and 300 -- which buys ~260 and ~143 files of headroom instead of zero. The reasoning is autobot-ai-58's, from #17338. 6800 is deliberately 58's value rather than a fourth number for one measurement: if #17338 lands first this line is identical and there is nothing to resolve. autobot-ai-f8 proposed 6736 for the post-#17318 tree, also inside the window; 6800 leaves more headroom. Rule agreed between the three sessions: first to land wins, the others adopt rather than re-derive. 3100 is new -- no peer has a re-pin of that declaration in flight. Verified with the declaring module explicitly in the session: pytest autobot-backend/security/prompt_injection_detector_strict_mode_test.py \ repo_tests/reach_declarations_test.py -> 192 passed That invocation is not incidental. The declaration's own docstring warns it reaches the sweep only when something else has already imported its module: running repo_tests/ alone drops the parameter count from 36 to 35 and reports a clean pass over a floor it never checked. 5.8%, and sixteen re-pins is the evidence that the allowance is what is wrong. Refs #13049, #17142, #17338 * chore(ratchets): adopt main's floors, re-pin the one it left one file short (#13049, #17142) Rebased onto origin/main e8c7bf9. Main moved three floors in the 40 minutes since the previous rebase; all three are ADOPTED, and one of them is red here. audio-extension-allowlist 5752 -> 5757 on main adopted, green import-hermeticity 602 -> 604 on main adopted, green hooks-path-override 6736 unchanged adopted, green prompt-injection-... 2956 -> 2958 on main adopted, RED -> 3100 Measured on this tree: audio pop 6164 completed 5778 pin 5757 slack 407/700 margin 21 hooks pop 6950 completed 6949 pin 6736 slack 214/401 import-herm pop 664 pin 604 slack 60/60 ZERO headroom prompt-inj pop 3259 pin 2958 slack 301/300 ONE OVER prompt-injection-detector-strict-mode is re-pinned 2958 -> 3100, mid-window of [2959, 3259]. Main pinned 2958 from its own population of 3258 -- slack exactly 300, the whole allowance, zero headroom. This branch adds ONE non-test module under autobot-backend/ and that is enough. THE PATTERN IS NOW THE FINDING, not any individual pin. This is the third time tonight a floor pinned at `population - growth` has gone red on the next branch to touch its roots: hooks-path-override, then this declaration at 2956, then this declaration again at 2958. Each time main pins from its own population, each time the next branch adds a file, and each time the branch that pays for the re-pin is not the branch that caused it -- which is what sixteen comment blocks in hooks_path_override_15961_test.py already say, one per re-pin. 3100 buys ~159 files of headroom so the fourth branch does not repeat it. The alternative -- adopting 2958 to honour first-to-land -- would be adopting a value that does not pass here, which the rule was never meant to require. import-hermeticity is adopted at main's 604 and sits at slack 60 against an allowance of 60: passing, zero headroom, and red for whoever next adds a module under api/ or autobot_shared/. Not re-pinned here because this branch does not add one; #17342 is the branch that does and carries 633. Refs #13049, #17142 * chore(ratchets): keep 3100 over main's 2960, and stop putting measurements in prose (#13049, #17142) Rebased onto origin/main e575e3c. KEEPING 3100 RATHER THAN ADOPTING, which inverts this branch's usual rule and is agreed rather than unilateral. #17327 merged carrying prompt-injection-detector-strict-mode at 2960 -- `population - growth`, so main sits at slack exactly 300 of a 300 allowance and the next branch adding one non-test backend module turns it red. This is that branch. autobot-ai-63 held back an identical 3100 so it would resolve here rather than race, and autobot-ai-f8 asked for it to be kept. First-to-land-wins presumes the landed value is VALID. 2960 is not, for any tree with one more file than main's, which is what "zero headroom by construction" means. Measured on this tree (in this message, not in the file -- see below): prompt-inj population 3261 pin 3100 slack 161/300 hooks population 6960 pin 6736 slack 224/401 audio population 6173 completed 5786 pin 5764 margin 22 import-herm population 664 pin 604 slack 60/60 zero headroom, untouched here hooks-path-override adopts main's 6736 unchanged: unlike the prompt-injection pin it is not at the bottom of its window, so this branch's files do not exhaust it. NO MEASURED NUMBERS IN EITHER COMMENT NOW, and that is the durable fix rather than a third cleanup. Both notes previously cited a population and a slack figure; both were stale one rebase later -- the prompt-injection block still said "main re-pinned this to 2958 ... this branch 3259" after main reached 2960 and the tree reached 3261, and the hooks note claimed slack ~204 against an actual 224. That is the same mechanism that produced four orphaned narratives in these files tonight: a rebase resolves the VALUE and nothing resolves the SENTENCE, because prose lands as a pure addition in a different commit. Deleting each one as it is found is a treadmill. Prose that cites no measurement cannot go stale, so both notes now state the DECISION and the METHOD and send the reader to re-derive. The arithmetic lives here, in a commit message, which cannot drift from the tree it describes. The prompt-injection declaration is also rebuilt on main's current comment rather than carrying this branch's copy of an older one, so main's own account of 2955 -> 2956 -> 2958 -> 2960 is intact and unedited above the note. Refs #13049, #17142, #17356
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…, re-pin two floors (#14881, #14631) Three failures, diagnosed from the run log rather than inferred. The log only became readable once the WHOLE run finished -- a job's log is not served while any shard of its run is still going, which is #17351. shard 10 one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks 34 direct `git ls-files` invocations in repo_tests/, MAX says 33 shard 12 reach_declarations_test[hooks-path-override] floor 6536 sits 403 below population 6939, allowance 401 shard 12 reach_declarations_test[import-hermeticity] floor 602 sits 61 below population 663, allowance 60 THE FIRST ONE IS MINE, and it is the one worth recording. enum_hand_copy_guard shelled out to `git ls-files` for itself. MAX_DIRECT_INVOCATIONS only ever shrinks, so that is a regression even though the guard worked -- a new fork of the repo's enumeration helper, added in a PR whose subject is hand-copied definitions. Routed through tools.lint._scan_helpers.tracked_paths, which also gets the exclusion right: `exclude` becomes a git `:(exclude)` pathspec so git does the matching, rather than a second matcher in Python that can disagree with the first (#15510). subprocess and scrubbed_git_env are no longer imported. THE OTHER TWO ARE MAIN'S, and this branch pays for them: hooks-path-override 6536 -> 6800. Population 6939, completed 6938, window [6537, 6938]. Main had spent 401 of its 401 allowance before this branch existed. Same 6800 as autobot-ai-58's #17338 and this session's #13049 branch -- one measurement should not produce three numbers, and if any two land the line is identical. import-hermeticity 602 -> 633. A FOURTH floor at zero headroom on main that nobody had named: 662 non-test modules against floor 602 is slack 60, exactly the allowance (skips=0). This branch adds autobot_shared/code_review_enums.py -- the shared home the four hand-copied enums moved to -- so 663, one over. 633 is mid-window of [603, 663]; the declaration's own comment records this population growing ~52 modules per 30 days, so the minimum 603 would be red again within three weeks of ordinary work. Measured through the declaration's own discover (repo_tests._import_hermeticity_scope.entries), not a re-derived glob. NEITHER FLOOR IS PINNED TO ITS MINIMUM. hooks-path-override has been re-pinned sixteen times and its own comment block records why every time: pinned at the bottom of the window, the next file main adds is another red, and the branch that pays is never the branch that caused it. #17142 is the real fix -- a 400-file growth allowance on a ~6900-file tree is 5.8%. Verified on this tree, rebased onto origin/main 3f0d4b3: one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency 50 passed prompt_injection_detector_strict_mode_test + reach_declarations_test 192 passed The second invocation is not incidental: reach_declarations_test only sees the prompt-injection declaration when something else in the session has imported its module, so running repo_tests/ alone drops the sweep from 36 parameters to 35 and reports a clean pass over a floor it never checked. Refs #14881, #14631, #17142, #17351
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…re-empt a zero-headroom floor (#16688) Both changes are pre-emptive: this branch was GREEN when they were made. It was green against a base that has since moved, which is the whole point. THE CENSUS GUARD WAS A SECOND FORK OF THE ENUMERATION. redaction_concept_census_test shelled out to `git ls-files` for itself, exactly as enum_hand_copy_guard did on the #14881 branch. one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks caught that one in CI; rebasing this branch onto origin/main 3f0d4b3 reproduces it here -- 34 invocations against a MAX of 33 -- because main reached 33 after this branch's last run. Found by predicting it and checking, not by a red. A census guard forking the repo's enumeration, in a PR whose subject is forks, is the joke writing itself. Routed through tools.lint._scan_helpers.tracked_paths, whose `exclude` becomes a git `:(exclude)` pathspec so git does the matching -- rather than the second Python-side matcher this had, which is how #15510 fired an exclusion against absolute paths and matched every file. subprocess and scrubbed_git_env are no longer imported. HOOKS-PATH-OVERRIDE 6536 -> 6800 WHILE PASSING. Measured on this rebased tree: population 6937 (glob hits for *.sh/*.py/*.yml/*.yaml plus 31 extensionless files whose shebang says shell), completed 6936, slack 401 against growth 400 + skips 1 = exactly 401. That is not "passing", it is "passing until the next file anyone lands", and four sessions moved main in the hour this branch sat in review. The cost of pre-empting is one line on a hub file three other PRs already change to the same value; the cost of not is an hour of CI and a round trip on a predictable red. 6800 is mid-window of [6536, 6936], not the minimum -- fifteen previous re-pins took the bottom, which is why there were fifteen. It is deliberately the same value as autobot-ai-58's #17338 and this session's #17342 and #17346: one measurement should not produce four numbers, and identical lines cannot conflict. First to land wins, the rest adopt on rebase. The other three floors were measured here and left alone, because this branch does not move their populations: audio-extension-allowlist 6151/5764 against the 5700 pinned earlier on this branch (margin 64), prompt-injection-detector-strict-mode 3256 (no new backend non-test module), import-hermeticity 662 (no new module under api/ or autobot_shared/). Two of those sit at exactly zero headroom on main and will need re-pinning by whichever branch next adds to their roots -- see #17142. Verified on this tree: one_git_enumeration + redaction_concept_census + threat_model_anchors + prompt_injection_detector_strict_mode + reach_declarations ... 274 passed reach_declarations_test is run with the prompt-injection module in the same session on purpose: its declaration reaches the sweep only when something else has imported it, so repo_tests/ alone silently checks 35 of 36 floors. Refs #16688, #15926, #17142
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…sources (#14881, #14631) (#17342) * refactor(enums): one definition per enum, and a guard that reads the tree (#14881) GROUP 1 -- the API schema hand-copied four enums it already imported from. api/schemas_analytics.py redeclared CheckSeverity, CheckCategory, ReviewSeverity and ReviewCategory with members identical to the code_intelligence originals, while api/analytics_precommit.py imported BUILTIN_CHECKS from that same module. The only real difference was the `str` mixin the schema copies carried for Pydantic. The domain classes carry it now, so one class serves the engine and the wire format, and the four copies are gone. The `CheckSeverity(x.value)` conversions at the seam still hold -- they became idempotent rather than breaking. No cycle: code_intelligence does not import api.schemas_analytics, and both modules pull only stdlib plus autobot_shared. schemas_analytics 3575 -> 3544 lines, re-pinned in both size baselines since a shrink has to be locked in (#14498). GROUP 2 -- one copy was laziness, five were not, and the issue's prediction about them was inverted. api/marketplace_422_test.py was the lazy one, and its enforcement was worse than absent. TestEnumParity was titled "Verify our local enums exactly match the real marketplace enums" and asserted this file's copy against a THIRD hardcoded literal set a few lines below it -- it never imported api/marketplace.py, so no change to the real enum could ever fail it. The real enums are imported now (tests/api/test_marketplace.py already did, so the import is proven in this environment, not assumed), `_make_app()` validates against the same class the real endpoint does, and the parity class is deleted rather than repaired: with the import, parity is structural. The other five are harness scaffolding -- enums injected into sys.modules stubs so a route module imports without its dependency graph, and FastAPI rejects anything but a real str-enum as a Query type, so a Mock cannot serve. Telling those tests to "just import it" would defeat the isolation they exist for, so they keep their copies and declare them. What the census found instead of the predicted drift: the copies carried members production never had. BatchJobType stubs declared ai_task and report. Production has neither, and has file_conversion, report_generation, backup and custom, which the stubs omitted. Two files. _ErrorCategory stubs declared CLIENT_ERROR. Production's ErrorCategory has 18 members and that is not one. Two files. No test referenced any invented member, which is why nothing failed and why only a census found them. All four stubs now mirror production. repo_tests/enum_hand_copy_guard_test.py enforces it, derived from the tree rather than a list, and asymmetric on purpose: a SUBSET is fine (a stub needs only what its test uses -- _SSOProviderType declares 5 of 9), an EXTRA member fails (a test exercising it asserts against a value the system cannot produce), and an exact copy fails unless declared scaffolding. _DISTINCT_CONCEPTS is the declared escape for a genuine name collision, empty today because the sweep found none. Measured: 6,150 files, 436 production enum names, 14 test-local, 0 invented, 0 undeclared exact copies, 0 stale exemptions, 0 unreadable. The guard caught one of my own changes while I wrote it: aligning BatchJobType to production made it an exact copy, which is only legitimate because it is the same stub BatchJobStatus is. Declared, not exempted by widening the rule. Closes #14881 * refactor(mcp): derive the bridge lists from disk instead of hand-maintaining them (#14631) Registering manual_mcp as the twelfth bridge (#14586) meant editing three separate hand-maintained lists of "every governed bridge", and the fourth caught the omission only by accident: test_discover_bridges_known_names happened to assert == against a literal set rather than a subset, so a shard failure rather than a design decision is what surfaced it. The filesystem scan is the SSOT because it is the only one of the four that cannot silently disagree with reality -- it reads the files on disk rather than a name someone remembered to type in a second place. LIST 4 IS GONE. test_discover_bridges_known_names no longer carries twelve literal names; it compares discover_bridges() against {bridge_name(p) for p in bridge_files()}, with a non-empty assertion first so an empty scan cannot make the comparison vacuous. A thirteenth bridge now needs no edit there, and a bridge that stops being discovered still fails. LIST 3 IS GUARDED. floor_membership_gaps() in the required lint check reports a governed bridge with no PER_BRIDGE_DISCOVERY_FLOOR entry, and a floor entry naming no bridge. This closes a real hole rather than a theoretical one: bridge_discovery_gaps() iterates PER_BRIDGE_DISCOVERY_FLOOR.items(), so a bridge absent from that dict was never checked by it, and the aggregate DISCOVERY_FLOOR could not catch the omission either because a new bridge only ever RAISES the sum. It runs before bridge_discovery_gaps(), which can only speak about bridges the dict already names. LIST 2 IS GUARDED, and measured as fully removable. repo_tests/mcp_bridge_list_consistency_14631_test.py pins the membership of both _BRIDGE_MODULE_REGISTRY and PER_BRIDGE_DISCOVERY_FLOOR against the scan, in both directions. It imports bridge_files() rather than re-globbing -- re-deriving the scan inside the test would make it a fifth copy of the thing it checks -- and reads the two lists by AST, since neither api/mcp_registry.py's dependency graph nor the lint script's sys.path manipulation is needed to read a literal. The investigate item is answered, and more strongly than #14631 expected. It assumed _BRIDGE_MODULE_REGISTRY carries endpoint URLs and feature tags the glob cannot supply, so only a partial derivation was possible. Measured: all 12 bridges declare MANIFEST = MCPBridgeManifest(...) carrying name, endpoint AND features, and both endpoint and features agree with the registry's tuples 12/12. It holds no information that is not already on the bridge it describes. Filed as #17340 rather than done here: it feeds discover_bridges()'s fallback, so it changes a discovery path rather than a literal, and wants its own review pass. Verified on this tree: scan finds 12 bridges; registry 12, floor 12, all three sets identical. The exactness rule is pinned on five synthetic cases including the #14586 shape -- a list that is a strict SUBSET of disk, which a subset assertion would pass and this one fails. Closes #14631 * fix(enums): move the shared vocabularies to autobot_shared, not into the API (#14881) CI caught a real defect in the first version of this PR, and the fix is the design argument rather than a patch over it. WHAT BROKE. startup-import-smoke went red: PydanticSchemaGenerationError: Unable to generate pydantic-core schema for <MagicMock id=...> Pointing api/schemas_analytics.py at code_intelligence.precommit_analyzer and code_intelligence.code_review_engine made every Pydantic model annotated with CheckSeverity/CheckCategory/ReviewSeverity/ReviewCategory depend on that package being real at import time. autobot-backend/conftest.py stubs the whole code_intelligence package -- its __init__ carries imports the smoke environment cannot satisfy -- so the enums resolved to MagicMock and the schemas could not be built. I checked for an import CYCLE before the first version and found none; I did not check whether the package is importable in every harness, which is the question that actually decided it. WHAT IT MEANS. A wire-format vocabulary the API cannot import without dragging in an analysis engine is not owned by that engine. These four are shared between the engines and the API, so they belong to neither. They now live in autobot_shared/code_review_enums.py -- the same move #11290 made collapsing SecretScope/VisibilityLevel/ScopeLevel onto one shared ScopeLevel. Both engines re-export their pair, so every existing `from code_intelligence.precommit_analyzer import CheckSeverity` keeps working: what moved is the definition, not the name. The `str` mixin stays, for FastAPI. The duplication #14881 exists to close is still closed -- one definition per enum, four copies gone -- and the definition now sits where both consumers can actually reach it. Sizes, all shrinking and all locked in per #14498 (both baselines): api/schemas_analytics.py 3544 -> 3543 code_intelligence/precommit_analyzer.py 731 -> 720 code_intelligence/code_review_engine.py 1235 -> 1219 `from enum import Enum` removed from both engines; it became unused. Verified on this tree: precommit_analyzer_test + analytics_code_review_test + marketplace_422_test 73 passed tests/test_startup_imports.py -- the exact job that failed: this branch 7 failed, 414 passed origin/main 7 failed, 414 passed (identical) PydanticSchemaGenerationError / MagicMock occurrences: 0 The 7 are the pre-existing Python-3.10 platform guard ("llc.scheduler.base requires asyncio.Task.cancelling()"); this machine runs 3.10, CI runs 3.14, so the delta against baseline is what carries the information here, not the count. Refs #14881, #11290 * fix(guards,ratchets): route the new guard through the one enumeration, re-pin two floors (#14881, #14631) Three failures, diagnosed from the run log rather than inferred. The log only became readable once the WHOLE run finished -- a job's log is not served while any shard of its run is still going, which is #17351. shard 10 one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks 34 direct `git ls-files` invocations in repo_tests/, MAX says 33 shard 12 reach_declarations_test[hooks-path-override] floor 6536 sits 403 below population 6939, allowance 401 shard 12 reach_declarations_test[import-hermeticity] floor 602 sits 61 below population 663, allowance 60 THE FIRST ONE IS MINE, and it is the one worth recording. enum_hand_copy_guard shelled out to `git ls-files` for itself. MAX_DIRECT_INVOCATIONS only ever shrinks, so that is a regression even though the guard worked -- a new fork of the repo's enumeration helper, added in a PR whose subject is hand-copied definitions. Routed through tools.lint._scan_helpers.tracked_paths, which also gets the exclusion right: `exclude` becomes a git `:(exclude)` pathspec so git does the matching, rather than a second matcher in Python that can disagree with the first (#15510). subprocess and scrubbed_git_env are no longer imported. THE OTHER TWO ARE MAIN'S, and this branch pays for them: hooks-path-override 6536 -> 6800. Population 6939, completed 6938, window [6537, 6938]. Main had spent 401 of its 401 allowance before this branch existed. Same 6800 as autobot-ai-58's #17338 and this session's #13049 branch -- one measurement should not produce three numbers, and if any two land the line is identical. import-hermeticity 602 -> 633. A FOURTH floor at zero headroom on main that nobody had named: 662 non-test modules against floor 602 is slack 60, exactly the allowance (skips=0). This branch adds autobot_shared/code_review_enums.py -- the shared home the four hand-copied enums moved to -- so 663, one over. 633 is mid-window of [603, 663]; the declaration's own comment records this population growing ~52 modules per 30 days, so the minimum 603 would be red again within three weeks of ordinary work. Measured through the declaration's own discover (repo_tests._import_hermeticity_scope.entries), not a re-derived glob. NEITHER FLOOR IS PINNED TO ITS MINIMUM. hooks-path-override has been re-pinned sixteen times and its own comment block records why every time: pinned at the bottom of the window, the next file main adds is another red, and the branch that pays is never the branch that caused it. #17142 is the real fix -- a 400-file growth allowance on a ~6900-file tree is 5.8%. Verified on this tree, rebased onto origin/main 3f0d4b3: one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency 50 passed prompt_injection_detector_strict_mode_test + reach_declarations_test 192 passed The second invocation is not incidental: reach_declarations_test only sees the prompt-injection declaration when something else in the session has imported its module, so running repo_tests/ alone drops the sweep from 36 parameters to 35 and reports a clean pass over a floor it never checked. Refs #14881, #14631, #17142, #17351 * chore(ratchets): record that this branch adopted main's floors, not its own (#14881, #14631) Empty commit, and deliberately so: the change is already in the rebase conflict resolutions, but the commits carrying them still SAY 6800 and 5700 in their messages because that is what they proposed when written. Rewriting four commit messages to fix a two-line outcome is worse than stating it once. Rebased onto origin/main 19006b3. Both floors this branch re-pinned had landed on main first, so both were adopted: hooks-path-override proposed 6800 -> adopted main's 6736 (#17318) audio-extension-allowlist proposed 5700 -> adopted main's 5752 (#17317) First to land wins, so one measurement does not produce four numbers. Re-derived on this tree rather than taken on trust: audio pop 6155 completed 5767 pin 5752 slack 403/700 margin 15 hooks pop 6941 completed 6940 pin 6736 slack 205/401 margin 204 prompt-inj pop 3256 pin 2956 slack 300/300 ZERO headroom import-herm pop 663 pin 633 slack 30/60 import-hermeticity stays at this branch's own 633: main is still 602 and this is the branch that adds a module under the counted roots (autobot_shared/code_review_enums.py), so nothing to adopt. TWO NUMBERS WORTH CARRYING TO #17142 rather than acting on here: audio's margin is 15, not the 402 its pin implies. 5752 is population - growth, the GAP bound, while the bound that fires on this guard is completed() -- its loop continues past the canonical file and all of repo_tests/ before reading anything, so completed sits ~388 below population and falls every time anyone adds a guard. Five sessions did tonight. This branch measures 15; the #13049 branch measures 16. Each new repo_tests guard spends one. prompt-injection-detector-strict-mode is at slack 300 against an allowance of 300 -- passing only because this branch adds no non-test module under autobot-backend/. It reds for whoever next does. Verified on this tree: prompt_injection_detector_strict_mode + reach_declarations + one_git_enumeration + enum_hand_copy_guard + mcp_bridge_list_consistency ............................. 242 passed Refs #14881, #14631, #17142, #17317, #17318 * fix(guards): migrate the enum-census floor to declare(), re-measured not carried (#14881, #15928) This is the shard-8 failure on this branch, and the second instance of it this session: FAILED repo_tests/reach_floor_migration_test.py::test_no_new_hand_rolled_floor_remains_unmigrated hand-rolled reach-sized floor(s) not migrated to repo_tests._reach.declare: repo_tests/enum_hand_copy_guard_test.py: ['_MIN_SOURCE_FILES'] #15928's rule: a floor >=100 that binds a tree sweep goes through declare(), because a number chosen by feel decays silently. Both of this session's new guards had one, and both had already decayed: enum_hand_copy_guard_test 3000 against a population of 6155 -- 51% below redaction_concept_census_test 1500 against a population of 3393 -- 56% below (migrated on the #16688 branch) Either would have passed over a sweep that lost HALF the codebase and still reported clean. Both were written by feel, hours apart, inside guards about measurement discipline, in PRs about hand-copied constants, by the same author who had spent the evening flagging this class of error in other sessions' work. One instance is carelessness; two is a property of hand-rolled floors, which is #15928's argument. Evidence posted there rather than left in a postmortem. declare("enum-hand-copy-census", floor=5755, growth=400, what="tracked python files") Re-measured from scratch with a throwaway script -- 6,155 tracked .py files excluding .worktrees/ -- rather than carried across, which is what #15928 requires. growth=400 matches the sibling audio-extension-allowlist declaration over the same population. REACH.completed(parsed) is wired in, which the bare constant had no way to express: the floor now binds what the sweep actually PARSED, not what it listed. Without it the guard could enumerate 6,155 files, fail to parse 6,000 of them, and report the same clean as a tree with no hand-copied enums in it -- the exact distinction the guard exists to make. discover() takes a root and returns [] on EmptyEnumeration rather than letting it escape: reach_declarations_test drives every declaration against an empty repository and demands ReachFloorError specifically, so letting EmptyEnumeration through means the declaration fails loudly while saying nothing about whether its floor binds. Diagnosed by autobot-ai-f8 from the shard-8 log, retrieved via the jobs API after `gh run view --log-failed` refused mid-run. Verified on this tree: reach_floor_migration + enum_hand_copy_guard + reach_declarations ...................................... 206 passed Refs #14881, #14631, #15928 * chore(ratchets): re-pin two floors mid-window, including one I pinned at the minimum an hour ago (#14881, #14631) Rebased onto origin/main e8c7bf9. Main's floors adopted where they hold; two did not. import-hermeticity 604 -> 633. Main pinned 604 for its own population of 664 -- slack exactly 60, the whole allowance (skips=0), zero headroom. This branch adds one module under the counted roots (autobot_shared/code_review_enums.py) so 665, slack 61. Second time tonight this declaration has gone red one file over a minimum pin. 633 is mid-window of [605, 665]. enum-hand-copy-census 5755 -> 5965, AND THIS ONE IS MINE. I created that declaration earlier tonight, migrating a hand-rolled floor under MINIMUM OF THE WINDOW. It went red on the very next rebase: main gained ten files, population reached 6165, slack 410 against an allowance of 400. Ten files. That is all the tolerance a minimum pin has, because it has none by construction. The timing is the point rather than an embarrassment to bury. I had just filed a comment on #17142 arguing that `population - growth` is not a floor but a snapshot with zero tolerance, and that every re-pin taking the bottom of the window is why hooks-path-override has sixteen of them. The declaration I wrote an hour earlier did exactly that. Knowing the rule did not prevent it; reach_declarations_test did, which is the entire case for the rule being mechanical rather than a review item. Re-measured on this tree rather than adjusted: 6,165 tracked .py files excluding .worktrees/. Window [5765, 6165]; 5965 is mid-window and buys ~200 files of headroom instead of ten. Measured after both re-pins: audio pop 6165 completed 5777 pin 5757 slack 408/700 margin 20 hooks pop 6951 pin 6736 slack 215/401 prompt-inj pop 3258 pin 2958 slack 300/300 ZERO headroom import-herm pop 665 pin 633 slack 32/60 enum-census pop 6165 pin 5965 slack 200/400 prompt-injection-detector-strict-mode is adopted at main's 2958 and left alone: it is green here because this branch adds no non-test module under autobot-backend/. #17346 is the branch that does, and carries 3100. Verified on this tree: reach_declarations + reach_floor_migration + enum_hand_copy_guard .................................... 206 passed Refs #14881, #14631, #17142, #15928 * chore(ratchets): re-pin import-hermeticity, and remove every measurement from floor prose (#14881, #14631) Rebased onto origin/main e575e3c. Main's floors adopted where they hold; one did not, and all the prose changed shape. import-hermeticity 604 -> 635. Main's 604 is `population - growth`, so main sits at exactly its allowance and one added module under the counted roots is enough to red it. This branch adds autobot_shared/code_review_enums.py, the shared home the four hand-copied enums moved to. Second time this declaration has gone red that way tonight. 635 is mid-window. audio, hooks, prompt-injection and enum-hand-copy-census all pass on main's values or this branch's existing ones and are left alone. Measured on this tree (here, not in the files -- see below): audio population 6174 completed 5785 pin 5764 margin 21 hooks population 6961 pin 6736 slack 225/401 prompt-inj population 3260 pin 2960 slack 300/300 import-herm population 665 pin 635 slack 30/60 enum-census population 6174 pin 5965 slack 209/400 NO MEASURED NUMBERS REMAIN IN ANY FLOOR COMMENT THIS PR TOUCHES, and that is the change worth reading. Verified: git diff origin/main...HEAD | grep '^+' \ | grep -cE 'slack [0-9]+|population [0-9]+|completed [0-9]+' -> 0 Tonight these files accumulated several stale narratives -- a comment claiming "5700 leaves 64" beside a shipped 5757, a note claiming "slack ~204" against an actual 225, a block describing main's pin as 2958 after main reached 2960. None was wrong when written. Every one went stale at a rebase, because a rebase resolves the VALUE and nothing resolves the SENTENCE: prose lands as a pure addition in a different commit, and no mechanism connects the two. Deleting each as it is found is a treadmill -- this branch did it three times. Prose that cites no measurement cannot go stale. So every floor comment here now states the DECISION and the METHOD and tells the reader to re-derive; the arithmetic lives in commit messages, which cannot drift from the tree they describe because they are not in it. That includes this branch's own self-report on enum-hand-copy-census, which was itself citing a population that had moved twice since it was written -- a comment about the hazard of stale measurements, stale. Verified on this tree: reach_declarations + reach_floor_migration + enum_hand_copy_guard + prompt_injection_detector_strict_mode .................... 213 passed Refs #14881, #14631, #17142, #17356
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…re-empt a zero-headroom floor (#16688) Both changes are pre-emptive: this branch was GREEN when they were made. It was green against a base that has since moved, which is the whole point. THE CENSUS GUARD WAS A SECOND FORK OF THE ENUMERATION. redaction_concept_census_test shelled out to `git ls-files` for itself, exactly as enum_hand_copy_guard did on the #14881 branch. one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks caught that one in CI; rebasing this branch onto origin/main 3f0d4b3 reproduces it here -- 34 invocations against a MAX of 33 -- because main reached 33 after this branch's last run. Found by predicting it and checking, not by a red. A census guard forking the repo's enumeration, in a PR whose subject is forks, is the joke writing itself. Routed through tools.lint._scan_helpers.tracked_paths, whose `exclude` becomes a git `:(exclude)` pathspec so git does the matching -- rather than the second Python-side matcher this had, which is how #15510 fired an exclusion against absolute paths and matched every file. subprocess and scrubbed_git_env are no longer imported. HOOKS-PATH-OVERRIDE 6536 -> 6800 WHILE PASSING. Measured on this rebased tree: population 6937 (glob hits for *.sh/*.py/*.yml/*.yaml plus 31 extensionless files whose shebang says shell), completed 6936, slack 401 against growth 400 + skips 1 = exactly 401. That is not "passing", it is "passing until the next file anyone lands", and four sessions moved main in the hour this branch sat in review. The cost of pre-empting is one line on a hub file three other PRs already change to the same value; the cost of not is an hour of CI and a round trip on a predictable red. 6800 is mid-window of [6536, 6936], not the minimum -- fifteen previous re-pins took the bottom, which is why there were fifteen. It is deliberately the same value as autobot-ai-58's #17338 and this session's #17342 and #17346: one measurement should not produce four numbers, and identical lines cannot conflict. First to land wins, the rest adopt on rebase. The other three floors were measured here and left alone, because this branch does not move their populations: audio-extension-allowlist 6151/5764 against the 5700 pinned earlier on this branch (margin 64), prompt-injection-detector-strict-mode 3256 (no new backend non-test module), import-hermeticity 662 (no new module under api/ or autobot_shared/). Two of those sit at exactly zero headroom on main and will need re-pinning by whichever branch next adds to their roots -- see #17142. Verified on this tree: one_git_enumeration + redaction_concept_census + threat_model_anchors + prompt_injection_detector_strict_mode + reach_declarations ... 274 passed reach_declarations_test is run with the prompt-injection module in the same session on purpose: its declaration reaches the sweep only when something else has imported it, so repo_tests/ alone silently checks 35 of 36 floors. Refs #16688, #15926, #17142
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…census (#16688) (#17335) * refactor(redaction): name the redaction boundary and pin the concept census (#16688) The issue asks which of four parallel redaction implementations is canonical. Measuring first changed the question twice. There are seven, not four. A census rule run over all 3,392 tracked production sources finds three more modules that independently decide what a secret looks like and that no issue had counted: chat_workflow/cot_events.py (_SENSITIVE_KEY_FRAGMENTS, 13 nouns), llc/services/portability.py (_SECRET_LIKE_KEYS, 13) and services/config_revision_service.py (_SECRET_SUBSTRINGS, 5). A fifth, autobot_shared/security/redaction.py, also declares itself the one canonical redactor (#12242). And one of the four is not an implementation at all. security/chat_message_safety.py declares no detector; it composes pii_pipeline and the injection detector. It is the shape the other seven should converge on, so the guard must not count it. There is therefore no single canonical redactor and there should not be one. What was missing is the boundary: one module per SHAPE of the problem. docs/developer/REDACTION_BOUNDARY.md states it, with a which-one-do-I-call table, and each of the modules points at it -- except portability.py, which sits on its grandfathered 936-line ceiling and would be pushed over by the note. Two behaviour changes, both strictly widening: - secret_redaction's JWT pattern required "eyJ" on the payload as well as the header, making it narrower than a2a/pii_pipeline.py's. A payload of {} encodes to e30 and was matched there and missed here -- in the module llm_shared.credential_redaction delegates log redaction to. Anchored on the header alone, as pii_pipeline already was. - credential_redaction.SENSITIVE_KEYS no longer hand-copies the credential nouns; it derives from secret_redaction.CREDENTIAL_SUFFIXES. Deliberately a UNION: that vocabulary carries no authorization terms, so a straight replacement would have stopped redacting Authorization, auth_header and x_auth. A redaction merge that narrows is a silent under-redaction, which is why both directions are now tested. repo_tests/redaction_concept_census_test.py pins the seven. It counts implementations of a concept rather than duplicated text, because jscpd finds zero clone lines across these modules -- they are forks, and a fork shares no 8-line run, so MAX_DUP_LINES neither rises when an eighth lands nor falls when one is collapsed (#17312). The rule is proven on a planted fork that shares no text with any real module, and on the two shapes that must NOT count: pure composition, and a vocabulary with no redaction entry point. The two autobot_shared redactors disagree on 7 of 10 sampled field names -- suffix matching vs substring, each a recorded ruling in its own file. Merging them would reverse one, so this documents the split and leaves it open. Refs #16688, #17312 * fix(redaction): correct the JWT divergence example to a token both rules size (#16688) The pre-push suite caught the previous commit's fixture asserting the wrong thing. `{}` base64url-encodes to `e30`, three characters -- below the `{8,}` this scanner requires for a segment and below the `{10,}` a2a/pii_pipeline.py requires, so NEITHER rule matched it. The test passed its "something matched" sibling only because the high-entropy rule claimed the span; the precise assertion (`pattern == "jwt"`) is what surfaced it, which is the argument for writing it that way. The divergence itself is real; the example was not. A payload starts "eyJ" only when its JSON begins exactly '{"'. A serializer emitting a space after the brace gives "eyAi" and one padding before it gives "IHsi" -- both long enough, both matched by pii_pipeline, both missed by this scanner before the widening. Fixture replaced with such a token, decoding to { "sub": "1234567890", "name": "A" }, verified matched by the new rule and unmatched by the old one. The source comment carried the same wrong example and is corrected with it. Refs #16688 * chore(ratchets): re-pin audio-extension-allowlist, red at zero headroom on main (#16688, #16724) origin/main sits at EXACTLY zero headroom on this floor: population 6150, floor 5450, and 6150 - 5450 = 700 = skips 300 + growth 400. Any branch adding one tracked .py file turns it red. This branch adds one test file, measures 6151, slack 701 -- one over. The floor is stale, not the branch. Measured on this rebased tree, two methods, not carried from another session: git ls-files "*.py" = 6151 (origin/main 6150) git ls-tree -r HEAD = 6151 (origin/main 6150) Re-pinned to 5700, NOT the 5751 that population - growth gives. The binding constraint is completed(), not verify_floor(): the guard's loop continues past the canonical file and everything under repo_tests/ before reading anything, so completed = 6151 - 386 - 1 = 5764. A 5751 floor leaves 13 files of margin against a number that FALLS as repo_tests/ grows -- and this branch grows it. 5700 leaves 64, comparable to the 47 the 5450 pin carried against its measured completed of 5497, which held from #16702 until now. verify_floor: 6151 - 5700 = 451 <= 300 + 400 = 700 ok completed: 5764 >= 5700, margin 64 ok skips and growth unchanged: neither is short, the floor was stale. #16724 already tracks re-measuring this from CI's own log rather than from a tree walk. Checked for the duplicated `floor=` keyword a conflict resolution produced elsewhere today: one occurrence. Also measured, not changed: hooks-path-override is green here but at ZERO headroom (population 6937 incl. 31 extensionless shell scripts, floor 6536, slack 401, allowance 401); origin/main has exactly 1. The next tracked .sh/.py/.yml/.yaml file anywhere turns it red. Left alone deliberately -- it passes, it is a hub file four sessions are touching tonight, and #17142 already tracks it. Refs #16688, #16724, #17142 * fix(docs): follow the four THREAT_MODEL anchors this branch moved (#16688) python-suite shards 5, 6, 8 and 10 were red on threat_model_anchors_resolve_test. The cause is in this branch's own diff, not in base: the boundary pointer added to autobot_shared/security/redaction.py's module docstring inserted five lines above every definition in the file, and the citations in docs/developer/THREAT_MODEL.md still pointed at the old numbers. Base agrees with itself, checked before assuming: git show origin/main:autobot_shared/security/redaction.py \ | grep -n "def redact_provider_error" -> 140 git show origin/main:docs/developer/THREAT_MODEL.md \ | grep -n redact_provider_error -> 116: ... (:140) FOUR anchors moved, not one. The failing test names only the anchor its shard reached first, so fixing that one would have left three and produced another red run. Every citation in the document was re-resolved with the test's own rule (AST definition lookup, not a line regex) rather than only the reported case: redact_text :97 -> :102 redact_mapping :109 -> :114 redact_cloud_identifiers :126 -> :131 redact_provider_error :140 -> :145 The two loader.py anchors that a definition-only check flags are NOT broken: import_module and spec_from_file_location are stdlib CALL sites, and the test has an explicit branch for anchors with no definition in the file. A first pass listed them as failures; that was the probe being wrong, not the doc. The other seven modules this branch adds a docstring pointer to are unaffected -- the sweep covered all 38 anchors, not just redaction.py, and they are either unanchored or anchored above the insertion point. Verified: repo_tests/threat_model_anchors_resolve_test.py -- 41 passed. THREAT_MODEL.md stays at 140 lines, which is its own ceiling (test_document_exists_and_is_short_enough_to_read_every_review asserts <= 140); this change edits line numbers in place and adds none. Root-caused by autobot-ai-f8 while sweeping, including the base-vs-branch attribution check; the widening from one anchor to four is this commit. Refs #16688 * fix(guards,ratchets): route the census through the one enumeration, pre-empt a zero-headroom floor (#16688) Both changes are pre-emptive: this branch was GREEN when they were made. It was green against a base that has since moved, which is the whole point. THE CENSUS GUARD WAS A SECOND FORK OF THE ENUMERATION. redaction_concept_census_test shelled out to `git ls-files` for itself, exactly as enum_hand_copy_guard did on the #14881 branch. one_git_enumeration_15926_test::test_the_direct_invocation_count_only_shrinks caught that one in CI; rebasing this branch onto origin/main 3f0d4b3 reproduces it here -- 34 invocations against a MAX of 33 -- because main reached 33 after this branch's last run. Found by predicting it and checking, not by a red. A census guard forking the repo's enumeration, in a PR whose subject is forks, is the joke writing itself. Routed through tools.lint._scan_helpers.tracked_paths, whose `exclude` becomes a git `:(exclude)` pathspec so git does the matching -- rather than the second Python-side matcher this had, which is how #15510 fired an exclusion against absolute paths and matched every file. subprocess and scrubbed_git_env are no longer imported. HOOKS-PATH-OVERRIDE 6536 -> 6800 WHILE PASSING. Measured on this rebased tree: population 6937 (glob hits for *.sh/*.py/*.yml/*.yaml plus 31 extensionless files whose shebang says shell), completed 6936, slack 401 against growth 400 + skips 1 = exactly 401. That is not "passing", it is "passing until the next file anyone lands", and four sessions moved main in the hour this branch sat in review. The cost of pre-empting is one line on a hub file three other PRs already change to the same value; the cost of not is an hour of CI and a round trip on a predictable red. 6800 is mid-window of [6536, 6936], not the minimum -- fifteen previous re-pins took the bottom, which is why there were fifteen. It is deliberately the same value as autobot-ai-58's #17338 and this session's #17342 and #17346: one measurement should not produce four numbers, and identical lines cannot conflict. First to land wins, the rest adopt on rebase. The other three floors were measured here and left alone, because this branch does not move their populations: audio-extension-allowlist 6151/5764 against the 5700 pinned earlier on this branch (margin 64), prompt-injection-detector-strict-mode 3256 (no new backend non-test module), import-hermeticity 662 (no new module under api/ or autobot_shared/). Two of those sit at exactly zero headroom on main and will need re-pinning by whichever branch next adds to their roots -- see #17142. Verified on this tree: one_git_enumeration + redaction_concept_census + threat_model_anchors + prompt_injection_detector_strict_mode + reach_declarations ... 274 passed reach_declarations_test is run with the prompt-injection module in the same session on purpose: its declaration reaches the sweep only when something else has imported it, so repo_tests/ alone silently checks 35 of 36 floors. Refs #16688, #15926, #17142 * chore(ratchets): record that this branch adopted main's floors, not its own (#16688) Empty commit: the change is in the rebase conflict resolutions, but the commits carrying them still SAY 6800 and 5700 in their messages because that is what they proposed when written. Stating it once beats rewriting four messages. Rebased onto origin/main 19006b3. Both floors adopted, first-to-land wins: hooks-path-override proposed 6800 -> adopted main's 6736 (#17318) audio-extension-allowlist proposed 5700 -> adopted main's 5752 (#17317) Re-derived on this tree rather than taken on trust: audio pop 6153 completed 5766 pin 5752 slack 401/700 margin 14 hooks pop 6939 completed 6938 pin 6736 slack 203/401 margin 202 prompt-inj pop 3256 pin 2956 slack 300/300 ZERO headroom import-herm pop 662 pin 602 slack 60/60 ZERO headroom The pre-emptive 6800 on this branch was still the right call when made -- it sat at slack 401 against an allowance of 401, one file from red, while passing -- but main moved first and one measurement should not produce four numbers. FOR #17142, the measurement that took three branches to produce. audio's pin is population - growth, the GAP bound, while the bound that fires on this guard is completed(): its loop continues past the canonical file and all of repo_tests/ before reading anything, so completed sits ~387 below population and falls every time anyone adds a guard. Main pinned 5752 with an apparent 402 of headroom. The real figure, measured on this session's three branches: issue-13049-lazy-import margin 16 issue-14881-enum-hand-copies margin 15 issue-16688-fork-collapse margin 14 Three ordinary branches, each adding one or two repo_tests guards, took it from 16 to 14. That is the allowance being wrong, not the floors, and it is why this guard has a re-pin history at all. Verified on this tree: prompt_injection_detector_strict_mode + reach_declarations + one_git_enumeration + redaction_concept_census + threat_model_anchors ................................... 274 passed Refs #16688, #17142, #17317, #17318 * fix(guards): migrate the census floor to declare(), re-measured not carried (#16688, #15928) reach_floor_migration_test::test_no_new_hand_rolled_floor_remains_unmigrated failed: redaction_concept_census_test declared `_MIN_SOURCE_FILES` as a hand-rolled reach-sized floor. #15928's rule is that a floor >=100 which binds a tree sweep goes through _reach.declare(), because a number chosen by feel decays silently. IT HAD ALREADY DECAYED, IN THE FOUR HOURS SINCE I WROTE IT. The constant was 1500. Re-measured with a throwaway script, as #15928 requires rather than carrying the old value across: the real population is 3,393 tracked production .py files (excluding .worktrees/, docs/, repo_tests/ and test files). 1500 sat 56% BELOW the tree it claimed to bind -- it would have passed over a sweep that had lost more than half the codebase and still reported "no new redaction implementations". Which is the argument #15928 makes, demonstrated on a guard whose own docstring is about not confusing "nothing found" with "did not look". I wrote the number by feel in a file about measurement discipline. declare("redaction-concept-census", floor=3093, growth=300, what="tracked production python files") growth=300 matches the sibling declaration in prompt_injection_detector_strict_mode_test over a population of similar size. Also wired REACH.completed(parsed), which the hand-rolled version had no way to express: the floor now binds what the census actually PARSED, not merely what it listed. Without it the sweep could enumerate 3,393 files, fail to parse 3,000 of them, and report the same clean as a tree with nothing in it -- the exact distinction this guard exists to make. discover() takes a root and returns [] on EmptyEnumeration rather than letting it escape: reach_declarations_test drives every declaration against an empty repository and demands ReachFloorError specifically, so letting EmptyEnumeration through means the declaration fails loudly while saying nothing about whether its floor binds. Found by autobot-ai-f8 reading the shard-8 log on the sibling #17342 branch, whose enum_hand_copy_guard_test.py carries the identical defect. Confirmed here by running the migration guard rather than assuming the sibling's diagnosis transferred. Verified on this tree: reach_floor_migration + redaction_concept_census + reach_declarations ...................................... 206 passed Refs #16688, #15928, #17312 * chore(ratchets): adopt main's floors and re-pin the census I set at the minimum (#16688, #17142) Rebased onto origin/main e8c7bf9. Main's floors adopted where they hold; the one this branch created did not. audio-extension-allowlist adopted main's 5757 (NOT 5752 -- superseded) hooks-path-override adopted main's 6736 prompt-injection-... adopted main's 2958, green here import-hermeticity adopted main's 604, green here redaction-concept-census 3093 -> 3247, MINE, and it was red THE CENSUS FLOOR WAS MY OWN MINIMUM PIN, AND IT IS THE SECOND ONE TONIGHT. I created that declaration earlier in this branch, migrating a hand-rolled floor under #15928, and pinned it at 3093 -- population 3393 minus growth 300, which is the MINIMUM OF THE WINDOW. It went red on this rebase: population reached 3397, slack 304 against an allowance of 300. Four files. The sibling declaration on the #14881 branch (enum-hand-copy-census, pinned 5755 at population 6155) went red the same way at 6165. Ten files. Both were committed AFTER I filed the comment on #17142 arguing that `population - growth` is not a floor but a snapshot with zero tolerance by construction. So: wrote the argument, made the mistake twice, noticed neither time until reach_declarations_test failed. That is the case for the check being mechanical rather than a review item, and it is self-reported on #17142 with timestamps because "people who understand the rule will follow it" is the objection it removes. Re-measured on this tree rather than adjusted: 3,397 tracked production .py files using the guard's own filter. Window [3097, 3397]; 3247 is mid-window and buys ~150 files of headroom instead of four. Measured after the re-pin: audio pop 6163 completed 5776 pin 5757 slack 406/700 margin 19 hooks pop 6949 pin 6736 slack 213/401 prompt-inj pop 3258 pin 2958 slack 300/300 ZERO headroom import-herm pop 664 pin 604 slack 60/60 ZERO headroom census pop 3397 pin 3247 slack 150/300 The two at zero headroom are adopted unchanged because this branch adds nothing to their roots. #17346 carries prompt-injection at 3100 and #17342 carries import-hermeticity at 633 -- those are the branches that do. audio is 5757, not the 5752 quoted to me during review: #17317's value was superseded while that message was in flight. Read by AST at e8c7bf9 rather than taken from the message, which is the same discipline this session has been applying all night -- a value quoted by name is already a snapshot. Verified on this tree: reach_declarations + reach_floor_migration + redaction_concept_census + threat_model_anchors .......... 247 passed Refs #16688, #17142, #15928 * fix(docs): delete a floor comment narrating a re-pin this branch does not ship (#16688) Third instance of the same defect, found by checking rather than by review. autobot-ai-f8 caught it on #17342 (fixed as acbfbe3) and #17346 (67f977f); this branch is the third of the three that re-pinned this declaration tonight, so it was checked before anyone had to report it. git diff origin/main...HEAD -- repo_tests/audio_extension_allowlist_test.py \ | grep -c 'floor=' -> 0 the file ships -> floor=5757 the block this branch ADDED said -> "Ratcheted 5450 -> 5700 (#16688)" An earlier revision of this branch did propose 5700 and the block described it correctly then. #17317 landed 5752, later 5757, and the rebase adopted main's value; the `floor=` hunk was resolved and the prose, living in a different commit, survived as a pure addition. The branch ended up ADDING a precise narrative about a change it no longer makes. Deleted rather than corrected, matching the #17346 fix: main's own comment sits immediately below explaining 5757, so a correction block would leave two voices on one floor -- which is the defect, not its remedy. The diff no longer touches this file. Note this differs from the #17342 fix, which rewrote its block instead. That was the right call there and the wrong one to repeat: at the time I had not seen that main already carried a true account a few lines down, so the correction became a second voice too. It is deleted there as well. WHY THIS FILE AND NO OTHER, which is f8's observation: three branches re-pinned this one declaration tonight. A floor whose comment block is append-only across concurrent branches accumulates contradictory narratives even when every individual re-pin is correct, because the rebase resolves the value and nothing resolves the sentence. The durable record of the mechanism belongs in commit messages and on #17142 -- not as competing prose in a hub file. Refs #16688, #17142, #17356 * chore(ratchets): ship main's floor files unmodified, and drop measurements from prose (#16688) Two distinct problems, both found by grepping this branch's own diff for prose naming a measurement -- which is the check autobot-ai-f8 asked for after the orphaned-narrative round. 1. THIS BRANCH WAS SHIPPING TWO STALE FLOORS AND TWO SUPERSEDED COPIES OF MAIN'S OWN COMMENTS. audio-extension-allowlist carried 5757 with main's older narrative while main is at 5764 with a rewritten one; prompt-injection-detector-strict-mode carried 2958's text while main is at 2960. Neither needs a branch-specific value -- measured on this tree, main's floors pass here -- so both files are now byte-identical to origin/main. That is worse than an orphaned note of my own: a superseded copy of ANOTHER branch's comment silently misattributes a stale account to them, and reads as authoritative because it is written in their voice. 2. EVERY MEASUREMENT IS OUT OF THE PROSE THIS BRANCH ADDS. The census declaration's migration note, its re-pin note, the hooks adoption note and one line of REDACTION_BOUNDARY.md all cited a population or a slack figure. Each was correct when written and stale one rebase later. The last one is the instructive case: "enumerate 3,393 files, fail to parse 3,000 of them" was an ILLUSTRATION, not a claim -- and an illustration carrying a real population goes stale exactly like a measurement while reading as more authoritative for being precise. Now "enumerate the whole tree, fail to parse most of it", which stays true. Prose that cites no measurement cannot go stale. The arithmetic lives in commit messages, which cannot drift from the tree they describe because they are not in it. Verified on this tree: reach_declarations + reach_floor_migration + redaction_concept_census ................................ 206 passed Refs #16688, #17142, #17356 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
mrveiss
added a commit
that referenced
this pull request
Sep 24, 2026
…17357, #17371, #17362, #17387) (#17398) * fix(venv): never leave a provenance marker RECORD cannot list (#17357) #17338 appended the marker to the distribution's RECORD, which is what stops a husk forming, but it did so as a second step that could silently no-op: no RECORD to append to, or an OSError swallowed by the same handler as the marker write. Either way the marker stayed on disk unrecorded -- the exact #17332 state, for that one package, produced by the code written to prevent it. The stamp and its RECORD entry are now one operation. _record_marker_in_record reports whether the marker is listed, and a marker that cannot be listed is taken back off disk. That is a rollback of this call's own write, never a cleanup of earlier state: the only file it can remove is the one written microseconds before in the same call. Why unstamped rather than stamp-anyway, which is the decision #17357 left open: write_provenance_marker's own docstring already promised this posture -- "a marker write failure means the NEXT candidacy check for this package fails closed (unverified)" -- while the code left an unrecorded marker instead. So this is not a new trade being chosen, it is the function doing what it says two lines above itself. The cost is provenance for one package whose RECORD is missing or unwritable; unverified is a state this module already handles, behind AUTOBOT_VENV_RECONCILE_ALLOW_UNVERIFIED_REMOVAL, whereas a husk breaks pip for every package after it. Tests: test_a_distribution_without_a_record_is_still_stamped asserted the old answer and is replaced by _is_left_unstamped, with the reasoning in the test rather than only in the issue. Two new tests cover the arm that had no coverage at all -- RECORD present but unwritable -- one for the marker and one for what its absence buys, that the dist-info still dies with its own uninstall. Monkeypatched rather than chmod-ed because as root the permission bits would not refuse the write and the test would pass without reproducing anything. Mutation-checked per path: removing the except-arm rollback fails the two unwritable-RECORD tests and leaves the absent-RECORD one passing; removing the no-RECORD rollback fails only the absent-RECORD test. Each test is bound to its own path rather than to the pair. Closes #17357 * fix(venv): make the RECORD rewrite atomic and stop the husk scan following links (#17371, #17362) Two same-file bugs batched onto #17357's PR rather than opening two more: both are in venv_provenance.py and #17371 is in the exact function #17357 changed. #17371 -- RECORD was rewritten with Path.write_text, which opens "w" and truncates before writing, so an interrupted run could leave a package's manifest short or empty. That is worse than the husk this function exists to prevent: a husk is locally repairable, a truncated RECORD breaks pip's management of a package that was working fine, chosen by whatever happened to be mid-write. The reconciler re-stamps every declared package on every run, so the exposure is per package per run. Now: temp file in the same directory, fsync, mode and ownership carried over, os.replace. The temp file is removed on any failure -- BaseException, because an interrupted provisioning run is the scenario -- so no RECORD.* litter survives. Ownership matters specifically because #17339 made this run as root on every provisioned host: os.replace installs the temp file's own owner, so without the chown a RECORD comes back root-owned and breaks pip for the venv's user -- the failure this module exists to prevent, by another route. Also #17371: UnicodeDecodeError is a ValueError, so the caller's except OSError never caught it and a non-UTF-8 RECORD aborted every remaining package in mark_current_set's loop -- fatal, against that caller's "never fatal to the surrounding install". Now treated as unrecordable, exactly like a missing RECORD. #17362 -- is_provenance_husk classified with is_dir()/iterdir(), which follow symlinks, so a *.dist-info link pointing at a directory holding a lone AUTOBOT_PROVENANCE file matched, and the removal reached THROUGH the link to unlink that file outside site-packages. It contradicted the docstring's own promise that the predicate "cannot reach a real installation, an operator's files, or another tool's". The predicate now refuses a symlink, and the removal enforces it independently: _unlink_marker_within opens with O_NOFOLLOW | O_DIRECTORY and unlinks by dir_fd, so it cannot act outside the directory it is clearing even if a link is planted after the check. Test-harness defect found while doing this, and it is the part worth reading: the two #17357 tests injected their fault by patching Path.write_text filtered on a file named RECORD. Making the write atomic moved it to RECORD.autobot-provenance-tmp via builtin open, so the patch stopped firing. One test failed loudly; the other kept passing while reproducing nothing, and a test that no longer injects the fault it names is indistinguishable from a working fix. Both now go through _record_update_refused, which takes the suffix from the module under test and asserts on exit that the injection actually fired -- so the next time the write moves, it fails instead of going quiet. Mutation-checked, 8 of 8 caught, each by its own test: symlink guard, O_NOFOLLOW unlink, atomic replace, UnicodeDecodeError arm, mode/ownership preservation, temp cleanup, and the two #17357 rollback arms re-verified after the refactor. Source restored byte-identical after each. Closes #17371 Closes #17362 * fix(ansible): test the installed STATE, not dpkg's knowledge of a package (#17387) Fleet provisioning died on a node whose PostgreSQL was removed but not purged. `dpkg -l <pkg>` answers "does dpkg KNOW this package", not "is it installed": an rc-state package (deinstall ok config-files) still exits 0. So the install was skipped, the postgres system user was never created, and pg_createcluster -- become_user: postgres -- failed with `sudo: unknown user postgres`. Late, and naming the wrong thing. Self-perpetuating, which is why it needed a guard and not a one-line fix: the run fails before anything installs PostgreSQL, so the next run finds the same rc-state package and skips the install again. It was a pattern with a documented false premise. Four roles carried the same detection AND the same comment asserting "dpkg exits rc=1 when package is not installed" (#2872) -- postgresql, monitoring/grafana, redis and disable-idle-nginx. The wrong belief was written down once and copied three times, so fixing one site would have left three, each explaining why it was correct. All four now use dpkg-query --showformat=${Status} and gate on 'install ok installed' in .stdout. AC3: pg_createcluster's guard fired when the data directory was absent, which is exactly the state a removed package leaves -- so it fired precisely when the postgres user was most likely missing. It now requires the user to exist, and a new task fails with the real cause naming the uninstalled package instead of the missing user. AC5's sweep found two things I had not been looking for, and both changed the answer: - install.sh:1141 gates `apt-get purge` on dpkg -l. That is CORRECT and is NOT fixed: purging an rc-state package is the entire point, and anchoring it on ^ii would skip exactly what the loop exists to clean up. Recorded as an exemption with that reason. - setup_browser_vnc.sh grep'd dpkg -l output unanchored, so an rc-state tigervnc reported as installed. Fixed by anchoring on ^ii. decommission-node.yml and secure-vm-exec.sh were already anchored. New guard repo_tests/dpkg_tests_installed_state_not_knowledge_17387_test.py. It keys on the COMMAND, never the variable name: five *_installed.rc gates read like this defect and are not -- they register `which ufw`, where rc is the right test because an rc-state package leaves no binary. Verified before excluding them rather than pattern-matching the name. Two false positives the guard found in itself, both fixed before this commit and both worth naming: it flagged its own failure message (a YAML msg: block is not a comment, so a comment-position check does not see it -- now prose in backticks is skipped), and it flagged `fuser /var/lib/dpkg/lock-frontend` because the PATH contains "dpkg" -- now narrowed to dpkg -l / dpkg-query. Reach floors declared so "nothing found" cannot pass for "did not look": 4 call sites (floor 3) and 6 dpkg registrations (floor 4, raised from 3 because at 3 the window exactly equalled the growth allowance and a single removal reddened it). Mutation check per AC4: reverting the postgresql detection to dpkg -l reddens the guard by name, naming install.yml and all six gate lines. Measured rather than assumed -- this host currently has 2 packages sitting in rc state, so the failing state is not hypothetical. Closes #17387 * fix(guards): the dpkg reach declarations ignored the root they were given (#17387) python-suite shard 12/12 red on four cases in reach_declarations_test: dpkg-ansible-registrations returned 6 items from an empty tree instead of failing dpkg-list-call-sites returned 4 items from an empty tree instead of failing dpkg-ansible-registrations returned identical results for an empty directory and the repository, so its discovery ignores the root it is given. A floor it clears unconditionally measures nothing. Both discovery functions took a root, named it `_root`, and never used it -- `_scanned_files()` read `repo_root()` itself. So both floors cleared unconditionally, against the real tree, no matter what root they were handed. The underscore was the tell: the signature accepted a root to satisfy the protocol and the body ignored it. Root now threaded through `_scanned_files(root)` and both callers, including the `relative_to(base)` that was also pinned to `repo_root()`. Measured: empty tree -> call sites 0, registrations 0 repository -> call sites 4, registrations 6 This is the eighth instance today of the class I posted to #15826 an hour ago -- an instrument returning a well-formed answer for a state it cannot distinguish -- and it is in the guard I wrote to enforce exactly that discipline. A floor that cannot fail is the same defect as a sweep that cannot see, and I shipped one while documenting the other. The meta-test caught it, which is the countermeasure #15826 proposes working as designed: `test_discovery_honours_the_root_it_is_given` exists because a floor is only worth what its discovery can fail to find. Nothing in my own review would have caught it -- the guard passed, its floors verified, and the numbers were right for the wrong reason. 10 passed in reach_declarations_test -k dpkg; 4 passed in the guard. Refs #17387, #15826 * fix(ansible): a held package is installed -- the new gate said it was not (#17387) Pre-merge review flagged this as a LOW theoretical edge on the grounds that the old `dpkg -l` + `.rc` check shared the blind spot. It did not. This is a regression I introduced, and the check it replaced got the case right: held + installed old: `dpkg -l` rc=0 -> `rc != 0` False -> skip install CORRECT new: stdout `hold ok installed` -> 'install ok installed' in stdout is False -> install re-runs on a package that is installed WRONG dpkg's Status is `<want> <error> <status>`. `want` is install/hold/deinstall and says nothing about presence; only the third field does. Keying on the whole `install ok installed` string tests want AND status, so `apt-mark hold` makes an installed package read as absent. Gating on `'ok installed'` instead, in all 18 places across the four roles. Verified against every Status dpkg can emit: install ok installed installed=True matches hold ok installed installed=True matches <- the regression deinstall ok config-files installed=False no match unknown ok not-installed installed=False no match install ok half-installed installed=False no match install ok half-configured installed=False no match `half-installed` and `not-installed` do not contain `ok installed`, so the shorter literal is not merely looser -- it is the one that matches the field that carries the answer. test_every_gate_literal_classifies_every_dpkg_status reads the literals out of the ansible files and checks them against that table, so the regression cannot come back silently. Mutation: restore 'install ok installed' in install.yml and it fails on exactly [hold ok installed-True]. Also from the review, and the reason it is worth catching now: the scan behind that test was first written to match every `'x' in y.stdout` in the ansible tree. It collected PONG, active (running), role:master and twenty others, then checked unrelated gates against a dpkg truth table. The test failed, which is the only reason the scan was caught being wrong rather than the code. Now scoped through _dpkg_registrations(), which yields exactly one literal. Second review finding fixed: clear_provenance_husks caught only OSError, but `os.unlink(..., dir_fd=fd)` raises NotImplementedError -- not an OSError subclass -- before the syscall on a platform without dir_fd support. Uncaught it would abandon the whole sweep rather than skip one husk. True on Linux today, which is the only platform provisioned, so this is an untested edge rather than a live bug. 17 passed in the venv suite, 11 in the dpkg guard. Refs #17387, #17362 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
4 of 6 tasks
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.
Refs #17331
Refs #17246
Refs #17332
Refs #17242
Two failures, one run, one PR. The non-manager node and the manager node failed the same
provisioning task for different reasons; #17331 is the first and #17332 is the second.
No issue closes on this merge, deliberately. #17331's last criterion is a multi-host
provisioning run, which this session cannot perform; #17246's third criterion —
_KNOWN_UNSTAGEDempty and the exemption mechanism removed — waits on #17334; #17332's repair is unreachable from
the ansible provisioning path, filed as #17339. A merge is not a closure argument.
Thinking Path
The owner hit a live provisioning failure this evening. The obvious suspect was #17242 —
"provisioning aborts on every non-manager host" — but that had already merged (
b77f0d618a) andthe deployed checkout at
31310392dalready carried it. So the first job was to stop guessing andread the run:
#17242 moved the generation of the filtered manifest onto the controller and left the path that
generation writes into the file pointing at
code_source. pip resolves a nested-cagainstthe directory of the file containing it (
pip/_internal/req/req_file.py,_parse_and_recursejoins to
os.path.dirname(filename)), so the controller's path is the path pip opens on thetarget. The executor was fixed; the payload was not.
That also explains why the #17243 guard did not catch it: it reads a task's payload, and this
path lives in the file's contents, written at run time by a shell script. A guard that reads
task payloads structurally cannot see a path a generator writes — so this needs a different guard,
not a widened matcher.
Fixing it required staging
constraints/shared.txton the node, which is the same thing the sevenpip:sites baselined by #17246 need. One mechanism, so one PR — and the same-file rule points thesame way.
One of the seven is deliberately not fixed here, and #17246's third criterion stays unmet as a
result.
roles/npu-workerreads its whole manifest from the checkout, and that manifesteditable-installs
autobot_shared. Routing it through the filter script — which strips that line —would silently drop the dependency: nothing else installs it in that venv (
roles/autobot_sharedsyncs to a PYTHONPATH directory and only runs under
deploy_role == 'autobot_shared'). That is#11135's class of regression, so it is filed as #17334 with the decision it needs and stays as
the single remaining baseline entry rather than being forced into a shape that fits.
What Changed
scripts/build-filtered-requirements.shservices/role_registry.py's post_sync_cmd (reads and installs on the manager) working unchanged.roles/_shared/tasks/stage_shared_manifests.ymlconstraints/shared.txtand the rootrequirements.txtonto the node. Per-host fact guard;tags: [always]for the reasonupdate-all-nodes.ymlgives at its own staging task.roles/backend/tasks/main.yml{{ autobot.base_dir }}as the rewrite root. Also corrects #17242's false invariant comment — "code_source … always exists as the git checkout" — which is the sentence that hid this half.roles/ai-stack/tasks/main.ymlroles/{backend_services,browser,tts-worker}/tasks/main.yml,roles/agent_config/tasks/{openvino,playwright,python_deps}.yml-cnow reads the staged copy, each includes the staging task.repo_tests/ansible_generated_manifest_paths_17331_test.pycode_source, the files it names are staged, and each role includes the staging task. Floor (_MIN_INVOCATIONS_SEEN) so an empty walk fails instead of passing.repo_tests/ansible_code_source_delegation_17243_test.py_KNOWN_UNSTAGEDshrinks 7 → 1, with the npu-worker reason recorded.repo_tests/test_deploy_constraint_rewrite_14272.pycode_sourcein the output.repo_tests/glob_declared_reads_15900_test.py*.ymldeclaration.#17332 — the manager node's half of the same run
The manager failed the same task for a different reason:
error: uninstall-no-record-file — Cannot uninstall cachetools None. The #15067 provenance marker is written inside a distribution'sdist-infoand was not listed in itsRECORD. pip deletes whatRECORDnames and then thedirectory if nothing is left, so an unrecorded file keeps the directory alive; Python then reports
a second distribution at version
Noneand every later pip run for that package aborts.Thirteen distributions across two venvs on this host were already in that state, and pip stops
at the first — thirteen failed deploys to find them all.
The count was twelve until f8 swept every venv instead of the one this issue named; a one-venv
measurement had been written with host-wide language. Re-measured here before correcting:
autobot-backend/venv12,autobot-slm-backend/venv1 (psycopg2_binary-2.9.12, live sibling2.9.13with a 46-line RECORD), the other five venvs 0. The thirteenth is in the SLM's ownvenv — the component that runs the builtin updater, which is the only path this repair is wired
into.
venv_provenance.pyRECORD, which makes the module's central claim — "a reinstall erases the marker" — true rather than assumed.is_provenance_husk+clear_provenance_husksrepair a whole venv in one pass.has_tool_provenancerefuses a husk, which used to answer "ours" for a distribution that is not installed. The docstring's false invariant is corrected at the sentence that states it.api/venv_reconcile.pytests/test_venv_provenance_husk_17332.pytests/test_venv_provenance.py,tests/test_venv_reconcile.pydist-infowithmkdir()alone — shaped exactly like the husk — so they now describe real installs through one shared helper._StubPathgrowsparents, modelled on pathlib's sequence rather than one plausible value.One fix here is not mine and was not optional. The sys.modules leak guard blocked the push
naming a new file, and the leak was in
tests/api/test_worker_component_resolve_12450.py, whichinstalled
multipartstubs at module import and never removed them — so they outlived it andwere whatever the next file to import inherited. The guard names the file that sees a leak, not
the one that causes it, which is why this only surfaced once the venv tests joined the same
pre-push selection. The stubs are now installed, used for the import, and removed in one
try/finally. Nothing was added torepo_tests/sys_modules_leak_baseline.txt; that list onlyshrinks.
Merging this does not repair this host, and the body should not be read as saying otherwise.
The husks predate the fix, and
clear_provenance_husksis reachable only from_run_pip_install—
grep -rn 'venv_reconcile\|clear_provenance_husks' autobot-slm-backend/ansible/returnsnothing, and this machine is ansible-provisioned. Prevention lands with this PR; repair of the
thirteen that already exist needs #17339, or an owner-approved removal. Deletion under the
install tree is the owner's call (#17038), and no agent on this machine has taken it.
On #17038:
clear_provenance_husksremoves tool-authored debris only — a directory that describesno installed distribution and whose sole content is this module's own marker. It cannot reach a
real installation, an operator's files, or another tool's, and every removal is logged by name.
Three latent sites fixed along the way: the GPU manifest generations passed
code_sourceas therewrite root. Those three manifests declare no relative includes today, so the rewrite is a no-op
and nothing failed — adding one
-c ../constraints/…line would have reproduced #17331 on afleet node.
Verification
Mutation-tested, both named in the new guard's docstring and both executed:
Baseline arithmetic, from the guard's own scan on this tree rather than a copied figure:
seen = 13is above the guard's_MIN_TASKS_SEEN = 8floor, so the scan still reaches itssubject rather than passing by matching nothing.
Every modified YAML parses, and every
include_tasksrelative path resolves to a file on disk(9 files checked, 0 problems). Note
roles/_shared/andansible/_shared/are differentdirectories —
../../_shared/from a role'stasks/reaches the first, which is where the newtask lives.
This is a local pass and local passes are not CI (#17144). What CANNOT be verified here, and
is stated rather than claimed: #17331's fix is unproven on a multi-host topology. The failure was
observed on one, the cause is read from the code and from pip's own resolution logic, but the
fixed run has not happened. The ai-stack site is an untested prediction by the same argument.
One observation, not filed:
ansible_shared_tasks_code_source_dir_default_16750_test.pyscanscomments as well as tasks, so prose quoting the defect pattern trips it. Arguably correct — a
commented pattern is one copy-paste from a real one — so the comment was reworded instead.
Single-issue rationale
This is not single-issue work — it carries #17331 and #17246 together, because they are the same
defect class fixed by one mechanism and they touch the same files (the same-file rule points the
same way). Neither is written as
Closes, so the batching check sees no closing link and asks forthis line.
Why it does not wait to ride with more: #17331 is a live provisioning blocker on every non-manager
node, confirmed from tonight's run, and every other open PR is in a different area (frontend,
llm_shared, the playbook log reader). Appending it to one of those would put an urgent deploy fix
behind an unrelated review.
Model Used
Claude Opus 5 (1M context)
🤖 Generated with Claude Code