Repository navigation
fix(guards): an AST finder for the secrets wiring guard, a reason on every dropped watch-folder event, and the #17502 revert fixture (#17822, #17547, #17558) - #17955
Merged
Conversation
…every dropped watch-folder event, and the #17502 revert fixture (#17822, #17547, #17558) #17822 finding 1 -- `repo_tests/secret_access_check_is_wired_16982_test.py`'s `_references_symbol` was `if _SYMBOL not in source`, raw text over the whole file. The owner's mutation on #16982 (2026-09-29): remove the real call and leave the docstring that describes it, and the guard asserting a security rule is wired stays green with the rule gone. Measured on that exact mutation -- old finder GREEN, new finder RED. The finder now walks `Name`, `Attribute`, `alias` and `getattr`-family string arguments, with a nine-case contrast pair (four prose shapes that must not match, five code shapes that must). #17547 AC3 -- `services/kb_folder_events.py` bound `logger` and made zero `logger.` calls, against 30 in the module it was split out of. All four early returns in `_handle_change` dropped a watch-folder event with no trace, so "the KB ignored my file" and "watchdog never fired" read the same. Each now names the folder, path, change type and reason; the failed cross-thread hand-off (#15636) is a warning where the three routine drops are debug. A structural test fails if a fifth silent return is ever added. #17558 Gap 2 AC2 -- only the inverse assertion existed, which a check that reports nothing ever satisfies. Reverting #17502 against the real requirement files reported three of #17557's four packages: `opentelemetry-proto` was silent because both its requirers say `==1.44.0`, which reads as a cap. It is not one -- those requirers are themselves unpinned once the fix is reverted, so the cap travels with them. `floating_packages` resolves that to a fixed point. Blast radius measured: the tree's reported set is `['protobuf']` before and after, and the fixture now reports all four. Closes #17822 Closes #17547 Closes #17558
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedToo many files! This PR contains 107 files, which is 7 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing (107)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request adds diagnostic logs for dropped watch-folder events, updates CI dependency-pin detection, and changes the secret-access wiring guard to identify code references through Python AST inspection. ChangesWatch-folder event diagnostics
CI dependency-pin detection
Secret-access wiring guard
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to Watch-folder drops are now logged, and dependency-pin checks are improved. The secret-access wiring guard can still pass when the access check is unreachable, so it does not fully protect against that regression. This is acceptable to merge with a follow-up. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changes improve repository checks and make dropped folder events diagnosable without showing new access authority. No new security vulnerability was established, but operational log exposure and end-to-end enforcement remain incompletely assessed. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
3 tasks
Owner
Author
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
1e3f09ea-56c6-45e9-9de6-d8c07c6ee0f8
📒 Files selected for processing (5)
autobot-backend/services/kb_folder_events.pyautobot-backend/tests/services/test_kb_watch_events_17546.pyrepo_tests/ci_plane_unenforced_pins_17558_test.pyrepo_tests/secret_access_check_is_wired_16982_test.pytools/lint/check_ci_plane_unenforced_pins.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
…17547) Review found the same defect at two sites: a detector that counts a call without asking whether execution reaches it. secret_access_check_is_wired_16982_test `_references_symbol` walked the whole AST, so `return True` with the access check left underneath still read as "wired" -- the owner's #17822 mutation in its most faithful form, and the one the comment-stripping fix did not cover. Walking now prunes each statement list after a terminator. Syntactic reachability only: an `if False:` body still counts, because guessing in the permissive direction is how a guard goes vacuous. `_forwards_by_name` scanned every argument of `getattr`/`setattr`/`hasattr`/ `delattr`, so `getattr("is_accessible_by", "other")` matched on the *object* position. Only the attribute-name position counts now. Contrast fixtures for both, plus two permissive controls (a call after a conditional return, and one after a sibling function returns) so the pruning cannot quietly swallow live references. Verified against the old detector: all three mutations read True before and False after; all four controls stay True. test_kb_watch_events_17546 `test_every_return_in_handle_change_logs_first` asked whether a `logger` call appeared anywhere in the same block as the bare return. A call placed below the return satisfied that and can never execute, so the guard accepted exactly the silent drop it exists to forbid. `_drop_sites` now counts only statements preceding the first bare return, with a three-way contrast fixture pinning the ordering. kb_folder_events The four drop logs named `path.name`, so two files called `notes.txt` in different subfolders of a recursive watch produced identical lines and the log could not say which file was lost. They now log the path relative to the watched root -- specific enough to identify the file, without putting the host's filesystem layout in a log. A path outside the root degrades to the basename rather than leaking the root it sits under. Refs #17822, #17547
… not just the floor (#17356, #17818, #17870, #16987) Closes #16987 Closes #17356 Closes #17818 Closes #17870 Also carries the draft-gate aggregator fix (#17957): `python-suite` read twelve draft-skipped shards and re-asserted `result != 'success'`, so every draft PR reported red for work deliberately not run. Guard added, parsed from YAML. Refs #17844, #17957, #17975
… not just the floor (#17356, #17818, #17870, #16987) Closes #16987 Closes #17356 Closes #17818 Closes #17870 Also carries the draft-gate aggregator fix (#17957): `python-suite` read twelve draft-skipped shards and re-asserted `result != 'success'`, so every draft PR reported red for work deliberately not run. Guard added, parsed from YAML. Refs #17844, #17957, #17975
The defect class is "a text-keyed guard is satisfied by its own comment". The ENABLER is that stripping comments was reimplemented per guard, so each new one rediscovered the hazard, re-solved it, or forgot -- and forgetting is silent, because the guard then passes. Fixing witnesses one at a time was never going to converge; this fixes the enabler. #17941 catalogued seven copies. An AST sweep of the live tree found 43. The catalogue was not the population, which is the same duplication problem one level up. CANONICAL HOME, and I nearly got this wrong. The first draft added `repo_tests/_code_lines.py` -- fork #44, written while fixing the forks. `tools/lint/_scan_helpers.py` is already the shared scan module, with 51 repo_tests files importing it, and it already owns `logical_lines` for backslash continuations. So the new module sits beside it as `tools/lint/_comment_syntax.py`, and its continuation folding DELEGATES to `logical_lines` rather than reimplementing it. `_scan_helpers` is 489 lines against a 600 ceiling, which is why this is a neighbour and not an append. WHAT THE SHARED ONE DOES THAT NONE OF THE PRIVATE ONES DID. Every copy dropped whole-line comments only, and `git_repo_root_calls_are_guarded_17418_test` stated the reason honestly: a `#` inside a string is not a comment, and a regex ignoring that reintroduces the class one level down. That reasoning was right, and it is why this scans character by character tracking quote state. It also handles block comments with the same tracking, which replaces the line-anchoring heuristic `slm_frontend_i18n_parity_test` needed because `import.meta.glob('../locales/*.json')` contains a block-comment opener -- the anchor approximated "not inside a string"; tracking quotes answers it. Heredocs are NOT parsed, so `strip_trailing=` defaults to False and a caller opts into the cleverness. The conservative default is what the migrated sites already relied on. All seven listed call sites migrated; none keeps a private copy. One is a real fix rather than a move: `dpkg_tests_installed_state_not_knowledge_17387_test` compared the position of a comment marker against the match position, which is not quote-aware, so a marker inside a string before the match skipped the real invocation -- a miss in the permissive direction. THE META-GUARD is AST-based, because a text scan for the stripping idiom matches its own source and every allowlist entry -- the #17941 defect reproducing inside its own fix. It ratchets: `_PENDING_PRIVATE_STRIPPERS` holds the 43 measured entries, a new copy fails `test_no_new_file_strips_comments_by_hand`, and a migrated file left listed fails `test_the_pending_list_only_shrinks`. That second test compares the record against the LIVE tree rather than against a mirrored record, which is #17970's point: a mirrored pair detects transcription errors, not movement. The detector excludes `assert` subtrees after a false positive on `git_repo_root_calls_are_guarded_17418_test:173`, which ASSERTS a fixture line is a comment rather than stripping one. A guard with false positives gets muted, which costs more than the copy it would have caught. Controls: one per syntax claimed, both directions per case (what is removed and what survives), the quote-awareness matrix, the glob that broke the heuristic, line-number preservation, and a contrast set for the meta-guard including the two prose shapes its own file contains. Refs #17941, #13916, #17312
…shared SkipLink serves both consoles (#17561, #17565, #17958) Closes #14771 Closes #17561 Closes #17565 Closes #17958 Also carries the fix for a review finding in the #17982/#17983 class: the NPU and Log Forwarding panels now track a successful load, so a failed fetch is no longer indistinguishable from defaults. Log Forwarding shows "Unknown" and disables Start/Stop rather than claiming "stopped". Refs #17984
…every dropped watch-folder event, and the #17502 revert fixture (#17822, #17547, #17558) #17822 finding 1 -- `repo_tests/secret_access_check_is_wired_16982_test.py`'s `_references_symbol` was `if _SYMBOL not in source`, raw text over the whole file. The owner's mutation on #16982 (2026-09-29): remove the real call and leave the docstring that describes it, and the guard asserting a security rule is wired stays green with the rule gone. Measured on that exact mutation -- old finder GREEN, new finder RED. The finder now walks `Name`, `Attribute`, `alias` and `getattr`-family string arguments, with a nine-case contrast pair (four prose shapes that must not match, five code shapes that must). #17547 AC3 -- `services/kb_folder_events.py` bound `logger` and made zero `logger.` calls, against 30 in the module it was split out of. All four early returns in `_handle_change` dropped a watch-folder event with no trace, so "the KB ignored my file" and "watchdog never fired" read the same. Each now names the folder, path, change type and reason; the failed cross-thread hand-off (#15636) is a warning where the three routine drops are debug. A structural test fails if a fifth silent return is ever added. #17558 Gap 2 AC2 -- only the inverse assertion existed, which a check that reports nothing ever satisfies. Reverting #17502 against the real requirement files reported three of #17557's four packages: `opentelemetry-proto` was silent because both its requirers say `==1.44.0`, which reads as a cap. It is not one -- those requirers are themselves unpinned once the fix is reverted, so the cap travels with them. `floating_packages` resolves that to a fixed point. Blast radius measured: the tree's reported set is `['protobuf']` before and after, and the fixture now reports all four. Closes #17822 Closes #17547 Closes #17558
…17547) Review found the same defect at two sites: a detector that counts a call without asking whether execution reaches it. secret_access_check_is_wired_16982_test `_references_symbol` walked the whole AST, so `return True` with the access check left underneath still read as "wired" -- the owner's #17822 mutation in its most faithful form, and the one the comment-stripping fix did not cover. Walking now prunes each statement list after a terminator. Syntactic reachability only: an `if False:` body still counts, because guessing in the permissive direction is how a guard goes vacuous. `_forwards_by_name` scanned every argument of `getattr`/`setattr`/`hasattr`/ `delattr`, so `getattr("is_accessible_by", "other")` matched on the *object* position. Only the attribute-name position counts now. Contrast fixtures for both, plus two permissive controls (a call after a conditional return, and one after a sibling function returns) so the pruning cannot quietly swallow live references. Verified against the old detector: all three mutations read True before and False after; all four controls stay True. test_kb_watch_events_17546 `test_every_return_in_handle_change_logs_first` asked whether a `logger` call appeared anywhere in the same block as the bare return. A call placed below the return satisfied that and can never execute, so the guard accepted exactly the silent drop it exists to forbid. `_drop_sites` now counts only statements preceding the first bare return, with a three-way contrast fixture pinning the ordering. kb_folder_events The four drop logs named `path.name`, so two files called `notes.txt` in different subfolders of a recursive watch produced identical lines and the log could not say which file was lost. They now log the path relative to the watched root -- specific enough to identify the file, without putting the host's filesystem layout in a log. A path outside the root degrades to the basename rather than leaking the root it sits under. Refs #17822, #17547
The defect class is "a text-keyed guard is satisfied by its own comment". The ENABLER is that stripping comments was reimplemented per guard, so each new one rediscovered the hazard, re-solved it, or forgot -- and forgetting is silent, because the guard then passes. Fixing witnesses one at a time was never going to converge; this fixes the enabler. #17941 catalogued seven copies. An AST sweep of the live tree found 43. The catalogue was not the population, which is the same duplication problem one level up. CANONICAL HOME, and I nearly got this wrong. The first draft added `repo_tests/_code_lines.py` -- fork #44, written while fixing the forks. `tools/lint/_scan_helpers.py` is already the shared scan module, with 51 repo_tests files importing it, and it already owns `logical_lines` for backslash continuations. So the new module sits beside it as `tools/lint/_comment_syntax.py`, and its continuation folding DELEGATES to `logical_lines` rather than reimplementing it. `_scan_helpers` is 489 lines against a 600 ceiling, which is why this is a neighbour and not an append. WHAT THE SHARED ONE DOES THAT NONE OF THE PRIVATE ONES DID. Every copy dropped whole-line comments only, and `git_repo_root_calls_are_guarded_17418_test` stated the reason honestly: a `#` inside a string is not a comment, and a regex ignoring that reintroduces the class one level down. That reasoning was right, and it is why this scans character by character tracking quote state. It also handles block comments with the same tracking, which replaces the line-anchoring heuristic `slm_frontend_i18n_parity_test` needed because `import.meta.glob('../locales/*.json')` contains a block-comment opener -- the anchor approximated "not inside a string"; tracking quotes answers it. Heredocs are NOT parsed, so `strip_trailing=` defaults to False and a caller opts into the cleverness. The conservative default is what the migrated sites already relied on. All seven listed call sites migrated; none keeps a private copy. One is a real fix rather than a move: `dpkg_tests_installed_state_not_knowledge_17387_test` compared the position of a comment marker against the match position, which is not quote-aware, so a marker inside a string before the match skipped the real invocation -- a miss in the permissive direction. THE META-GUARD is AST-based, because a text scan for the stripping idiom matches its own source and every allowlist entry -- the #17941 defect reproducing inside its own fix. It ratchets: `_PENDING_PRIVATE_STRIPPERS` holds the 43 measured entries, a new copy fails `test_no_new_file_strips_comments_by_hand`, and a migrated file left listed fails `test_the_pending_list_only_shrinks`. That second test compares the record against the LIVE tree rather than against a mirrored record, which is #17970's point: a mirrored pair detects transcription errors, not movement. The detector excludes `assert` subtrees after a false positive on `git_repo_root_calls_are_guarded_17418_test:173`, which ASSERTS a fixture line is a comment rather than stripping one. A guard with false positives gets muted, which costs more than the copy it would have caught. Controls: one per syntax claimed, both directions per case (what is removed and what survives), the quote-awareness matrix, the glob that broke the heuristic, line-number preservation, and a contrast set for the meta-guard including the two prose shapes its own file contains. Refs #17941, #13916, #17312
…17941) Two of the three are behaviour fixes, not moves. `release_sync_main._uncommented` stripped trailing comments with `line.split(" #", 1)[0]`, which is not quote-aware: `echo "a # b"` was truncated at its own data. The shared scanner tracks quote state, so only a marker outside quotes ends a line. `doc_index_worktree_contamination` skipped whole-line comments but not trailing ones, so `exec thing # the old doc-sync path` read as a live invocation. Its own docstring says a comment noting the #16934 move is fine -- that has to hold for a trailing comment too. `ansible_backend_path_defects` is the straight move, plus trailing-comment coverage it did not have. THE LIST IS NOT A BATCH. Reading the 43 showed three different shapes wearing one idiom, and migrating them alike would be an error in the other direction: (a) source scanners -- carry the #17941 hazard; migrate. (b) format parsers -- `requirements.txt`, the CodeQL ceiling list, a distributions list. A leading `#` is part of the FORMAT. The concept is "parse a config line", not "ignore a comment"; forcing them onto a comment-syntax helper conflates two concepts, which is this module's own error pointed the other way. They need their own shared parser. (c) comments as data -- `detect-hardcoded-values_test` extracts a comment HEADER, and `comment_line_number_citations_test`'s whole subject is comment line numbers. Migrating these deletes their input. That classification is now recorded in the allowlist, so a later pass does not read a shrinking count as licence to batch the remainder. Also worth recording: my first attempt at these three inserted the import by matching the last import-looking LINE, which landed it inside a string literal -- a fake script a test writes to disk -- and the module then failed with NameError. Text matching satisfied by something that is not code, in the commit removing text matching satisfied by something that is not code. The insertion now walks `tree.body` and anchors to a real top-level import. Allowlist re-derived from the live tree rather than hand-edited: 40 entries. Refs #17941, #13916
…17941) Four files, five copies -- `man_page_indexer_path_resolves` had two, and the ratchet is what found the second. One is a behaviour fix. `store_authority_test` split baseline entries on `line.split("#", 1)[0]`, which is not quote-aware, so an entry containing a quoted `#` was truncated at its own data. The shared scanner tracks quote state. `scan_secrets_hook_test` gains trailing-comment coverage it did not have: a hook line ending `# the old hex-escape form` named the needle without using it, and the guard read that as an offence. `man_page_indexer_path_resolves` and `slm_self_update_socket_activation_order` are straight moves. The latter's own docstring already records why it strips at all -- "a text scan cannot tell code from prose about code" -- which is #17941 stated by a guard that then wrote its own copy anyway. That is the enabler in one file: the hazard was understood, and the fix was still local. THE RATCHET CAUGHT MY OWN INCOMPLETE MIGRATION. After migrating four files the count went 40 -> 37, not 36. `man_page_indexer_path_resolves` stayed listed because it had a SECOND private copy sixty lines below the first, which I had not read. `test_the_pending_list_only_shrinks` compares the record against the live tree, so a file that still strips by hand cannot be delisted -- the discrepancy was the signal, and a mirrored-record check would have shown two agreeing numbers and nothing wrong (#17970). Allowlist re-derived from the tree after each step, never hand-edited. Refs #17941, #13916
…rded (#17941) `hardcoded_values_scope_agreement`, `parked_branch_merge_workflow` and `release_sync_main_test` onto the shared stripper. The last two performed the IDENTICAL operation -- strip comments from a workflow `run:` body -- in two files, which is the duplication in miniature: not a subtle fork, the same four lines twice. A FOURTH CLASS, found by trying and failing. `check_ci_system_package_ provisioning` cannot use the shared helper and that is correct, not laziness. It runs in a CI job that installs linters and NOT the application's dependencies, and `ci_system_package_provisioning_test` enforces this with a stdlib-only import check, plus a SECOND test keeping its one exemption (`_scan_helpers`) dependency-free so the exemption cannot launder new imports. Importing `_comment_syntax` there means relaxing both guards to buy a straight move that fixes no bug at that site. I reverted it and recorded the reason in the allowlist as class (d), dependency-constrained. Worth stating plainly, because the goal is one shared implementation: that goal has a real boundary. A module that must run in a minimal environment cannot take a dependency on a shared helper merely to be tidy, and a guard that forces it to would be trading a deliberate constraint for uniformity. The allowlist now distinguishes four classes -- migrate, format parser, comments as data, dependency-constrained -- so a later pass argues each entry rather than batching toward zero. Refs #17941, #13916
The rebase onto the new main rewrote commits that were already pushed, so the push was refused as non-fast-forward. Merging the published tip back in rather than force-pushing: the two sides are byte-identical on every file they share (verified file by file against origin/main), so this reconciles the histories without rewriting anything that was published. # Conflicts: # repo_tests/one_comment_stripper_17941_test.py
Owner
Author
Correction: the squash message for this merge overstates what landedI wrote the merge commit body for Actually merged — the comment-stripper work: NOT merged, despite the message saying so — the The cause: I committed that work locally as That is the same error class this PR is full of fixes for — asserting a state instead of measuring it — committed in the sentence describing the fixes. I caught it only because I checked main for the canonical definition afterwards instead of trusting the merge. What is true as of now
What happens to the unmerged half
|
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.
Thinking Path
Three issues that triage found were each one criterion from done, batched because
all three are guard-shaped and none of them shares a file with an open PR.
#17822 finding 1 — the mutation decided the design. The guard's finder was
if _SYMBOL not in source. The owner stated the killing mutation on #16982 on2026-09-29, and the first thing I did was run it rather than assume it: remove the
real call in the coordinator, leave the docstring that describes it. The old
finder stayed GREEN, so the guard that asserts a secret-access rule is wired
passes with the rule deleted. That decided the fix: a docstring is an
ast.Constantin statement position and a comment is not in the tree at all, sowalking for
Name/Attribute/aliasseparates prose from code byconstruction rather than by a heuristic chasing how people write comments.
A first attempt at the mutation —
return Trueinserted above the call — didnot kill the new guard, and that is recorded here rather than quietly fixed:
the call text is still in the AST as dead code. The issue's own wording ("remove
that call and leave a docstring that mentions it") is the mutation that bites,
and it is the one the result below reports.
The reference mechanisms are enumerated rather than guessed, because
MEASUREMENT_DISCIPLINE.mdsays a control witnesses only the shape it is writtenin.
getattr(obj, "symbol")is included as a string argument to a forwardingbuiltin specifically — counting string constants generally would reinstate the
defect, since a docstring is one.
#17547 AC3 — the control was already in the tree.
kb_folder_events.pybindsloggerat module scope and makes zerologger.calls;kb_folder_watcher.py, the module it was split out of, makes 30. Every earlyreturn in
_handle_changedropped a watch-folder event leaving nothing to read,so "the KB ignored my file" and "watchdog never fired" are the same observation.
There are four such returns, not the three named in triage — the fourth is the
#15636 cross-thread hand-off failure, which is the one that is not routine, so it
is a
warningwhere the other three aredebug.#17558 Gap 2 AC2 — the measurement changed the fix. The existing assertion is
the inverse ("OpenTelemetry is not reported"), which a check that reports nothing
ever satisfies. Building the revert fixture and running it reported three of
#17557's four packages.
opentelemetry-protowas silent because both itsrequirers say
opentelemetry-proto==1.44.0, which a per-requirement test reads asa structural cap.
It is not one.
==1.44.0is what grpc 1.44.0's metadata says, and grpc itself isunpinned once the fix is reverted — pip takes grpc 1.45.0, whose metadata says
==1.45.0, and proto moves with it. That is family F: a correct answer to "doesthis requirement have an upper bound", read as an answer to "can this package's
version climb".
floating_packagesresolves it to a fixed point, and the blastradius was measured before the rule was trusted:
['protobuf']on the tree beforeand after.
What Changed
repo_tests/secret_access_check_is_wired_16982_test.py_references_symbolis an AST walk (Name/Attribute/alias/getattr-family string arg), parameterised by symbol so the positive control uses the same instrument. Nine-case contrast pair added.autobot-backend/services/kb_folder_events.py_handle_changelog folder, path, change type and reason. Routine dropsdebug; the failed hand-offwarning.autobot-backend/tests/services/test_kb_watch_events_17546.pytools/lint/check_ci_plane_unenforced_pins.pyfloating_packages: a cap is structural only when the requirer stating it is itself held.unenforced_pairsconsumes it.repo_tests/ci_plane_unenforced_pins_17558_test.pyTestRevertingTheFixReproducesIt(the #17502 revert against the real requirement files) andTestFloatingPackages(the rule as data). One pre-existing fixture made faithful, with the half it assumed now stated as its own test.Deliberately not touched:
.github/workflows/code-quality.yml— see the lastsection.
Verification
Commands run with
PATH=/home/martins/.venv-python-suite/bin:$PATH(python 3.14.6;the system
python3is 3.10 and fails collection onStrEnum).Suites —
101 passedacross the five changed suites pluscode_quality_guard_reach_test.py,watchdog_thread_handoff_15636_test.pyanddependency_floor_report_test.py. Pre-push selected and passed 5 suites.Mutation results — every new assertion was mutation-checked.
secret.is_accessible_by(...)call inservices/secrets_coordinator.py, leave the docstring (owner's mutation, #16982 2026-09-29)['secrets_coordinator.py'], new finder RED[]—test_the_access_check_is_wired_into_production_or_absentfailedreturn Trueabove the call, call text left in placelogger.debugtest_a_debounced_edit_says_it_was_coalesced_and_not_refused,test_every_return_in_handle_change_logs_firstwarning→debugtest_a_failed_handoff_is_a_warning_naming_the_file_handle_changefloating_packagesto the per-requirement cap testtest_a_cap_stated_by_a_floating_requirer_is_not_structural,test_all_four_are_reported_once_the_pin_is_reverted,test_floating_is_transitivefloating_packagesreturns every closure packagetest_the_tree_currently_has_no_unbaselined_pairrequirements-ci/storage.txtfor realtest_the_fix_being_reverted_is_actually_in_the_treePer-criterion evidence
#17822 — finding 1 (finding 2 was already delivered in #17772 and is untouched):
_references_symbolwalks AST nodes rather than matching text." —repo_tests/secret_access_check_is_wired_16982_test.py,_references_symbolwalksast.Name.id,ast.Attribute.attr,ast.alias.name/.asnameand_forwards_by_name. Noin sourceremains in the finder.test_prose_alone_is_not_a_reference(4 cases: the owner's mutation shape, a comment, a function docstring, a bare string literal) andtest_real_code_is_a_reference(5 cases: attribute call, bare name, import-only, aliased import,getattr).test_the_pair_differs_only_in_prose_versus_codeasserts both fixtures contain the literal text, so a raw-text finder cannot satisfy it.#17547 — AC3 (AC1/AC2/AC4 were already delivered and are re-confirmed below):
on_modifiedis unconditional,kb_folder_events.py:51-68; pinned bytest_a_modification_to_a_file_never_seen_before_is_dispatched.:139-141, written only at:169.loggercalls, one per early return;test_every_return_in_handle_change_logs_firstasserts 4 bare returns and 4 logging guards, so the count cannot drift.#17558 — Gap 1 (verified, not re-done) and Gap 2:
audit()returns how many declared packages were not installed, andrender()states it on both the pass and the fail path." —FloorAudit.not_installed/.not_compared_declarationsinpipeline-scripts/dependency_floor_report.py;uncomparedis interpolated into both the all-satisfied block and the below-floor block.{result.compared} of {result.declared}, andcomparedcounts only names present with readable metadata.test_a_clean_pass_counts_comparisons_and_admits_what_it_skipped(128 declared / 86 compared / 42 absent) andtest_clean_environment_says_how_many_were_checked(206/206).--alllists the not-compared names, as it does shortfalls." —_print_reportinpipeline-scripts/check_dependency_floors.pyprints "Declared but not installed, therefore not compared" and each name undershow_all.tools/lint/check_ci_plane_unenforced_pins.pyis its own module with its ownUnenforcedPairfinding.test_all_four_are_reported_once_the_pin_is_revertedasserts exactly the four;test_the_fix_being_reverted_is_actually_in_the_treeis the non-vacuity check that the thing being reverted is still present.BASELINEplusTestTheBaselineOnlyShrinks, both directions.UnenforcedPair.describe();test_the_report_names_which_plane_declares_and_which_merely_installs.Model Used
Claude Opus 5 (1M context).
Not delivered here, and why
tools/lint/check_ci_plane_unenforced_pins.pyhas no--auditrun step in.github/workflows/code-quality.yml— 17 othercheck_*.py --auditsteps arethere, and this checker's only mention in that file is a comment explaining why
packagingis installed for it. It is reached today only throughrepo_tests/ci_plane_unenforced_pins_17558_test.pyunderpython-suite.That file is owned by an open PR in this queue, so it is left alone rather than
worked around. It is not an acceptance criterion of #17558 — both gaps' criteria
are satisfied above — and it is filed separately.
Closes #17822
Closes #17547
Closes #17558
Summary by CodeRabbit