Skip to content

test: remove machine- and time-dependence from five tests and personal data from a fixture - #1353

Merged
Juliusolsson05 merged 19 commits into
mainfrom
fix/machine-time-dependent-tests
Sep 27, 2026
Merged

Juliusolsson05 merged 19 commits into
mainfrom
fix/machine-time-dependent-tests

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Refs #1296. Items 1, 2, 4, 5, 8 and 9 are fixed here. Items 3 and 7 (goal-loop settle sleeps and weak negatives) move to #1341, which has the same root cause: persistence outliving afterEach. Its fix is a drain seam in GoalLoopService that touches GoalLoopService.test.ts, which #1337 is changing. Item 6 is declined (below). So this is Refs, not Fixes.

Changes

  1. prerequisites.firstRun.test.ts: machine-wide rows on either side are not compared, when they are the provider's own command.
    • binaryResolver scans /opt/homebrew/bin and /usr/local/bin whatever HOME and PATH say.
    • The test skipped rows the recording found machine-wide, but not rows the live probe finds there, so a Mac with Homebrew claude/codex/opencode failed.
    • A row either side found outside the simulated HOME and staged app root is a machine fact.
    • Review: the skipped live row must be the provider's own executable (basename(path) equals the registry's binary name). A resolver returning /opt/homebrew/bin/grok for opencode is now compared, and fails.
    • No production seam was added.
  2. githubCli.system.test.ts: dropped the "elapsed < timeout + 2 s" wall-clock assertion around a real gh spawn. The production timeout and the test's 20 s limit already catch a hang.
    • Review: a gh shim first on PATH makes the real runner's answer knowable (it prints a fixture token, so no real credential is read). A runner that never spawns gh now fails.
  3. BrowserPocketHost.renderer.test.tsx: waits for the observed restart (vi.waitFor; the first crash backs off 250 ms) instead of a fixed 400 ms sleep.
  4. SubAgentWatcher.test.ts: awaits watcher.refresh() instead of polling a 1.5 s wall-clock deadline. CoalescedRefresh runs another full pass for any request made during a pass, so the awaited promise covers a pass that started after the call.
  5. Electron harness secrets:
    • Both harnesses used a fixed os.tmpdir()/agent-code-harness-secrets-<pid> directory with a plaintext codec, and never removed it.
    • Each run now gets its own mkdtemp directory, removed on process exit (which covers the app.exit(1) failure paths) and before app.quit().
  6. testing/fixtures/agent-activity/runtime-states.json personal data (read in full, per q36).
    • Beyond the /Users/juliusolsson paths and private project names the issue lists, it held queued prompts, drafts, prompt suggestions, sub-agent task descriptions and streaming baselines, plus branch and worktree names of private repos.
    • Redaction policy (scripts/agent-activity-redaction-policy.ts, same-length throughout):
      • text fields and every branch are filled with x;
      • the home user becomes a placeholder;
      • each project other than the exact public agent-code gets its own placeholder, as does each segment of the rest of its path, keys included;
      • a key collision refuses the file rather than merge entries.
    • The committed file is the policy's output, byte for byte. It is regenerated from the pre-redaction corpus in git history (15e43abe^, blob d2653405). A process test reproduces it from the blob (skipped on a shallow checkout).
    • How the tracked fixture can change: only two ways.
      • --redact-from <file> --home-user <recorder>, which accepts ONLY that recorded corpus (checked by sha256).
      • A manual copy of a staged live extraction, after a person's key-by-key privacy audit.
    • Live extraction never writes the tracked fixture.
      • It takes no arguments; --out and anything else are refused.
      • The temp root is realpath'd and refused when it or any ancestor holds a .git entry. That is a filesystem walk, not a git query, so no git environment variable can switch it off. It is checked before any bundle is read.
      • The output is ONE new file, agent-activity-staging-<uuid>.json, opened with wx and mode 0600 under that root (scripts/agent-activity-staging.ts). It is written through its descriptor, never by path, so swapping or linking the path cannot redirect the bytes.
    • Guards on every write (they refuse, printing counts, never names):
      • every home in the source must be the recorder;
      • an ambiguous dash-encoded home refuses;
      • the output may name only the exact canonical placeholder home, and any other spelling refuses (lowercase, backslash, \/, %2F, ~name, a prefix of the placeholder).
    • Stated residual: the pattern-based pass can't recognise every identifier a new live corpus might hold (a project outside Development/, a bare { "user": … }). That is why a staged file needs a person's audit before any copy.
    • The row model reads none of the redacted text, so the agent-activity tests pass unchanged. Two row-model guards are also newly pinned: the terminal guard, and title over Goal.
    • Fleet fixtures: private project names are same-length placeholders. This PR does NOT claim full personal-text redaction across all three fixtures. Six fleet titles are the owner's own prompt fragments, kept verbatim; that is an owner choice (q73):
      • A keeps them;
      • B replaces them with synthetic stand-ins of the same shape. B is prepared as a separate commit on the local branch option-b/synthetic-fleet-titles, ready to push.
    • Owner decision: the original content is still in main's git history, and removing it means rewriting main.

