fix: anchor logging.audit_file/log_file to repo root, not process cwd#484
Merged
Conversation
retrieval/indexer.py's _anchor_index_paths() already fixed this class of bug
for corpus.path/indexing.bm25_path/indexing.chroma_path; logging.audit_file
and logging.log_file were never in scope for that fix and stayed cwd-relative.
utils/logger.py's own _get_config() already anchors the *config.yaml file
itself* to _REPO_ROOT -- but not the path *values read from it*, so
setup_logging()'s log_file and audit_log()'s audit_file silently depended on
the process cwd. gate.py's /audit/summary endpoint re-reads the same raw
string with its own bare default ("audit.jsonl", diverging from config.yaml's
actual "logs/audit.jsonl") and had the identical gap.
Every existing fixture overrides audit_file/log_file with an absolute
tmp_path (test_gate.py's client fixture, etc.), so a relative-path cwd bug
passed CI clean -- new tests exercise the relative-path case explicitly via
monkeypatch.chdir + a patched _REPO_ROOT/_BASE_DIR.
Fix: utils/logger.py gets a small _anchor() helper (mirrors _get_config's
existing is_absolute() check) used by setup_logging/audit_log's 2 new call
sites; gate.py's audit_summary reuses its own existing _BASE_DIR idiom
(_BASE_DIR / audit_file, which pathlib resolves correctly whether audit_file
is relative or already absolute).
GROK_API_KEY=dummy pytest tests/ -q --tb=short -> full suite green (only expected CYCLAW_DB_URL-gated Postgres skips). ruff clean. invariant-guard: 27 passed, 0 failed.
…ing) windows-latest CI caught it: ntpath.expanduser() (verified against CPython's ntpath.py source) checks USERPROFILE first, falling back to HOMEDRIVE+HOMEPATH -- it never reads HOME at all. posixpath.expanduser() reads HOME. The test only set HOME, so on windows-latest it silently no-op'd and ~ expanded to the real runner profile (C:/Users/runneradmin) instead of tmp_path. Set both env vars so the test is meaningful on every CI leg, not just ubuntu. _anchor()'s own production code was never wrong -- it just correctly delegates to Path.expanduser(), which is platform-correct by construction. This was a test bug, not a logic bug.
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
2 participants
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.
What
utils/logger.pygets a small_anchor()helper (mirrors_get_config's existingis_absolute()check in the same file) used bysetup_logging()'slog_fileandaudit_log()'saudit_file.gate.py's/audit/summaryendpoint gets the equivalent one-line fix using its own existing_BASE_DIRidiom.Why / benefit
retrieval/indexer.py::_anchor_index_paths()already fixed this exact class of bug forcorpus.path/indexing.bm25_path/indexing.chroma_path— butlogging.audit_fileandlogging.log_filewere never in scope for that fix and stayed cwd-relative.utils/logger.py's own_get_config()already anchors the config.yaml file itself to_REPO_ROOT(soconfig.yamlis always found regardless of cwd) — but not the path values read from it, sosetup_logging()/audit_log()silently depended on the process cwd for where the log/audit files actually land.gate.py's/audit/summaryendpoint re-read the same raw string with its own bare default ("audit.jsonl", diverging fromconfig.yaml's actual"logs/audit.jsonl") and had the identical gap.This is the same fragility class
gate.py's_BASE_DIRcomment already documents ("When CyClaw is launched by double-clicking gate.py (Windows) the cwd is not guaranteed to be the repo root") — just an instance nobody had closed yet for these two config keys specifically.Every existing fixture happens to override
audit_file/log_filewith an absolutetmp_path(test_gate.py'sclientfixture,conftest.py, etc.), so this relative-path cwd bug passed CI clean with 100% existing test coverage green. The new tests exercise the relative-path case explicitly viamonkeypatch.chdir+ a patched_REPO_ROOT/_BASE_DIR, which is the only way to actually observe the bug.Risk to monitor
None identified —
gate.py's fix relies onPath.__truediv__discarding the left operand when the right is already absolute (verified interactively:Path('/base') / '/etc/foo' == Path('/etc/foo')), so it's correct for both relative and already-absoluteaudit_filevalues, matching every other_BASE_DIR / "..."usage already in the file.Shared-file note: this PR and the already-open #415 both touch
gate.py. Verified no real conflict — #415's three edits sit at lines 372/422/475 (retriever construction,check_input, themin_scorefallback); this PR's onlygate.pyedit is at line 625 (/audit/summary), 150+ lines away. A literalgit mergetrial hit a shallow-clone "unrelated histories" artifact in this sandbox, so I verified at the content level instead: #415's three target snippets are byte-identical to what its diff expects to find, completely untouched here.Verify
GROK_API_KEY=dummy pytest tests/ -q --tb=short→ full suite green (only the expectedCYCLAW_DB_URL-gated Postgres skips).ruff check --select E,F,I,B,C4,UP,Sclean on all touched files.python3 .claude/skills/invariant-guard/check_invariants.py→ 27 passed, 0 failed.Scan context
CyClaw-Optimize (ponytail + Karpathy + fable-protocol loaded). Finding #7 of an 8-finding scan; deduped against open PRs #473, #477, #415.
Generated by Claude Code