refactor(ci): consolidate duplicated, drifted and never-run CI guards (#14371, #13984, #12986, #14034)

[mrveiss]

Thinking Path

Five issues, one cluster: guards that are duplicated, drifted, or never run. Each was measured before being changed, and in three cases the measurement changed the plan.

#14034 — the shared hook lib's argv/git filter disagreement was already fixed in _common.sh, but the two tests covering that branch still asserted the OLD behaviour. They were red against the code they cover, on the base branch. A third test in the same file was red for the same reason since #14151. The Python suite is not a required check here, so a red test in it blocks nothing — which is how three accumulated in one file. Audited every other get_staged_files caller and every argv-vs-git branch in hooks/, tools/lint/ and pipeline-scripts/: one more instance, in pre-commit-no-module-singletons.

#12986 — .github/filters/ already held two canonical filter files loaded by dorny/paths-filter, so the pattern was established and the job was to extend it. GitHub has no include mechanism for on.*.paths triggers, so those copies cannot be removed — they get a guard instead, which turns a silent shrink into a red check.

#13984 — the issue counted four copies; there are five (enforce-precommit.yml has one too). The union is not a pick of one: each copy had rules the others lacked. The first version of the resolver collapsed the "explicit base" and "event payload base" cases into one, and check-pre-commit-hook-pr_test.py caught it — an explicitly supplied base that does not resolve was being silently replaced by a smaller scope. They are separate entry points now, with the difference documented on both.

#14371 — enumerated all three detectors' rules as sets first. Detector 1 was the only one scanning .sh/.yml/.yaml and the only one with the account rules; detector 2 (dormant, never invoked) was the only one with per-value SSOT suggestions, the generic model regex and the URL rule; detector 3 was the only one with magic numbers, roles, categories, DSNs, timeouts and the .get() call-argument shape. Detector 1's PORT_PATTERN was declared and never referenced by any scan function. Running the merged set over the tree found 1977 findings where the CI detector reported 0.

#14181 is NOT in this PR. See below.

What Changed

#14371 — union table, all three detectors

Measuring only decided which file was the cleanest base to build on. It never decided which capabilities live.

Rule	pipeline-scripts (CI)	autobot-infrastructure (dormant)	pre-commit hook	Canonical rule now covering it
VM IP, any fourth octet	yes (172.16.168.[0-9]+)	20-25 only	19-25 only	_hv_rule_ip — keeps the WIDE range
Per-IP SSOT config-key suggestion	no	yes	no	HV_SSOT_VM_IPS table
Infra ports in :port context	declared, never referenced	yes	yes	_hv_rule_port
Ports 8443 / 5432 / 9090	dead PORT_PATTERN only	no	no	HV_SSOT_PORTS — wired in
Per-port SSOT suggestion	no	yes	no	HV_SSOT_PORTS table
Explicit model-name table	no	2 models	8 models	HV_SSOT_MODELS — union of both
Generic model-tag regex	no	yes	no	HV_MODEL_GENERIC
Hardcoded URL rule	no	yes	no	_hv_rule_url
Account path /home/<user>	yes	no	no	_hv_rule_account
Account position (User=, chown, sudoers)	yes	no	no	_hv_rule_account
AutoBot filesystem paths	no	no	yes	_hv_rule_path
Database DSNs	no	no	yes	_hv_rule_dsn
Role strings	no	no	yes	_hv_rule_role
Category / search-mode strings	no	no	yes	_hv_rule_category
Timeout literals	no	no	yes	_hv_rule_timeout
Magic numbers 10 / 50 / 100 / 5	no	no	yes	_hv_rule_magic_number
offset=0 as a WARNING, not a violation	no	no	yes	_hv_rule_magic_number — severity preserved
.get("field", default) call-argument shape	no	no	yes	_hv_get_call_argument
Extensions .sh / .yml / .yaml	yes	no	no	HV_SCAN_EXTENSIONS
autobot-infrastructure/ in scan scope	yes	no	n/a	SCAN_DIRS
noqa suppression	yes	no	no	_HV_NOQA_RE
SVG path-data skip	no	yes	no	_hv_rule_ip
Array-slice / docker-map / OWASP-id skip	no	yes	yes	_hv_rule_port
JSON-schema-example / placeholder skip	no	yes	no	_hv_rule_port
Cost-table / discovery-list skip	no	yes	no	_hv_rule_model
Example-domain / W3C-namespace skip	no	yes	no	_hv_rule_url
Security-module file skips	no	yes	no	_hv_rule_url
JSDoc * continuation skip	no	no	yes	_HV_COMMENT_RE
TypeScript union-type skip	no	no	yes	_HV_UNION_TYPE_RE
--json / --report output modes	yes	yes	no	CI entry point

Zero rules dropped. scripts/lib/hardcoded_value_rules_test.py names each former detector and a rule only it had, and fails by name if the merge lost it.

The dormant fork is retired: nothing invoked it, and its only references were in HARDCODING_PREVENTION.md under a path prefix (autobot-autobot-infrastructure/) that never existed. Docs repointed.

pipeline-scripts/hardcoded_values_baseline.txt records the 1977 findings the merged set finds today, so it can be enforced without reddening ssot-coverage on every PR and blocking every local commit. It only ever shrinks; the count is part of the key, so a second occurrence of a known value is a new finding; every run prints how many it suppressed; and a new --audit-baseline CI step fails on entries that no longer match anything.

#13984 — one base-ref resolver

scripts/lib/git-scope.sh carries the union of all five copies: the merge-commit rule, the event-payload fallback, the origin/<base> branch-name fallback (only check-pre-commit-hook-pr.sh had it), the last-resort parent, the explicit-base rule, three-dot range semantics and range splitting (only lint-conventions.sh), the anti-no-op zero-files guard (only code-quality's conventions step), the deleted-path filter, the explicit --base validation (only verify-done.sh), and "an unresolvable ref is FATAL" throughout. All five call sites source it; no inline copy remains.

branch_state() goes from 113 lines to six named predicates plus a 22-line decision. Every function in verify-done.sh is now under 30 lines. Not one rule changed.

#12986 — one backend-Python path filter

.github/filters/backend-python-paths.yml defines the set once and composes each consumer's filter from it with a YAML alias. Five changes jobs load it by path; their inline copies are gone and their output key names are unchanged. The six on.*.paths triggers GitHub forces to stay literal are policed by pipeline-scripts/check_workflow_path_filters.py, which also asserts api-wiring.yml still has NO pull_request.paths (#12934) so that asymmetry cannot be tidied back into a deadlock.

Also recorded, not fixed here: verify-generated-types.yml watches neither backend tree's autobot_shared dependency, so it can self-skip on a change that alters both generated api.ts files. That is a behaviour change to a gate rather than a deduplication, so it is filed as #14885 and registered in the guard's NOT_CONSUMERS table with the reasoning, so it reads as considered rather than missed.

Coverage fix found on the way: api-wiring's wiring filter omitted autobot_shared/**/*.py while scripts/audit_api_wiring.py imports autobot_shared.api_routing.router_prefixes and autobot_shared.openapi_schema from that tree. The required api-wiring context self-skipped and reported green over an audit that never ran.

#14034 — the two invocation paths agree, and it is pinned

_common_test.py's argv tests now assert the fix instead of the bug, plus a new test that the git and argv branches return the same set for the same input — the property that was actually broken. pre-commit-no-module-singletons had the identical asymmetry in Python: its .py filter lived only on the git branch, so CI fed .ts/.vue/.yml paths to ast.parse. It survived only because check_file swallows SyntaxError — a silent mis-scan rather than a loud one.

Rebase onto d70a1f99e (#14877 conflict)

Base moved three commits and one file overlapped: #14877 (35f48ba92) converted autobot-infrastructure/shared/scripts/detect-hardcoded-values.sh's source lib/ssot-config.sh 2>/dev/null || true into the fail-loud abort, and this PR retires that file. Rebased (not merged) so the branch stays linear and verify-done.sh's own merge-content check can still judge it.

Resolved as a union of capabilities, not a survivor-pick:

• The abort idiom travels. Every source in scripts/lib/hardcoded-value-rules.sh and both entry points ends in an explicit failure block that exits non-zero, and an absent or empty baseline is fatal for the same reason.
• The ssot-config.sh dependency deliberately does not, and the reason is recorded in the library rather than left to be re-derived: ssot-config.sh set -a-loads the master .env and exports all of it, AUTOBOT_REDIS_PASSWORD included. The consolidated detector is a scanner whose entire output is matched source lines, printed to a public CI log and posted onto pull requests — pulling a live secret into that process is a disclosure risk it gains nothing from. Its SSOT tables are detection patterns, not runtime config it reads to work (which is why detector 2's tables were literal too), and PROJECT_ROOT now resolves from each entry point's own location, needing no .env and working on a deployed install with no checkout.
• Deriving live fleet addresses from ssot-config.sh so a renumbered fleet is still caught is a real improvement with its own secret-handling question — noted in the library, not bundled here.

Read the three guards #14877 added. My work adds no competing shell-source guard, so there was nothing to consolidate — but the read found a gap, filed as #14891: shell_lib_sources_resolve_test.py walks *.sh only, and all 16 hooks in autobot-infrastructure/shared/scripts/hooks/ are extensionless, so none is covered — including two live offenders (post-commit-doc-sync sources with no failure block; pre-commit-warn-untracked discards stderr on the decisive attempt). Not fixed here because widening the walk needs a design decision first: pre-commit-warn-untracked's documented contract is exit 0, which the guard's exit [1-9] rule cannot express. Two things were in scope and are done: this PR's hook now uses the PROJECT_ROOT spelling that guard can expand, so its site resolves rather than being silently skipped once the walk widens; and retiring the fork takes the guard's distinct-script count 56 → 55, its exact floor, recorded next to the floor so the next legitimate retirement is a deliberate edit rather than a mystery.

Review round: the baseline could be silently grown (HIGH) — fixed

The reviewer found a bypass of the gate this PR exists to build, and it was right. The baseline suppresses findings, so anything that can append to it can silence the detector. --audit-baseline only checked that no entry had been stranded; nothing checked that none had been added:

hardcode a fleet IP, and in the same change append 1|ssot|autobot-backend/new_file.py|<ip> to the baseline — hv_partition suppresses it as already-known, ssot-coverage reports ssot_violations=0, and the PR merges green over a finding the detector made correctly.

The count-in-key design already made a bump on an existing key cost something; a brand-new key was the more direct route and was undefended. My own comment stated the invariant exactly — "the baseline would grow silently while appearing to hold" — and asserted it nowhere. A rule living in a comment is the shape of defect this whole PR is about.

pipeline-scripts/check_baseline_no_growth.sh now diffs the baseline against the base ref and fails on any new key or increased count. Base resolution comes from scripts/lib/git-scope.sh (built in this same PR) — a hardcoded origin/main would be wrong in a repo whose PRs target Dev_new_gui. Fail-closed on an unresolvable base, an unreadable baseline on either side, or a parse failure. hv_baseline_growth reuses hv_parse_baseline_into rather than carrying a second parser — the guard needs two baselines in one process, and a private parser would be the exact fork this PR exists to stop; that refactor also adds parse validation the loader never had.

Mutation-proved in both directions (scratch repo, nothing committed):

Mutation	Result
append a fabricated new key (the bypass)	RED
bump an existing count	RED
remove an entry	GREEN — shrinking is allowed
decrease a count	GREEN — shrinking is allowed
corrupt a count (parse failure)	RED
baseline missing at HEAD	RED
unresolvable base ref	RED

Mutation also caught the guard's own untidiness: with totals computed before the verdict, a missing baseline exited out of awk under set -e and never reached the designed FATAL. Fail-closed either way, but a failure path nobody can reason about is not one to keep — the verdict is now settled first.

ssot-coverage.yml gains the blocking step and fetch-depth: 2, without which the merge-commit rule resolves nothing and the guard would red every run instead of comparing.

Both non-blocking review notes were taken, because this PR is where the secret-exposure thread lives:

• _hv_rule_dsn and _hv_rule_url emitted the matched literal verbatim, so postgresql://user:pass@host/db reached a tracked file and a public CI log. Demonstrated, not hypothetical — two such values were already in the generated baseline. One shared _hv_redact_userinfo now serves both (same defect, same reason); scheme and host survive so the finding stays actionable, and a path like https://h/a@b is untouched. Baseline regenerated.
• enforce-precommit.yml's git-scope calls use the explicit || exit 1 form rather than the runner's default shell flags, matching every other call site here.

Verification

Every guard was verified by MUTATION, not by re-running it. No mutation was ever committed; each was applied to a scratch copy outside the repo, or reverted before staging.

Guard	Mutation	Result
_common.sh argv filter	restore the #14034 bug	both new tests go red
no-module-singletons	revert _python_only(args) → args	both new tests go red (payload chosen so SyntaxError cannot mask it)
path-filter guard	drop autobot_shared from api-wiring.yml	red, names the file and the entry
path-filter guard	add one entry to the canonical set only	all six inline consumers go red
path-filter guard	reintroduce api-wiring pull_request.paths	red
path-filter guard	rename a declared workflow	red, "the table is stale"
git-scope.sh	delete the merge-commit rule	red
git-scope.sh	weaken the explicit-base rule	red
hardcoded-value baseline	second occurrence of a known value	reported as new, not suppressed
hardcoded-value baseline	remove the baseline file	FATAL, not a clean report
rule library	remove the library from a hermetic tree	FATAL, not a clean report

Suites, run locally on Python 3.10.12 / pytest 9.0.2 (this interpreter — the CI runner is 3.14):

• scripts/lib/git-scope_test.sh — 33 assertions, 0 failed (real throwaway repos, not stubbed git)
• scripts/lib/hardcoded_value_rules_test.py — 40 passed
• pipeline-scripts/check_workflow_path_filters_test.py — 14 passed (every one a mutation test)
• pipeline-scripts/detect-hardcoded-values_test.py — 9 passed
• autobot-infrastructure/shared/scripts/hooks/pre-commit-hardcoded-values_test.py — 60 passed
• lint_conventions_test.py, verify_done_test.py, check-pre-commit-hook-pr_test.py, shell_lib_test.py — 97 passed
• Whole affected tree (pipeline-scripts/, hooks/, scripts/, repo_tests/shell_lib_test.py) — 499 passed, 8 failed

Those 8 failures are pre-existing and not from this branch, proven rather than asserted: pre-commit-no-tag-pinned-action{,_test}.py and pre-commit-worktree-branch-guard{,_test}.py are byte-identical to origin/Dev_new_gui (compared by blob hash), and this branch touches none of them. The no-tag-pinned-action set fails with FileNotFoundError: 'command' — the harness invokes a shell builtin through subprocess.run. Filed as #14884 rather than dropped; it matters beyond the red, because one of the eight is the fail-closed test proving a git worktree list failure is not reported as a clean tree, and that property is unverified while it cannot run.

Performance is a correctness property here, and two rounds of it were found by measuring rather than by review:

• The first version expressed every predicate as printf … | grep, forking two processes per rule per line. It had not finished a tree scan after ten minutes. The per-line path now uses bash's own [[ =~ ]] with no subprocess — 1m36s over six trees.

• The staged-file path had no prefilter, and pre-commit run --all-files hands it every tracked file on a REQUIRED check: 200 files in 78s, about 50 minutes over the 7639 tracked in-scope files. A check that never reports is the same fail-open outcome the rules exist to catch. Both paths now apply the same filter — 78s → 16s on the sample — and that also removes an asymmetry between the two entry points, which is the larger reason: a tree scan that filtered lines while a staged scan did not would judge the same file differently depending on which one looked at it, exactly the bug(hooks): get_staged_files ignored its file-type filter in CI, and a parse crash was reported as a Redis violation #14034 shape. Proven to cost no coverage: the same sample gives byte-identical output before and after (43 findings both ways).

  Measured against the hook this replaces, on the same 200 files: the base-branch hook did not finish in 10 minutes (killed at the timeout) where the new one takes 16s. Its per-line path forked two processes per rule; this is a large improvement on a required check, not a regression.

End-to-end:

• detect-hardcoded-values.sh --json → status: pass, 0 new, 1977 baselined, 1m36s over six trees
• detect-hardcoded-values.sh --audit-baseline → every baseline entry still matches something
• check_workflow_path_filters.py → 6 inline path lists match the canonical set
• check_hook_exec_bits.py → no new hook entry missing its exec bit
• lint-conventions.sh --range origin/Dev_new_gui...HEAD → clean, 4 commits conform
• Exec bits verified with git ls-files -s and set with git update-index --chmod=+x — core.fileMode=false means chmod never reaches the index. Entry points 100755, sourced libraries 100644.
• No commit trailers.

actionlint is not installed locally and did not run here. #12986's acceptance criterion for it is verified by CI, not by me.

#14181 dropped from this PR — reason

Its remaining work cannot be delivered here, so it is not in the Closes list. Measured against the current base, most of it is already done: enforce-precommit.yml's || exit 0 is gone (a dedicated step now fails on hooks that could not RUN), and check_hook_exec_bits.py's _KNOWN_DORMANT has shrunk from sixteen entries to two. The two that remain are tools/lint/check_no_blocking_io_in_async.py (201 blocking-I/O calls, owned by epic #7442) and tools/lint/check_no_local_schemas.py (~1370 local Pydantic schemas). The acceptance criterion is that the baseline is EMPTY at the end, which requires clearing both backlogs — each a substantial piece of work in its own right, and one of them already has its own epic. Bundling either into a CI-guard consolidation PR would be partial delivery, and partial delivery never closes.

Model Used

Claude Opus 5 (1M context), model id claude-opus-5[1m].

Closes #14371, #13984, #12986, #14034

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

HIGH finding addressed in a9d3c9805. Agreed with it — the bypass was in the enforcement mechanism this PR exists to build, and my own comment in hardcoded-value-rules.sh stated the invariant while asserting it nowhere.

pipeline-scripts/check_baseline_no_growth.sh diffs the baseline against the base ref and fails on any new key or increased count. Base resolution via scripts/lib/git-scope.sh rather than a hardcoded ref. Fail-closed on an unresolvable base, an unreadable baseline on either side, or a parse failure.

hv_baseline_growth reuses hv_parse_baseline_into instead of carrying a second parser — the guard needs two baselines in one process, and a private parser would be the exact fork this PR exists to stop. That refactor also added parse validation the loader never had (non-numeric count, missing separator → FATAL).

Mutation-proved both directions, scratch repo, nothing committed:

Mutation	Result
append a fabricated new key (the bypass)	RED
bump an existing count	RED
remove an entry	GREEN (shrinking allowed)
decrease a count	GREEN (shrinking allowed)
corrupt a count	RED
baseline missing at HEAD	RED
unresolvable base	RED

Mutation also caught the guard's own untidiness: with totals computed before the verdict, a missing baseline exited out of awk under set -e and never reached the designed FATAL. Fail-closed either way, but that failure path is now the designed one.

ssot-coverage.yml gains the blocking step and fetch-depth: 2 — without it the merge-commit rule resolves nothing and the guard would red every run rather than compare.

Both non-blocking notes taken, since this PR is where the secret-exposure thread lives:

• DSN/URL literal exposure was not hypothetical: two postgresql://user:pass@host/db values were already in the generated baseline, a tracked file that also reaches public CI output. _hv_rule_url had the identical shape, so one shared _hv_redact_userinfo serves both rather than two redactors that could drift. Scheme and host survive so findings stay actionable; https://h/a@b is untouched. Baseline regenerated.
• enforce-precommit.yml now uses the explicit || exit 1 form at every git-scope call site.

Not merging — over to you. #14181 stays dropped.