Declined

  1. Electron harness budgets (electronHarness.ts, runtimeHarness.ts). These are bounded condition waits in the Electron journey tier. They fail fast rather than hang, and they are not part of the unit or system suites. Replacing them needs Electron-level events (frame teardown, binding publication) that the harness can't observe today. Widening them is forbidden anyway.

Verification (head afebaca5, which is d084251f merged with main)

  • Items 2, 4 and 5 pass 3/3 runs.
  • The first-run suite passes (4 passed, 1 skipped); its grok-for-opencode mutant fails it.
  • The gh never-spawns mutant fails the shim case.
  • Redaction and staging (testing/unit/agentActivityRedactionPolicy.test.ts, testing/unit/agentActivityStaging.test.ts, testing/system/fixtures-privacy/): 36/36 green.
  • Each of these turns a test red when removed or broken:
    • the source home check, the output guard, the recorded-corpus check, and key redaction;
    • the no-arguments rule and the .git walk (including under GIT_CEILING_DIRECTORIES and a bogus GIT_DIR);
    • writing through the descriptor (a by-path write follows a swapped symlink), and mode 0600.
  • Agent-activity passes 32/32. npx tsc -b is clean.
  • Not reproduced locally: item 1's original failure needs a Homebrew claude/codex/opencode. This Mac has only Homebrew grok.

Plan: docs/plans/2026-09-26-machine-time-dependent-tests.md.

🤖 Generated with Claude Code

Juliusolsson05 and others added 5 commits September 26, 2026 19:52
…ture (#1296)

Refs #1296

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…corpus

The fixture kept every field verbatim apart from the screen mirrors,
which committed the home path, private project names, queued prompts,
drafts, prompt suggestions, subagent task descriptions and streaming
baselines. The row model reads none of that text. Text fields are now
filled with x at their own length; the home user and non-Agent-Code
project names are same-length placeholders (keys included). The
extractor applies the same pass, so a regeneration stays redacted. The
original content is still in main's history (owner decision).

Refs #1296

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y the recording's

The resolver scans /opt/homebrew/bin and /usr/local/bin whatever HOME
and PATH say, so on a Mac with a Homebrew claude/codex/opencode the live
probe finds a tool the clean recording did not, and the comparison
failed for a reason unrelated to the code. A row either side found at a
machine-wide path outside the simulated HOME and app root is a fact
about that machine and is not compared.

Refs #1296

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tart, subagent watcher)

- githubCli.system: drop the "timeout + 2 s" elapsed assertion around a
  real gh spawn; the production timeout and the test's own limit already
  catch a hang, and a loaded machine could exceed it legitimately.
- BrowserPocketHost: wait for the observed restart (nextCrashDelay =
  250 ms) instead of a fixed 400 ms sleep.
- SubAgentWatcher: await refresh(), which resolves after a pass that
  started after the call, instead of polling a 1.5 s deadline.

Refs #1296

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Electron harnesses use a plaintext secrets codec (a journey must
never touch the real keychain) in a fixed os.tmpdir()/agent-code-
harness-secrets-<pid> directory that was never removed. Each run now
gets a mkdtemp directory, removed on exit (covering the app.exit(1)
failure paths) and before app.quit().

