test: stop the suite writing to the real ~/.pi/agent audit log - #21
Open
carstenlucke wants to merge 1 commit into
Open
test: stop the suite writing to the real ~/.pi/agent audit log#21carstenlucke wants to merge 1 commit into
carstenlucke wants to merge 1 commit into
Conversation
Tests that delegate with the default config (auditLog is on) resolve their log path through getAgentDir(), which falls back to the real ~/.pi/agent — so every run appended fixture rows (fake providers, images that never existed) to the developer's production vision-audit.log, the very log that answers "where did my image bytes actually go?". tests/setup.ts now redirects the agent dir to a throwaway temp dir, and every tests/*.test.ts imports it as its first statement. That way the redirect holds however the suite is started — `pnpm test`, a bare `tsx --test`, a single file, or a run from another working directory — rather than only through the npm script. A meta-test in delegate.test.ts fails if a test file omits the import; a guard test checks the redirect actually took effect. Details worth knowing: - an inherited PI_CODING_AGENT_DIR is honoured (CI, wrapper scripts) unless it resolves inside ~/.pi, which is exactly the pollution this prevents; the check is separator-aware, so a sibling like ~/.pi-sandbox is fine - the expanded path is written back, so the env var and getAgentDir() agree even when the dir was passed as ~/… - cleanup runs on SIGINT/SIGTERM/SIGHUP as well as exit, so Ctrl-C no longer leaks one temp dir per test file - integration.test.ts drops its own ad-hoc redirect and cleanup test in favour of the shared one Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Problem
Tests that delegate with the default config (
auditLogdefaults to on,auditLogPathunset) resolve their log path throughgetAgentDir(), which falls back to the real~/.pi/agent. Every run therefore appended fixture rows to the developer's productionvision-audit.log.Measured on a clean checkout before the fix:
After a handful of runs the log held 84 fixture rows naming providers the user never configured (
ollama/minimax-m3:cloud,openrouter/qwen3.5:cloud) for images that never existed (/tmp/vision-delegate-*/pixel.png), interleaved with the genuine ones.No network traffic occurs —
globalThis.fetchis mocked throughoutdelegate.test.ts— so nothing leaked. But the audit log is precisely the artefact that answers "where did my image bytes actually go?", and fixture noise makes it unusable for that.README.mdsells this log as a privacy feature; it should not be forgeable by a test run.tests/integration.test.tsalready guarded itself with an ad-hocPI_CODING_AGENT_DIRredirect. Nothing else did.Fix
tests/setup.tsredirects the agent dir to a throwaway temp dir, imported as the first statement of everytests/*.test.ts. Doing it per file rather than via a runner flag means the redirect holds however the suite is started —pnpm test, a baretsx --test, a single file, or a run from another working directory.Details worth knowing:
PI_CODING_AGENT_DIRis honoured (CI, wrapper scripts) unless it resolves inside~/.pi, which is exactly the pollution this prevents. The check is separator-aware, so a sibling such as~/.pi-sandboxstays valid.getAgentDir()agree even when the dir was passed as~/….SIGINT/SIGTERM/SIGHUPas well asexit, so Ctrl-C no longer leaks one temp dir per test file.integration.test.tsdrops its ad-hoc redirect in favour of the shared one.Two guards keep it from regressing silently: a meta-test fails if a test file omits the import, and a second test asserts the redirect actually took effect.
Verification
The guard fires exactly when it should.
tsc --noEmitclean.Scope
Test-only — no production code touched, hence the
test:prefix. The bug was never about what the suite executes, only about where it writes.tests/audit.test.tsneeded no behavioural change: it passes explicit paths and never callsgetAgentDir(). The audit-specific tests indelegate.test.ts(T60 ff.) setauditLogPaththemselves and are likewise untouched.Note
Independent of #20 — different cause, different scope, no shared commits. Either can merge first.