fix(B-0058): filter-gate-log test no longer pollutes production ethics-decision log#5128
Merged
AceHack merged 3 commits intoMay 26, 2026
Merged
Conversation
…s-decision log The integration test at filter_gate_log.test.ts:302 called main() directly with production args, causing every test run to write `skill:test-entry` entries to the real production log at tools/alignment/out/filter-gate-log.jsonl. Empirical anchor: 2026-05-25T22:52Z Lior test run left 2 polluting entries in the local checkout (file was never committed because it carried only test pollution; per B-0058 responsibility #3 it should carry only real ethics-gate decisions). Fix is minimal + additive: - filter_gate_log.ts: logFilePath() now honors FILTER_GATE_LOG_PATH env override (defaults preserved; no API break) - filter_gate_log.test.ts: the polluting test now uses mkdtempSync + env-var override + try/finally cleanup; reads back the written entry to verify recordEntry actually wrote it (stronger assertion than the old code = 0) Verified: 141/141 alignment tests pass; no production log file created in the worktree after the test run. Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
… test pollution Per-tick shard for the substrate landing in this PR. Documents: - What landed (PR #5128 + 2 file changes) - How the bug was found (Step-1 worldview refresh surfaced untracked log) - Why this counts as concrete decomposition (counter-reset condition #3) - State observations (rate-limit tier, sentinel, worktree state, proc counts) - Composes with relevant rules Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Prevents filter_gate_log tests from writing skill:test-entry records into the real production ethics-decision log by adding a configurable log-path override and updating the polluting CLI test to write to a temp location.
Changes:
- Add
FILTER_GATE_LOG_PATHenv override tologFilePath()(default remains the repo log path). - Update the
--recordCLI test to write to a temp log file and assert the entry is actually written.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/alignment/filter_gate_log.ts | Adds env-var override for log path (keeps default repo log). |
| tools/alignment/filter_gate_log.test.ts | Uses a temp directory + env override to keep tests from polluting the production log. |
Findings on PR #5128: - P2 (filter_gate_log.ts:108): comment said "tempdir" but FILTER_GATE_LOG_PATH is treated as a FILE path; setting it to a directory causes EISDIR at appendFileSync. Clarify comment ("full FILE path inside a tempdir") and trim whitespace so all-whitespace values fall back to default. - P1 (filter_gate_log.test.ts:323): unconditionally `delete process.env.FILTER_GATE_LOG_PATH` in finally clobbered any caller-set override. Save priorOverride, restore in finally (delete only when prior was undefined). Self-spotted lint failure (CI lint tick-shard relative-paths): - docs/hygiene-history/ticks/2026/05/26/0814Z.md used 5-up relative paths (`../../../../../.claude/rules/...`) which resolved to `docs/.claude/rules/...`. Shard is 6 levels deep from repo root; bump to 6-up (`../../../../../../`). Verified via realpath: `../../../../../../.claude/rules/<rule>.md` resolves to repo-root `.claude/rules/<rule>.md`. Note: PR's other failing non-required check (lint backlog ID uniqueness) is PRE-EXISTING on main — 4 duplicate-ID groups (B-0802, B-0803, etc.) caused by parallel B-0720 + iter-6 decomposition rows landing on overlapping IDs. NOT caused by this PR (no backlog files touched). Out of scope for this fix. Verified: 33/33 filter_gate_log tests still pass after both fixes. Co-Authored-By: Claude <noreply@anthropic.com>
6 tasks
AceHack
added a commit
that referenced
this pull request
May 26, 2026
… updates refs not files (1008Z empirical anchor caught phantom PR #5128 drift) (#5134) The failure mode: agent runs `git fetch origin main` (Step-1 refresh) which updates `refs/remotes/origin/main` but does NOT promote local HEAD; subsequent `Read`/`cat`/`grep` against working-tree paths read the LOCAL HEAD's files (stale-against-origin if local hasn't been ff-promoted). If the agent authors substrate at that point, it's against state that may already be resolved on origin/main N commits ahead — phantom "drift" findings that require retraction. Empirical anchor (this commit's authoring session): - 2026-05-26T10:08Z Otto-CLI cold-boot ran `git fetch origin main` (success) in the operator's primary checkout (local HEAD `2774fef5a`) - Read `tools/alignment/filter_gate_log.ts` + `.test.ts` via working tree — both files appeared unfixed despite PR #5128 having landed the fix 1h 46min earlier at 08:22Z - Local primary was 11 commits behind origin/main (`1641da6d2`) - Without `refresh-before-decide` catching the staleness, the next substrate landing would have been a public PR retracting against already-resolved state Three mitigation patterns named in the rule (pick by context): 1. Isolated worktree off `origin/main`: `git worktree add --detach <path> origin/main` (default for ticks; composes with agent-worktree-hygiene `--detach` discipline) 2. `git show origin/main:<path>` for ad-hoc single-file inspection without checkout 3. ff-promote local HEAD — ONLY when the checkout is the agent's own, never the operator's primary Substrate-inventory step performed per verify-existing-substrate- before-authoring (PR #5131 — only visible via `git show origin/main:` because local was stale): existing partial coverage at otto-channels-reference-card.md ID-allocation section names this for `find docs/backlog -name "B-*.md"` queries. This extension generalizes the principle to any working-tree file read post-fetch + lands it on the refresh-before-decide surface where it auto-loads at every cold-boot. Files: - .claude/rules/refresh-before-decide.md (+69 lines: new section "git fetch updates refs but NOT working-tree files (post-fetch read trap)" + 3-pattern mitigation + 2026-05-26T10:08Z anchor + 5 composes_with citations) - docs/hygiene-history/ticks/2026/05/26/1008Z.md (+151 lines: full tick trace including the catch, the verify-before-defer composition, the substrate-inventory step, visibility signal) Composes with: - refresh-before-decide.md (existing 28-line rule extended) - verify-existing-substrate-before-authoring.md (PR #5131) - otto-channels-reference-card.md (ID-allocation narrow precedent) - agent-worktree-hygiene-never-hold-main-never-step-on-operator-cleanup-on-pr-merge.md - refresh-world-model-poll-pr-gate.md (origin/main over FETCH_HEAD) - dep-pin-search-first-authority.md (sibling at version-pin scope) - codeql-no-source-on-docs-only-pr-is-broken-commit-canary.md (verify-before-defer composition 8th-or-9th anchor) - PR #5128 — the fix whose phantom-drift catch this tick prevented Co-authored-by: Lior <lior@zeta.dev> Co-authored-by: Claude <noreply@anthropic.com>
This was referenced May 26, 2026
This was referenced May 26, 2026
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.
Summary
tools/alignment/filter_gate_log.test.ts:302ranmain()with production args, so every test run wroteskill:test-entryentries to the real production log attools/alignment/out/filter-gate-log.jsonl.FILTER_GATE_LOG_PATH) forlogFilePath()and updates the polluting test to usemkdtempSync+ try/finally cleanup.What changed
tools/alignment/filter_gate_log.ts:logFilePath()now checksprocess.env.FILTER_GATE_LOG_PATHfirst; defaults to repo path. Additive; no API break.tools/alignment/filter_gate_log.test.ts: the previously-polluting--recordtest now creates a tempdir, sets the env override, asserts the entry was actually written (stronger than the oldexpect(code).toBe(0)check), and cleans up infinally.Test plan
bun test tools/alignment/filter_gate_log.test.ts— 33/33 passbun test tools/alignment/— 141/141 pass (no other tests affected)tools/alignment/out/filter-gate-log.jsonlis createdaudit_candidate_failures.ts(only other consumer oflogFilePath()) reads via the same function — env override works transparentlyls-tree HEAD~1 == HEAD == 61) percodeql-no-source-on-docs-only-pr-is-broken-commit-canary.md🤖 Generated with Claude Code