Refs #1296

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Juliusolsson05 and others added 2 commits September 26, 2026 21:44
…s (review of #1353)

- first-run: a live machine-wide row is skipped only when its executable
  is the provider's own command; a resolver returning /opt/homebrew/bin/grok
  for opencode now fails instead of being excused.
- githubCli: a PATH-first gh shim makes the real runner's answer knowable;
  a runner that never spawns gh now fails (the real-gh case accepts null).
- activityRow: a runtime that needs the user as claude must not as a
  terminal; the title wins the name when a Goal also exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…daction policy

Review of #1353 (q70): the committed runtime fixture was not the
extractor's output (hand-edited placeholders), the placeholder merged
same-length projects, and branch names plus worktree suffixes of private
repos survived. Also `startup` in live-fleet.

- scripts/agent-activity-redaction-policy.ts: one distinct same-length
  placeholder per project, rest of a private path x-filled, branch filled.
- extractor --redact-from: re-run only the privacy pass; the committed
  file is byte-identical to its output over 15e43ab^.
- testing/unit/agentActivityRedactionPolicy.test.ts pins the policy and
  that the committed file is a fixed point of it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Owner decision pending (steering q73): the two tracked fleet fixtures still carry verbatim agent titles that include owner prompt fragments. W4 kept them on purpose, because the titles are what the agent-activity finding is about. Options: (A) keep them verbatim, or (B) replace them with synthetic titles that preserve the meaning and adjust the tests. The manager recommends B, since this is a public repo. Until the owner answers, this PR does NOT claim complete personal-text redaction across all three fixtures. The reviewer still completes the new-head audit and the byte-regeneration check.

Juliusolsson05 and others added 11 commits September 26, 2026 22:11
…the exact public repo (q74)

- --redact-from refuses without an explicit, plain --home-user, checked
  before anything is read or written; both modes refuse output that still
  names a home directory other than the placeholder (count only, never
  the name). A missing flag used to fall back to the user RUNNING the
  script, leaving the recorder's path in the tracked fixture.
- The public-name exemption is the exact `agent-code` segment, not a
  prefix: `agent-code-private` (and sibling agent-code-<task> checkouts)
  are redacted.
- Regenerating exposed a data-loss bug: the x-filled tail made
  same-length paths under one project one object key. Each tail segment
  now gets its own placeholder, and a key collision refuses the file.
- Fail-first: the new unit and process-level system tests fail on
  2f93023's scripts. The fixture is regenerated (structure identical,
  every length preserved, no private names).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
assertNoForeignHome accepted any home segment the placeholder starts
with: for an 8-character recorder the placeholder is `fixture-`, so an
unredacted `/Users/fixture/...` passed and a wrong --home-user could write
it. Slash paths must now BE the placeholder; for Claude's dash-encoded
dirs every `-Users-` must be followed by the whole placeholder and a dash
(a user name can contain dashes, so ambiguity refuses).

Fail-first: the unit counterexample and a process-level wrong-user case
both fail on 42ce78e's guard. Also pins text-field rules inside arrays;
key redaction is killed by the same-length-keys test (reviewer c, round 2).
Regeneration unchanged (byte-identical).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pelling (q76/q78)

Round-3 privacy review of #1353 (a) and steering q78:
- a foreign user spelled like the placeholder (`/Users/fixture-`) passed
  the output guard; a new source check before redaction requires every
  home to be the named recorder;
- the dash encoding has no delimiter after the user, so
  `-Users-recorder-secret-...` passed as `recorder`; a dash home is now
  accepted only when a standard home folder follows the recorder AND the
  source names that folder canonically, else it refuses as ambiguous;
- lowercase, backslash, JSON-escaped, %2F and ~name home spellings pass
  through the pass untouched, so the output guard refuses them;
- a bare --redact-from (or an unknown flag) fell through to the LIVE
  extraction; it now exits before any read or write.

Fail-first: the four new process cases fail on 913f5e5. Regeneration
unchanged (byte-identical).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s stay refused (q78)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… final round)

Every privacy round found another input shape a pattern-based pass lets
through (latest, round-3 c: a private project outside Development/). This
mode exists to reproduce the audited committed file, so it now accepts
only the recorded pre-redaction corpus (15e43ab^, blob d265340, by
sha256), after the source home check and before any write. Refusals exit
2 with the curated message only.

A process test reproduces the committed fixture byte for byte from the
git blob (skipped on a shallow checkout). The live extraction's
remaining limits are stated in the plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Final privacy round a (a valid blocker, steering q79): a no-argument live
run wrote `/Users/fixture-home/Projects/secretproject/private-task` into
the tracked runtime-states.json, because the pattern-based pass only
rewrites projects under Development/.

- Live extraction requires --out <path>, refuses the tracked fixture and
  any path inside the repository that git would commit
  (temp/fixture-staging/ is ignored), validated before any bundle is read.
- The tracked fixture changes only through --redact-from (the pinned
  corpus) or a manual copy after a key-by-key audit; the header says so.
- Real process tests from the A reproduction: a no-argument run is
  refused and the tracked file is unchanged (red on 6b042b7); each live
  guard (argument, tracked path, git-ignored, source homes, output homes)
  turns a test red when removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y (q80)

Steering q80 (a scope narrowing, not another hardening round): an --out
staging path validated with git check-ignore was bypassed by a symlink at
or above it, which overwrote the tracked fixture. Live mode now takes no
arguments and writes only into a directory it just created with
mkdtemp(os.tmpdir(), "agent-activity-staging-") (flag wx), prints the
path and exits. liveOutputPath and the git logic are deleted.

