Repository navigation
Conversation
`hack project target` let the shared project input guards escape as detail-less errors. A project directory the caller cannot search, or an unparseable legacy adoption receipt, printed a detail-less E_CONFIG_INVALID envelope, which the MCP adapter turned into data: null. The inspector now maps those guard refusals wherever they are raised, including inside config reads: native or conflicting inputs become project_inputs_unsupported, an unsafe or uncheckable adoption state becomes the new provisional adoption_state_unsafe refusal, and an input access failure becomes input_unreadable. The adoption guard error now carries its kind so callers classify it without parsing its message. Paired CLI/MCP fixtures cover each case, and the MCP argv builder is pinned literally so a dropped selector is caught independently of the paired runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The evaluator computed a window mean with built-in sum(), which rounds after every addition and changed its algorithm in Python 3.12. Ten idle samples of 0.01 averaged to 0.009999999999999998 on the macOS system Python 3.9, so a mean exactly at the exclusive 1% idle ceiling passed there and failed on 3.12+. Use math.fsum so every interpreter judges the same correctly rounded mean, and add a regression test that fails with plain sum() on both 3.9 and 3.12+. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The protocol page says every number in a protocol or summary must be finite and within +/-(2^53 - 1), but the checker only bounded the fields it judged with _number. A count read with _count (p95_min_samples, expected) and any field the checker never reads accepted 2^53, 10^400, NaN or Infinity: a protocol with p95_min_samples = 2^53 validated, and a summary with expected = 10^400 was judged inconclusive instead of being refused. Scan every number in the whole document iteratively and add the closed codes protocol_number_out_of_range and summary_number_out_of_range, reported alongside any field-specific code. The page now names both codes and says the bound covers unread fields. Refs: HACK-1333, HACK-1169 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nspector The shared project input guard checks each candidate input with lstat and treats only ENOENT as absence. Any other fault escaped as a plain Error, so a `.dev` file, or a `.dev` directory the caller cannot search, beside a valid `.hack` made `hack project target` print a detail-less E_UNEXPECTED envelope (and the MCP adapter returned data: null). The guard now throws a typed, value-free ProjectInputAccessError that classifies the fault as permission (EACCES, EPERM) or unreadable without keeping the OS message or its path. The target inspector maps it to the closed input_permission_denied / input_unreadable refusals. Other commands now see E_CONFIG_INVALID with the same value-free message as the adoption guard's input-access refusal, instead of E_UNEXPECTED. Paired CLI/MCP fixtures and domain tests cover both cases; each was proven by mutation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rain late The foreground graph owner drained its child's stdout and stderr for only 250 ms after exit. Under heavy oversubscription the pipe readers could be scheduled after that window, so the closed native error code (or readiness line) was dropped and a generic startup failure was reported. Replace the window with the named, documented NATIVE_EXIT_DRAIN_BOUND_MS (5 s), still measured from the observed exit. EOF still ends a normal drain at once; the bound only ends a drain that a descendant holding the pipes keeps open. Tests: a writer that emits the code only after the stand-in is reaped is kept (fails with the 250 ms window), and a held descendant still settles within the bound while it is alive (fails with a widened drain). Refs HACK-1350 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every file in `bun test` shares one Bun process. A test that changed PATH, HOME, HACK_HOME or cwd and then failed before its restore leaked that change into every later file, which then failed with "Executable not found in $PATH: git/bun" and hid the first failure (HACK-1327). - Preload tests/helpers/global-state-guard-preload.ts from bunfig.toml. It snapshots the environment, cwd, umask, exit code, guarded global functions and process listeners around every test and at every file boundary, fails the leaking test or the run with a value-free message naming the file, and restores the environment, cwd, umask and exit code so later files run clean. Replaced functions and listeners are reported but kept, because they may still sandbox a timed-out test's unsettled work. - Fix leaks it and a review found: fzf replaced the process.env object; the VM-file, retained-build and retained-source-bind helpers could fail setup after changing PATH/HOME with no restore registered; the retained routing and retained-file fixtures kept HACK_HOME/PATH on the fixture when their lifetime was unknown; the protected-files negative left a process.kill spy for later files. Refs HACK-1327 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…chctl shim per test The M3 full suite at the first guard commit failed 16 shared-host-lifecycle cases (helpers reported as foreign listeners or never ready) and flagged tests/e2e-launchd-guard.test.ts for a process exit listener. - Root cause of the shared-host cascade: the engine-identity ambient-proxy test set HTTP(S)_PROXY and their lower-case forms, then restored them with a delete. Bun 1.4 copies assignments to proxy keys into the environment that fetch and Bun.spawn (without env) use, but never a delete, so every later loopback fetch in the process went through the dead proxy while process.env looked clean. restoreEnv now assigns "" before deleting. - The guard now also compares Bun's native environment at each file boundary (one /usr/bin/env child per file), names the file, and turns proxies off natively with NO_PROXY=* because Bun cannot take a deleted key back. Restores of deleted keys assign "" first as well. - The e2e harness's launchctl shim registered a process exit listener that lived for the whole Bun process; releaseLaunchctlGuard removes the shim and listener, and e2e-launchd-guard calls it after each test. Refs HACK-1327 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve bunfig.toml: keep both test preloads, this branch's global-state guard (HACK-1327) and next's checkout-write guard (HACK-1343, #390). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve LegacyComposeAdoptionSelectionError: keep this branch's public kind field, typed over next's message map (adoption, input-access, permission). target-inspect now maps next's new permission kind (#402, unsearchable .hack) to input_permission_denied instead of adoption_state_unsafe; the unsearchable-directory test expects it, and fails if the mapping is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HACK-1375. In a git checkout without a key, a failed root .gitignore write (read-only file, read-only mount, full disk) refused `hack env set --secret` but left `.hack.secret.key` behind. The retry the refusal suggested then found the key, skipped the ignore step and succeeded, so `git status` offered the key for commit. - Write the root .gitignore entry before a new checkout-local key; a failed entry write refuses with no key on disk. - Before `--secret` set or hydrate uses an existing checkout-local key that git does not ignore, add the entry; if that fails, or a later rule re-includes the key, refuse closed and encrypt nothing. Keys git already ignores, and checkouts outside git, cause no write. - `hack doctor` reports a present, un-ignored checkout-local key as `env-key-unignored` (warn) with a value-free remedy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ep every key Two concurrent `hack env set` commands with different keys on the same layer both exited 0 and one key was lost: each read the layer, changed it in memory and renamed a new file over it with no lock, so the last rename won (HACK-1376). Every env layer writer now holds a per-layer cooperative lock at `<layer dir>/.internal/<layer name>.lock` for its whole read, change and write: `hack env set`/`add`/`unset`, `hack env hydrate`, and the legacy `.hack/.env` set and remove. The lock reuses the projects-registry owner protocol (exclusive hard-link receipt, dead-owner recovery only on ESRCH), waits at most 10 seconds and then refuses closed, naming only the layer, with nothing written. set and unset also compare-and-swap against the bytes they read, so a writer that takes no lock (an editor, Hack v4) turns into a retryable AtomicWriteConflictError instead of a silent overwrite. A symlinked `.internal` is refused; a layer whose directory is not writable runs unlocked, since no writer can replace it. Refs HACK-1376 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Found 10 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
Fixes the cross-file state leaks tracked in HACK-1327. Only test infrastructure, test fixtures
and contributor guidance change. No product code changes.
## Root cause
`bun test` runs every file in one Bun process, so `process.env`, the working directory, the
umask, `process.exitCode`, global functions and process listeners are shared by all files. When
a test changed one of them and failed (or timed out) before its restore ran, every later file
inherited the change. With PATH pointing at a fixture directory, later files failed with
`Executable not found in $PATH: git/bun` (for example `tests/projects-registry-
admission.test.ts`, `tests/env-backend-command.test.ts`). Those cascades hid the first, real
failure.
Note: `Bun.spawn` without an `env` option uses Bun's own copy of the environment, taken at
startup. Code that passes `env: { ...process.env }`, and `node:child_process`, use the live
(leaked) PATH. That is why only some later files failed.
Sources found in this change:
- `tests/native-compose-adoption-file-generation.test.ts`: when the fixture lifetime was
unknown, `afterEach` threw before restoring PATH, so PATH stayed on the fixture root (no git,
no bun) for every later file.
- `tests/helpers/retained-routing-adoption.ts` (changed by #383) and its wrapper in
`tests/native-compose-adoption-routing-generation.test.ts`: with an unknown lifetime,
`cleanup` refused before restoring HACK_HOME, and the wrapper skipped `cleanup`, so HACK_HOME
stayed on the fixture home.
- `tests/helpers/retained-build-adoption.ts`, `tests/helpers/retained-source-bind-adoption.ts`:
PATH was changed before an awaited setup step; if that step threw, no fixture (and no cleanup)
reached the caller.
- `tests/helpers/native-compose-vm-files.ts`: PATH, HOME, HACK_HOME and the Docker keys were
deleted before the restore was registered, and one failing cleanup skipped the rest.
- `tests/fzf.test.ts`: replaced the `process.env` object with a plain copy, which detached it
from `Bun.env` and reset unrelated keys.
- `tests/native-config-protected-files-fixture.test.ts`: the intentional-unknown negative left a
pass-through `process.kill` spy installed for every later file.
- `tests/e2e/harness.ts` via `tests/e2e-launchd-guard.test.ts`: the launchctl shim registered a
process `exit` listener for the whole Bun process (found by the guard on the M3 full run).
- **Proxy keys that Bun keeps after a delete** (found by the M3 full run of the first commit: 16
`shared-host-lifecycle` failures, helpers reported as `foreign-listener` or never ready). The
engine-identity ambient-proxy test (`tests/native-compose-engine-identity.test.ts`) sets
`HTTP_PROXY`, `HTTPS_PROXY` and their lower-case forms, then restores them with `restoreEnv`,
which deleted them. Bun 1.4.2 copies assignments to the proxy keys (`HTTP_PROXY`,
`HTTPS_PROXY`, `NO_PROXY`, lower-case too) into the environment that `fetch` and `Bun.spawn`
without `env` use, but a delete never reaches that copy, and after a delete Bun ignores later
assignments to that key too. So every later `fetch` in the process, loopback included, went
through a dead proxy while `process.env` looked clean. Bisected on this Mac: `engine-identity`
+ `shared-host-lifecycle` alone gave 67 pass / 16 fail, with or without the guard preload.
Full runs on `next` passed only because `tests/fzf.test.ts` runs earlier and left
`process.env` as a plain copy, which Bun no longer reads: after it, proxy assignments never
reached `fetch` at all (checked with a standalone script). So the fzf fix above exposed this
leak, and on `next` the engine-identity ambient-proxy case has not really exercised a proxy in
full runs.
## Fix
- **New guard**, preloaded from `bunfig.toml`: `tests/helpers/global-state-guard-preload.ts`
with the logic in `tests/helpers/global-state-guard.ts`.
- It snapshots the full environment (and the `process.env` object identity), cwd, umask,
`process.exitCode`, guarded globals (`fetch`, timers, `Date`, `Bun.spawn`, `Bun.spawnSync`,
`Bun.which`, `process.exit`, `process.kill`, `process.platform`) and the listeners of
signal/exit/error process events.
- Per test: the preload `beforeEach` runs before a file's own hooks and the preload
`afterEach` after them, so a correct `beforeEach`/`afterEach` pair passes. A mismatch fails
that test with a message naming the file.
- Per file: a runtime plugin `onLoad` for test files is the file boundary (it runs after the
previous file's `afterAll` and before the next file's module code; it returns the source
unchanged). A mismatch is reported at once, naming the previous file, and the preload
`afterAll` fails the run with every leaking file. This catches module-scope and
`beforeAll`/`afterAll` leaks.
- Per file it also compares **Bun's native environment** (one short `/usr/bin/env -0` child
per file, about 3 ms) and reports `native env <KEY> changed` for a proxy key that
`process.env` no longer shows.
- On a mismatch it restores the environment (in place, on the original object), cwd, umask and
exit code, so later files run clean. Bun cannot take a deleted proxy key back, so for a native
mismatch the guard sets `NO_PROXY=*` in Bun's native environment (and leaves `process.env`
unchanged): later fetches go direct.
- Replaced functions and listeners are reported but **not** restored. A leak there usually
comes from a timed-out test whose work is still running, and the fake is what keeps that work
away from real processes, signals, the network and the Docker socket.
- Messages name keys and kinds only (`env PATH changed`, `cwd changed`, `native env HTTP_PROXY
changed`), never values.
- The env allowlist is empty; no exception was needed.
- **Leak fixes** for every source listed above. The fixtures keep their roots and spies when a
lifetime is unknown, but never keep the environment. This matches #388 (HACK-1299).
- `restoreEnv` (`tests/helpers/env.ts`) assigns `""` before deleting a key, so a proxy key
ends empty (no proxy) in Bun's copy instead of keeping the test's value. This fixes the
engine-identity source and every other `restoreEnv` caller.
- `releaseLaunchctlGuard()` in the e2e harness removes the shim and its `exit` listener;
`e2e-launchd-guard.test.ts` calls it after each test. The e2e runner still keeps one shim per
process.
- `.ai/skills/hack-repo-verify/SKILL.md`: explains the guard, says to fix the named file, and to
restore env keys with `restoreEnv`.
- Merged the current `next` (`616407a5`); no conflicts.
## Tests
- `tests/global-state-guard.test.ts`:
- The diff names added, changed and removed keys and a cwd change, without values; the native
diff names keys only and treats empty as unset.
- Restore puts back keys, the env object, cwd and exit code.
- Replaced functions and listeners are reported but kept.
- Two nested `bun test` runs in a temp dir (stub executable on a temp PATH, temp HOME/TMPDIR,
no proxy) compare a suite without the guard and with it. Without the guard, a file that sets
PATH and throws makes a later file fail with `Executable not found in $PATH`, and a file that
sets `HTTP_PROXY` and deletes it makes a later loopback `fetch` fail. With the guard, the run
names `a-leak-in-test.test.ts` (per test), `b-leak-in-hooks.test.ts` (per file, `beforeAll`
without restore) and `c-leak-native-proxy.test.ts` (`native env HTTP_PROXY changed`), and both
later checks pass.
- A child-process probe shows `restoreEnv` leaves no proxy in Bun's native environment, while
a bare delete does (it fails if a Bun upgrade fixes the delete, so the workaround can go).
- `tests/native-compose-adoption-routing-generation.test.ts`: a refused teardown still restores
HACK_HOME.
- `tests/e2e-launchd-guard.test.ts`: releasing the launchctl guard removes its shim and `exit`
listener.
### Mutation proofs (each applied and reverted)
| Mutation | Result |
| --- | --- |
| Preload never restores | nested "later files pass" case fails |
| Diff ignores env keys | diff case and nested case fail |
| Env object not reinstated | restore case fails |
| cwd not restored | restore case fails |
| Bindings not reported | function/listener case fails |
| Bindings restored too | function/listener case fails |
| File-boundary check removed | nested case fails |
| Native env check removed | nested case fails (no `native env` message; later loopback fetch refused) |
| `disableNativeProxies` is a no-op | nested case fails (later loopback fetch refused) |
| `restoreEnv` deletes without the `""` assignment | guard names `native-compose-engine-identity.test.ts` (`native env HTTP_PROXY/HTTPS_PROXY/http_proxy/https_proxy changed`); with the guard's repair `shared-host-lifecycle` still passes 83/83 |
| `releaseLaunchctlGuard` keeps the `exit` listener | guard fails each `e2e-launchd-guard` test with `process 'exit' listeners changed`; the new release case fails |
| Routing `cleanup` restores HACK_HOME only after the lifetime check | refused-teardown case fails |
| `releaseEnvironment` is a no-op | refused-teardown case fails |
| Old fzf `process.env = {...}` | guard fails 9 fzf cases with "process.env object replaced" |
The required injection (3) is kept as the nested regression tests above, so no injected test
file remains in the tree.
## Evidence
- Bun 1.4.2 (`mise x bun@1.4.2`).
- Local (macOS, this Mac) at the pushed head: `bun run --cwd packages/cli typecheck` 0 errors;
root `bun run check` passes (privacy-check ok; only existing warnings, none in changed files);
gitleaks 8.30.1 over `origin/next..HEAD`: no leaks.
- Local focused runs at the pushed head: guard, launchd-guard, fzf, cli-spawn-env-isolation,
e2e-scenario-selection, protected-files fixture: 73/73. `native-compose-engine-identity` +
`shared-host-lifecycle`: 83/83 (before the fix: 67 pass, 16 fail). Routing-generation + file-
generation: 47/47.
- M3 full suite at the first commit (`58ed88dd`): 7,691 pass, 21 fail. The guard named
`tests/e2e-launchd-guard.test.ts` (exit listener; fixed). 16 `shared-host-lifecycle` failures
(proxy root cause above; fixed). 1 `repository catalog` failure and 2 `new no-host` failures
also fail in every other M3 full run on `next` (unrelated; see findings).
- Hosted CI at `8b207083`: all 16 checks pass, including `test` and the four docker-e2e shards.
- M3 full suite at the pushed head: queued, not yet run. The shared M3 lock is held by the disk
guard because `/` has 57 GiB free (floor 58 GiB). This PR will be updated with the result.
## Gaps
- The guard restores the baseline environment under a timed-out test's unsettled work. If that
work later spawns `docker` or resolves `HACK_HOME`, it sees the real PATH and (when unset) the
default `~/.hack`. Before this change it kept the fixture values and broke later files
instead. The suite has no run-wide sandbox for HACK_HOME, DOCKER_HOST or DOCKER_CONFIG.
- After a native proxy leak, children spawned without `env` still inherit the stale proxy key
(next to `NO_PROXY=*`). The run fails anyway and names the file.
- `mock.module` and `spyOn` on module exports (and on `Date.now`) are process-global in Bun and
are not tracked: the guard cannot see them. The retained-routing fixture deliberately keeps
its module spies when its lifetime is unknown.
- The file-boundary check depends on Bun calling a runtime plugin `onLoad` for each test file in
order (verified on Bun 1.4.2). With `--parallel`, each worker gets its own preload. The native
check needs `/usr/bin/env`; without it, it is skipped.
## Round G68 (HACK-1392): SIGINT listener leak found by the guard on `next` + this PR
Merge gate b48 (#445) failed twice on `next` + this PR with four deterministic failures,
reproduced locally in the same order (`bun test tests/loki-short-lived-logs.test.ts
tests/native-authored-project-oneoff.test.ts tests/global-state-guard.test.ts`): the two `Loki
follow ...` tests from #417, `public cancellation after run completion returns130 without
rendering or replay`, and the guard's `(unnamed)` file summary, each with `process 'SIGINT'
listeners changed`.
### Root cause
One product leak, blamed on two files:
- `src/ui/loki-logs.ts` `lokiTail` registered `process.once("SIGINT", () => stop())` and never
removed it. A follow that ends because the socket closes (the normal path, and what both #417
tests do) left that `once` handler in the process. Each follow test added one more.
- `tests/native-authored-project-oneoff.test.ts` runs after the Loki file and its cancellation
test calls `process.emit("SIGINT")`. That fired the two stale Loki handlers, which removed
themselves, so the SIGINT listener list changed (`2 -> 0`) during the one-off test and the
guard named that test and file. `runAuthoredCommand` in `src/lib/native-authored-command.ts`
(the `process.on`/`process.off` pair around line 920-965) is symmetric on every path; the
other two registrations in that file (`logs`, line 98-141, and the startup wrapper, line
649-668) are too. The one-off file passes on its own with the guard active (50/50), and passes
after the Loki fix when run after the Loki file.
### Fix
- `src/ui/loki-logs.ts`: the SIGINT handler is a named function installed right before the tail
awaits its close and removed in a `finally`, so a socket close, an error close and a SIGINT
(where the `once` handler already removed itself and `off` is a no-op) all leave the listener
list as it was. Everything between `new WebSocket` and that await is synchronous, so the
registration point has no observable change.
- `tests/helpers/global-state-guard.ts`: the listener message now carries counts, `process
'SIGINT' listeners changed (2 -> 0)`, so a handler consumed by the named test (an earlier
file's leak) reads differently from one it added (`0 -> 1`). Counts only, no values. No
allowlist, skip or special case was added; the guard's own tests stay green.
### Tests
- `tests/loki-short-lived-logs.test.ts`:
- `Loki follow removes its SIGINT listener once the tail closes`: one listener while the tail
is open, the same list as before once the socket closes.
- `Loki follow stops on SIGINT and leaves no listener behind`: the follow's own handler
(called directly rather than `process.emit`, which would reach every listener in the shared
test process) closes the tail with exit 0 and the list is back to the baseline.
- `tests/global-state-guard.test.ts`: the listener change line includes the counts.
Mutation proofs (applied and reverted): with the `lokiTail` fix reverted, both new tests fail on
their own `expect` (and the guard names the file, `0 -> 1`, `1 -> 2`, ...); with the count
change reverted, the guard test fails (`Expected to contain: "process 'SIGUSR2' listeners
changed (0 -> 1)"`).
### Evidence
- Bun 1.4.2 (`mise x bun@1.4.2`), this Mac, at `9870275f`: the three files above 77/77 with the
preload active (before: 72 pass, 4 fail); `tests/log-backend.test.ts` 5/5; `bun run --cwd
packages/cli typecheck` 0 errors; root `bun run check` passes (privacy-check ok, 34 existing
warnings, none in changed files); `ultracite check` on the four changed files clean; gitleaks
8.30.1 (`gitleaks dir` over a `git log -p origin/next..HEAD` export): no leaks.
- Merged the current `next` (`a74eedbc`); no conflicts.
- Hosted CI at `9870275f`: see the PR checks.
- M3 full suite at `9870275f`: queued behind other units on the shared M3 lock; this PR will be
updated with the result.
### Gaps
- A test that calls `process.emit("SIGINT")` still reaches every SIGINT listener in the shared
process, so another file's leaked handler would again be charged to that test; the counts in
the guard message now make that readable, and the guard already names the leaking file at its
own boundary.
- No CLI surface, agent guidance, or pinned toolchain file changed.
Refs HACK-1327, HACK-1392
🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Integration gate for merge batch b48: origin/next + #396 (HACK-1350) + #377 (HACK-1333) + #379 (HACK-1332) + #405 (HACK-1327) + #433 (HACK-1375) + #435 (HACK-1376). Tree 439043e2. Closed after the batch squashes.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.