fix(config): one canonical project root, and no path that resolves to the wrong thing silently (#13149, #13357, #14113, #14892, #14070)

[mrveiss]

Thinking Path

Five issues, one defect class: a path or config key resolves to the wrong thing instead of failing.
Nobody sees an error; they see behaviour that is subtly off.

I started at #13357, because it explains the others' blast radius. What I found there changed the
shape of the work: the canonical resolver already exists and is already correct.
autobot_shared/paths.py::project_root() was landed by #14517/#14544/#14624, walks up in a single
pass with a checkout root as a hard boundary, treats .git with exists() so a worktree's .git
file matches, and raises ProjectRootUndeterminable rather than guessing. So the ask was not
"write a resolver" — it was find who still isn't using it, and prove the worktree case with a test
that cannot be faked.

That reframing drove four findings the issues did not predict:

1. The existing worktree test is fixture-shaped. TestWorktreeBoundary builds a directory that
   looks like a worktree and calls resolve_project_root(path_it_chose). It never calls
   project_root(), and .git is a file only because the fixture wrote one — the test supplies the
   answer it checks. I added a test that creates a real git worktree and runs the resolver in a
   subprocess whose __file__ is inside it.

2. The placeholder guard has a hole, and a live offender was sitting in it. The guard walks
   ast.Constant only. An f-string never holds the placeholder in any single Constant —
   f"${AUTOBOT_PROJECT_ROOT:-/opt/...}" parses as Constant('$') + FormattedValue(Name(...)) — so
   test_distributed_system_integration.py:703 carried one while the guard reported the file clean.
   The guard's docstring claims pyflakes F821 covers the f-string form; F821 only fires where pyflakes
   runs, and autobot-infrastructure/shared/tests/ is outside its scope.

3. Fixing get_data_path would have destroyed the secrets it was fixed to protect.
   secrets_store_migration._legacy_absolute_path deliberately delegates to the buggy resolver to
   find the old location, with a comment saying this keeps it correct "even if that resolver's
   behavior changes later". It does the opposite: once get_data_path returns the canonical
   directory, source == destination and migrate_legacy_secrets_store no-ops (:207). Every existing
   deployment's secrets.key/.json/.db would be orphaned and a fresh key minted over the top —
   exactly the "Docs improvements #1 risk" bug(config): PathsManager reads a config.yaml key that does not exist and silently falls back to a CWD-relative 'data' path #14113's migration note names. The formula is now pinned as a literal.

4. A bot commit is a change, not noise. An Auto-fix: code formatting commit landed on
   this branch mid-review. Reviewed line by line it was genuinely formatting-only — but it
   reflowed one Field(...) across three lines, pushing a grandfathered file over the very
   ceiling this PR had just recorded, and its isort pass trimmed a line from four others,
   leaving those stale in the opposite direction. Formatting commits are not behaviour-neutral
   with respect to guards that measure the file.

5. Four of bug(config): PATH.CONFIG_DIR names a directory that does not exist — nine consumers read a missing config tree #14892's consumers didn't just fail to read — they wrote a decoy. On a miss they
   mkdir(parents=True) and yaml.dump their built-in defaults into the path they had just failed to
   read. The next boot loads the decoy and reports success. A one-time miss became a permanent,
   self-confirming wrong answer, and the security policy in force was one nobody wrote or reviewed.
   Correcting CONFIG_DIR alone would have relocated that into the real tree for the two files that
   are absent at either spelling.

Measured populations

Derived here, not taken from the issues.

Population	Issue claimed	Measured now	Note
Python placeholder literals	60 in 40 files	1 live offender	59 swept by #14517. Raw grep showed 28 hits in 17 files before the fix and 29 after (the fix replaced the offending line with a comment explaining it); all are prose, docstrings, the 3 derived exemptions, or guard fixtures. The one live site was invisible to the guard (f-string). Audit now clean over 5,136 files
Shell scripts defaulting to the live install	34 remaining	0	38 scripts source scripts/lib/project_root.sh; the single surviving :-/opt literal is that helper's own deployed-install fallback, which is where it belongs
Competing Python root resolvers	—	2 → 1	Removed PathConstants.PROJECT_ROOT = Path(__file__).parent.parent
CONFIG_DIR / get_config_path consumers	nine	9 files (confirmed)	8 of 9 were silent on a miss; 4 wrote a decoy; only error_catalog.py was distinguishable
Legacy paths_manager accessors	—	6, all CWD-relative	Plus 6 except-path ultimate fallbacks, also CWD-relative
Non-test importers of the legacy resolver	—	5 + 1 deliberate	The deliberate one is the secrets migration

What Changed

One canonical resolver, everything routed through it. autobot_shared.paths.project_root()
is the single implementation; nothing new was added beside it. PathConstants.PROJECT_ROOT now takes
it (via ssot_config.PROJECT_ROOT) instead of Path(__file__).parent.parent, which was blind to
AUTOBOT_PROJECT_ROOT/AUTOBOT_BASE_DIR and resolved to the main checkout from a worktree.

Per-case decisions for missing keys and directories — each one stated, not defaulted:

Case	Decision	Evidence
PathsManager reads a paths: key	The key was wrong. Source the SSOT instead	No config.yaml that the backend's ConfigManager loads has ever had a top-level paths: key, so every call took the fallback — the fallback was the real behaviour
PathsManager relative fallback	Fail-safe, not fail-loud. Resolve absolute through ssot_config	Raising at these call sites would break boot for a value that now has a correct answer. The defect was CWD-dependence, and absolute resolution removes it
PATH.CONFIG_DIR names a missing dir	The key was wrong. autobot-infrastructure/shared/config	Verified present in a checkout and in a deployed install — one spelling serves both, so #14892's "may not be the same spelling" trap does not apply here
5 security configs, 3 absent at either spelling	Fail loudly, never write. WARNING naming the searched path + a source attribute	Writing defaults back is what made the miss permanent and self-confirming
Legacy secrets location	Pin the literal. Only place a CWD-relative path is correct	It describes where files are, so they can be moved — not where they should go

Other changes: the placeholder guard now reassembles f-strings via ast.unparse; the audit-log
default has one definition (ssot_config.default_audit_log_file), late-bound so derivation is
provable; the Field-default guard is now AST-based and lives in tools/lint/check_field_defaults.py.

The guard's own literal is assembled from fragments. LIVE_INSTALL_PREFIX = "/opt/" + "autobot".
Inside ssot_config_test.py the surrounding Field( context happened to satisfy a skip in the
repository's hardcoded-value rule; standing alone as its own module it did not, so extracting the
guard made its own source trip detector 3. The sibling guard in this same PR already assembles its
banned text this way — this one now matches, plus a self-guard that replicates detector 3's
complete skip set and fails if either the guard or its tests re-acquires the literal. The self-guard
has its own can-it-fail test, because a self-guard that cannot fire is decoration.

Removals (the most-missed criteria — grepped explicitly):

• security_layer._AUDIT_LOG_FILE_DEFAULT — gone. git grep '_AUDIT_LOG_FILE_DEFAULT' -- '*.py' returns only prose.
• The text regex default(?:_factory)?\s*=\s*"[^"]*/opt/autobot[^"]*" — gone.
• _save_config in four security managers — its only caller was the decoy write. git grep '_save_config' -- 'autobot-backend/security/enterprise/' returns 0.

File-size ratchet, diffed against the base branch: 6 entries lowered, 0 raised, 1 removed,
0 added. security_layer_test.py came in under the 600-line limit entirely, so its grandfather
entry was deleted rather than lowered. Four of the six were lowered twice — once by this PR and
again after the formatter's isort pass trimmed a line from each. The formatter's reflow was
reverted at source rather than absorbed by raising a number — see Verification.

Verification

The worktree test, and its mutation evidence. Real git init + git worktree add, real
paths.py copied in, resolver run in a subprocess. Preconditions asserted (.git is a file, the
worktree has no .env, the main tree does), plus a control asserting the main checkout still
resolves to itself — without it, a resolver returning Path.cwd() would pass for the wrong reason.

Mutation to paths.py	Result
Two-pass walk (all ancestors for .env, then markers) — the pre-#13357 form	FAILED — resolved to .../main, expected .../main/.worktrees/issue-1. Control still passed
.git tested with is_dir() instead of exists()	FAILED (3 tests)
restored	git diff autobot_shared/paths.py empty; 28 passed

The Field-default guard, old vs new (grep -c MUTATION = 0 in both files afterwards):

Spelling	Old regex	New AST
double-quoted	CAUGHT	CAUGHT
single-quoted / lambda-body / lambda+single / f-string / concatenation / nested-call	missed (6)	CAUGHT (6)
comment mentioning the path	false positive	not fired
docstring mentioning the path	false positive	not fired

Injecting a real default_factory=lambda: "/opt/autobot/logs/audit.log" into ssot_config.py — the
exact form the regex missed — failed the live sweep, then restored.

Population floors are asserted before offender counts, and floored separately per sweep
(OFFENDING_SPELLINGS and BENIGN_SPELLINGS each get their own floor, so one populated tuple cannot
satisfy the other's). field_call_count("") == 0 < FIELD_CALL_FLOOR proves the floor can fail;
the real file reaches 582 against a floor of 200.

The bot commit's subject needs no squash — the gate exempts it by design. lint-conventions.sh
skips any commit whose author name or email contains [bot], with a comment explaining exactly why
(this repository's own auto-fix workflows push commits with no issue number, and without the
exemption every PR touching a generated file would fail a required check on a commit no human wrote).
Replaying the gate's own logic over the PR range: 6 commits PASS, 1 BOT-EXEMPT, 0 fail.
Squashing it would have required a force-push for no gain.

Post-review re-verification, against the post-format tree.

The formatter's reflow was undone at source, not absorbed: # noqa: PLW0108,E501 was removed
(nothing in this repository runs ruff — Black, isort, autoflake and flake8 only, so the code was
suppressing a rule no linter enforces), which brings the Field(...) call to 113 characters against
a 120 limit. black --check now reports the file unchanged, so it is stable rather than waiting
to be re-split, and ssot_config.py is back at its recorded 3297 with no ceiling raised.

File	Ceiling	Now	Direction
ssot_config.py	3297	3297	reflow reverted at source — not a raise
compliance_manager.py	911 → 910	910	lowered
security_policy_manager.py	957 → 955	955	lowered
sso_integration.py	917 → 916	916	lowered
threat_detection/engine.py	720 → 719	719	lowered

The re-measurement script asserts every change is a lowering before writing, so a raise cannot
be recorded by accident. python_file_size_ratchet_test.py: 31 passed.

Hardcoded-value rule. The failing finding was real and is fixed. Re-running detector 3's logic
over all changed files — with its own *_test.py exclusion and its complete skip set applied —
returns 0 findings across 15 non-test files. A second finding the CI run had not reached, in a
comment this PR itself rewrote in ssot_config.py, was cleaned in the same pass.

Formatters run proactively (black --target-version py310, isort, autoflake) over all 23
changed files: no further changes, so there is nothing left for a bot pass to land.

Mutation evidence re-confirmed after all of the above — the two-pass walk still fails
TestRealGitWorktree::test_resolves_to_the_worktree_not_the_main_checkout while the control passes;
restored to an empty diff, grep -c MUTATION = 0.

Test runs (only changed files — this machine is the CI runner):

• 203 passed, 3 skipped across ssot_config_test, paths_test, paths_install_root_test,
  security_layer_test, security_layer_audit_default_test, paths_manager_test,
  config_dir_resolves_14892_test, check_field_defaults_test,
  check_no_shell_placeholder_paths_test, python_file_size_ratchet_test
• 148 passed on the root-resolution consumer smoke set (error_catalog_resolution, config/,
  editable_install_layout, deployment_declares_project_root_14544, compliance_audit_path,
  source_paths)
• flake8 clean on every changed file; placeholder audit clean over 5,136 files
• The 3 skips need an optional dependency absent locally; their behaviour was verified directly
  (miss → builtin-defaults + nothing written to disk; hit → file; empty YAML → builtin-defaults)

Pre-existing failure, not from this branch: test_ci_import_smoke_paths_14252.py::test_every_extracted_program_parses
fails identically on unmodified Dev_new_gui at 14b7e6be5. Filed as #15039.

Per-issue criteria

#13149 — no canonical project-root resolution → Closes

Criterion	Status	Evidence
git grep 'AUTOBOT_PROJECT_ROOT:-' -- '*.py' returns 0	Met as reinterpreted — the literal command does not return 0	It returns 29 hits across 17 files. Every one is prose: a comment or docstring describing this defect, or one of 3 derived, re-proved exemptions. Reaching a literal zero would mean deleting the documentation of the bug and the migration that depends on the literal. The AST audit — clean over 5,136 files — is what the criterion is asking for, and is stated here as a substitution rather than ticked as if the grep passed
Shell scripts resolve from a shared helper; none embeds the live-install default	Met	38 scripts source the shared helper; 0 carry the literal. The single remaining occurrence is the helper's own deployed-install fallback
A gate rejects reintroduction	Met	check_no_shell_placeholder_paths.py, now covering the f-string form it was blind to
YAML occurrences audited per consumer, not bulk-edited	Met — audit below	Four named files, five occurrences, each traced to its consumer

The YAML audit the criterion asked for. It anticipated two outcomes — "expanded, fine" or
"unexpanded, broken". The answer is neither: nothing is broken, and most of it is never read.

Occurrence	Consumer	Verdict
logging.yml:15 filename:	none — the file is written by the log aggregator under --setup, read by nothing. No dictConfig/fileConfig loads it	INERT
promtail-config.yml:40 __path__	none — Ansible renders its own template to a different destination; this copy is never shipped	INERT
promtail-config.yml:50 __path__	none — same	INERT
compliance.yaml:15	n/a — it is inside a comment block, not a live value. The key it documents is absent, so the Python default resolves correctly	Not a live occurrence
threat_detection.yaml:28 model_storage_path	the file is loaded, but the key has zero Python readers. The engine reads only two ml_models.* keys; persistence uses a path computed in code	INERT

Three supporting facts, each verified rather than assumed:

1. No env-expanding YAML loader exists anywhere. git grep expandvars -- '*.py' returns two
   prose comments and no implementation — so any ${...} reaching a Python consumer would be a
   literal. That is what makes "which consumer?" the whole question.
2. The promtail unit passes no -config.expand-env=true. So the orphaned copy is not merely
   unused — if anything were ever pointed at it, both lines would break.
3. logging.yml's generator emits absolute paths. The checked-in placeholder cannot have come
   from the writer; it is a hand-edit to a generated artifact that the next --setup clobbers.

Why this closes #13149 rather than deferring it: the criterion asked to confirm per consumer
rather than bulk-editing, and that is done, per occurrence, with consumers named. The inertness is a
genuinely new finding — a config that reads as if it configures something and configures nothing —
and it is tracked as #15044 with the acting-on-it criteria, together with nine further
occurrences in Ansible playbooks that #13149's criterion did not name and which deserve the same
treatment. If you read "audited" as requiring those to be resolved here rather than tracked, this is
one label change from Refs and I will not argue it.

#13357 — PROJECT_ROOT escapes a worktree → Closes

Fix direction	Status	Evidence
Resolve from __file__, not a marker walk	Met	Single-pass walk from Path(__file__).resolve(), checkout root a hard boundary
Use a marker every tree has, handling .git as file or dir; fail loudly	Met	CHECKOUT_MARKERS uses exists(); ProjectRootUndeterminable replaces the guess
Add a test asserting PROJECT_ROOT resolves inside the tree containing the module	Met	TestRealGitWorktree — real worktree, real subprocess, mutation-proven. This is the criterion the pre-existing fixture test did not satisfy
Blast radius: PATH.* also escaped the worktree	Met (beyond ask)	PathConstants.PROJECT_ROOT now canonical

#14113 — PathsManager reads a key that does not exist → Closes

Criterion	Status	Evidence
get_data_path() and ssot_config.path.data_path agree, or one is removed	Met	test_both_resolvers_agree
Setting AUTOBOT_DATA_DIR demonstrably moves the secrets store	Met	test_data_dir_override_moves_the_resolved_path asserts get_data_path("secrets.db") lands under the override. Both secrets classes already resolve via config.path.data_path (#14081)
No resolver falls back to a CWD-relative default	Met	test_no_accessor_returns_a_cwd_relative_path sweeps all 6 accessors under monkeypatch.chdir, floor asserted first. All 6 except-path fallbacks converted too
Every get_data_path() caller audited and listed	Met	Table above; 5 non-test importers + the deliberate migration pin
Test: with AUTOBOT_DATA_DIR set, the secrets store path resolves under it	Met	As above
Migration note: existing stores must not be orphaned	Met	The delegation bug found and fixed — the single highest-risk item in this PR

#14892 — PATH.CONFIG_DIR names a directory that does not exist → Closes

Criterion	Status	Evidence
Establish what it should resolve to in a checkout and on a deployed host	Met	Same spelling for both, verified against a real install layout; asserted by test
Fix the constant, or replace with a searching resolver	Met	Constant corrected; test_config_dir_exists_in_a_checkout would have failed on the original value the day it was written
For each of the nine consumers, confirm a miss is distinguishable; where not, make it so	Met	8 of 9 were silent. Five now report config_source/config_searched_path and WARN naming the path; error_catalog already was. The 9th (service_discovery) needs a services.json schema that does not exist — filed as #15038
A test asserts the resolved directory exists in a checkout	Met	Plus 5 parametrised reachability cases with a separately-asserted floor

#14070 — audit-log default hand-copied; guard is text-based → Closes

Criterion	Status	Evidence
Exactly one definition; the other derives or is removed	Met	_AUDIT_LOG_FILE_DEFAULT removed; both sites call default_audit_log_file()
A test proves derivation, not coincidental equality	Met	TestAuditLogFileDerivesFromOneDefinition redirects the canonical formula and asserts both the SSOT field and security_layer._AUDIT_LOG_FILE move. Two hand-copied formulas cannot pass. Required making the factory late-bound — bound directly, the field would not have moved and "derivation" would have stayed unprovable
Guard catches a literal inside a default_factory=lambda: body	Met	Parametrised case + a real injected mutation into ssot_config.py
Guard does not fire on a comment or docstring	Met	Both cases parametrised; the old regex fired on both
Note: _resolve_audit_log_file's if not value branch is unreachable	Disagreed, with evidence	It is reachable — AUTOBOT_AUDIT_LOG_FILE="" makes the field falsy. Kept and covered by test_the_empty_env_fallback_also_derives_from_it, rather than removed as the issue suggested

Closes #13149, #13357, #14113, #14892, #14070
Refs #15038, #15039, #15044

Model Used

Claude Opus 5 (1M context)

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!