The tracked fixture changes only through --redact-from (the pinned
corpus) or a manual copy after a key-by-key audit. Process tests: a live
run with a symlink planted at the old staging path leaves the tracked
file unchanged and stages into the fresh directory; --out is refused;
foreign-home bundles stage nothing. Mutations killed: staging at the old
fixed path, arguments not refused, the source check removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…q81)

os.tmpdir() follows $TMPDIR, so a TMPDIR pointing into a repository (or a
symlink into one) staged the unaudited live extraction inside the working
tree, where `git add -A` would commit it. Before any bundle is read, the
temp root is realpath'd and refused when `git rev-parse --show-toplevel`
succeeds there (or when git cannot run).

Process test: a private bundle with TMPDIR symlinked into a git repo
refuses and leaves no private bytes in the repo. Red on 0d9a02a and with
the guard removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (final check)

Final check of #1353 (a and c): the q81 guard asked `git rev-parse` and
read every non-zero exit as "outside a worktree", but git's discovery
can be switched off from the environment (GIT_CEILING_DIRECTORIES above
a subdirectory, a bogus GIT_DIR), and the unaudited file was staged
inside the repo. The temp root is now realpath'd and refused when it or
any ancestor holds a `.git` entry; no git process, nothing an env var
can change.

Process tests with both environments are red at 980a78b and with the
walk removed. Residuals stated in the plan: a concurrent same-user race
on the fresh directory (needs openat), and `wx` being defensive only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…by descriptor (q82)

Round 5 a (a valid blocker per the manager, q82): a concurrent process
could swap the fresh staging directory for a symlink between mkdtemp
and the by-path write, redirecting unaudited private bytes into a git
worktree. There is no staging directory now: writeStagedFile opens one
new agent-activity-staging-<uuid>.json under the checked temp root with
wx and 0600, writes through that handle and closes it; renaming or
replacing the path afterwards cannot move the bytes.

A unit test swaps the path for a symlink between the open and the write:
the victim stays untouched and the bytes are in the created file.
Mutations killed: write by path, mode 0644.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review disposition (final head afebaca5: d084251f merged with main)

Round 1 (a/b codex, c pi, at cb047747):

  • a major, first-run: a machine-wide row was excused even when it was the wrong executable. Fixed (cd893952): the executable's name must match the provider. The grok-for-opencode mutant is red.
  • b major, gh: the real-gh case passed when gh was never spawned. Fixed (cd893952): a PATH-first shim. The never-spawns mutant is red.
  • c majors (private project and branch names left; the fixture was not the script's output; placeholders merged same-length names). Fixed (2f930233): a policy module, one placeholder per identity, byte-identical regeneration. c's minors (terminal guard, title over Goal) are pinned.

Round 2 (2f930233): a, b and c were all MERGE-READY. c's key-redaction coverage note was pinned later.

Privacy hardening rounds (steering q74, q76, q78, q79, q80, q81, q82; a/c final checks):

  • --redact-from now:
    • requires --home-user;
    • reads homes from the source, not the output;
    • refuses every non-canonical home spelling and ambiguous dash homes;
    • accepts ONLY the recorded corpus (sha256).
  • Live mode:
    • never writes the tracked fixture and takes no arguments;
    • refuses a temp root that is inside a working tree (a .git walk, not a git query);
    • writes ONE wx 0600 file through its descriptor (q82, manager-verified by inspection).
  • Every step has a real-process or unit regression that was red on the previous head, and each guard's removal is a killed mutation.

Not done here / owner:

  • Fleet titles (q73): six titles are the owner's prompt fragments, kept verbatim.
    • A = keep them;
    • B = synthetic stand-ins, ready as a separate commit.
    • This PR does not claim full redaction of the fleet fixtures.
  • History: the pre-redaction content is still in main's git history. Rewriting it is the owner's call.
  • Stated residual: a staged live extraction needs a person's key-by-key audit before any copy into the tracked fixture. The pattern-based pass can't recognise every identifier a new corpus might hold.

@Juliusolsson05

Copy link
Copy Markdown
Owner Author

OWNER-APPROVED: the fleet fixtures KEEP the verbatim agent titles (q73 option A). The owner judges them innocent enough.
Owner, 2026-09-27T07:33:05Z: "just make these for me, just keep the fixtures in, they are innocent enough. do not delete stuff often" (manager applied it).

@Juliusolsson05
Juliusolsson05 merged commit e250840 into main Sep 27, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/machine-time-dependent-tests branch September 27, 2026 11:49
Juliusolsson05 added a commit that referenced this pull request Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

class:C9-tests Test hygiene sev:P2 Real bug with a workaround type:bug Something works wrong

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant