Skip to content

test: stop the suite writing to the real ~/.pi/agent audit log - #21

Open
carstenlucke wants to merge 1 commit into
getpipher:mainfrom
carstenlucke:fix/tests-pollute-real-audit-log
Open

test: stop the suite writing to the real ~/.pi/agent audit log#21
carstenlucke wants to merge 1 commit into
getpipher:mainfrom
carstenlucke:fix/tests-pollute-real-audit-log

Conversation

@carstenlucke

Copy link
Copy Markdown

Problem

Tests that delegate with the default config (auditLog defaults to on, auditLogPath unset) resolve their log path through getAgentDir(), which falls back to the real ~/.pi/agent. Every run therefore appended fixture rows to the developer's production vision-audit.log.

Measured on a clean checkout before the fix:

Audit-Log before test run:  174 lines
Audit-Log after test run:   188 lines
=> one `pnpm test` writes 14 entries into the real audit log

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.fetch is mocked throughout delegate.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.md sells this log as a privacy feature; it should not be forgeable by a test run.

tests/integration.test.ts already guarded itself with an ad-hoc PI_CODING_AGENT_DIR redirect. Nothing else did.

Fix

tests/setup.ts redirects the agent dir to a throwaway temp dir, imported as the first statement of every tests/*.test.ts. Doing it per file rather than via a runner flag means the redirect holds however the suite is started — pnpm test, a bare tsx --test, a single file, or a run from another working directory.

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 such as ~/.pi-sandbox stays valid.
  • 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 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

tests growth of the real audit log
with the redirect 354 pass 0
without it (counter-check) 353 pass, 1 fail 14

The guard fires exactly when it should. tsc --noEmit clean.

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.ts needed no behavioural change: it passes explicit paths and never calls getAgentDir(). The audit-specific tests in delegate.test.ts (T60 ff.) set auditLogPath themselves and are likewise untouched.

Note

Independent of #20 — different cause, different scope, no shared commits. Either can merge first.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant