Repository navigation
fix(native): pin storage binding call sites, tag carrier state mismatches and restore owned listeners on timeout - #497
Merged
Conversation
…ches and restore owned listeners on timeout HACK-1455 follow-ups after #469, #464 and #465. - New in-process test drives the real Docker storage carrier on the synthetic daemon with one changed saved-binding read per call site, so each assertNativeComposeStorageBindingUnchanged call site, the owner creation refuseFrom and the helper's stage are pinned. - Carrier stopped-state mismatches report helper-policy-stability and the deferred-retirement binding check reports private-state, so neither reads storage-budget past the carrier deadline. - withOwnedProcessListeners restores its saved listeners on test timeout (onTestFinished plus the guard preload) and keeps once listeners once; the guard's file-boundary restore keeps them once too. restoreProcessListeners documents that removing a leftover signal handler drops its cleanup. - The refused-teardown routing case asserts it holds the fixture gate; the guard storage proof case uses STORE_CASE_BUDGET_MS. - runtime-core label review test covers the dependency-cache form. Refs HACK-1455 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
roodboi
marked this pull request as ready for review
October 24, 2026 21:50
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
… or non-ASCII task programs (HACK-1438, HACK-1461) (#490) Refs HACK-1438 ## Root cause The task-catalog denylist (`isInlineCommandArgv` in `src/lib/task-catalog-recipe.ts`) did not model the `git send-email` options that run a command. On `next` (1281184), every one of these was accepted: - `--to-cmd`, `--cc-cmd`, `--header-cmd` and `--sendmail-cmd`, in both the `--opt cmd` and `--opt=cmd` forms. - `--smtp-server=/abs/path`. send-email runs an absolute path given here as a sendmail program. A second reason it was open: `git send-email` is a Perl script that reads its options with Getopt::Long's defaults (`pass_through` only). Getopt::Long's grammar is wider than Git's parse- options. It accepts `--`, `-` or `+` before a long name, ignores letter case, and accepts any unique prefix. So even a rule written for `--to-cmd` would miss `-to-cmd=cmd`, `+to-cmd=cmd` and `--CC-CMD=cmd`. I confirmed each spelling against real Git 2.56 in a scratch repo. Each one ran a marker command: `--to-cmd X`, `--to-cmd=X`, `--cc-cmd=X`, `--header-cmd=X`, `--sendmail- cmd=X`, `--smtp-server=/abs`, `--cc-cm=X`, `--hea=X`, `--sendm=X`, `--to-cm=X`, `--CC-CMD=X`, `-cc-cmd=X`, `-to-cmd=X`, `-header-cmd X`, `-sendmail-cmd=X`, `+to-cmd=X`, `-smtp-server=/abs`, and `git Send-Email --cc-cmd=X`. The last one works because APFS is case-insensitive, so Git finds the `git-send-email` file for `Send-Email`. The `-c sendemail.ccCmd=…` / `toCmd` / `headerCmd` / `sendmailCmd` / `smtpServer` config forms also ran, including identity-scoped `sendemail.<id>.ccCmd`. They were already refused, because `git -c` and `--config-env` are refused whatever the key. This PR adds tests for them. ## Fix (`src/lib/task-catalog-recipe.ts`) - **New `subcommandGetoptLong` table** on the evaluator, plus a `getoptLongSelects` matcher. The matcher strips a `--`, `-` or `+` prefix, takes the name up to `=`, and lowercases it. It refuses the argument when the name is a prefix of an inline option. The one exception is a name that exactly matches another option (`to`, `cc`, `h`), because Getopt::Long takes an exact name as that option. `git send-email` gets `to-cmd`, `cc-cmd`, `header-cmd`, `sendmail- cmd` and `smtp-server`. - **Subcommand scoping.** The table is scoped to the `send-email` operand the same way `subcommandInline` is, so other commands are not refused (`git rev-list --header`, `git status -s`, `git format-patch --to=…`, `git log --cc`). The dashed form (`git-send-email`, `/usr/lib/git-core/git-send-email`) goes through the existing dashed-builtin mapping. - **Case-insensitive lookup.** Both subcommand-keyed tables now look up operands lowercased, because of the case-insensitive filesystem point above. - **Sweep additions**, each refused the same way and each confirmed against real Git where it is installed (see the sweep below): - `grep -O` / `--open-files-in-pager` (scoped to `grep`) - `daemon --access-hook` (scoped to `daemon`) - `instaweb -d` / `--httpd` (scoped to `instaweb`) - `svn --authors-prog` (scoped to `svn`) - the `remote-ext` word - `ext::` URLs (new `extUrls` flag) - the `archimport` word ## Tests (`tests/project-task-catalog.test.ts`) - **`HACK-1438: git send-email options that run a command`** (gap case). - Refused: every spelling listed above, plus the dashed, absolute-path, behind-`--git-dir` and mixed-case forms. Also refused: the `-c` config forms in any key case, identity-scoped and `smtpServer` keys, `--config-env`, and `-c sendemail.ccCmd=… -c alias.se=send-email se`, which only the `-c` rule catches. - Accepted: `--to=… --cc …`, `--suppress-cc=all`, `--sender=…`, `--no-header-cmd`, `--smtp- server-port=587`, `--no-to`, `-h`, and the same option names under other subcommands. - **`HACK-1438: Git porcelain options that run a command`** (gap case). Covers the sweep additions, with scoped accepted controls: `git grep -n`, `git grep --only-matching --or`, `git branch -d`, `git svn fetch --authors-file=…`, `git daemon --export-all`, `git instaweb --stop`, an `https://` remote and `git add extension.txt`. - **`HACK-1438: documented send-email and grep fail-closed collateral stays refused`.** Covers `send-email -s`, `--smtp`, `--smtp-server=mail`, `git grep -o`, and a path operand `Grep` scoping `-O`. ## Evidence - **Before and after.** I probed `isInlineCommandArgv` directly with 24 argvs that should be refused. On `next`, 22 of them were accepted; only the 2 `-c`/`--config-env` forms were refused. After the change, all 24 are refused, and the probed controls (`--to=…`, `--cc=…`, `git log --cc`, `git format-patch --to=…`) are still accepted. - **Mutation testing.** I applied 25 mutations one at a time, ran `-t HACK-1438` after each, and then restored the source. Every mutation fails at least one new test: | Mutation | Failing tests | | --- | --- | | Drop the Getopt::Long check | 2 | | Accept only the `--` prefix | 2 | | Drop the `+` prefix | 1 | | No abbreviations | 2 | | Case- sensitive option name | 1 | | Drop the exact list | 1 | | Case-sensitive subcommand lookup | 2 | | Unscoped (every `git` argv) | 1 | | Drop each of `to-cmd`, `cc-cmd`, `header-cmd`, `sendmail-cmd` | 1 each | | Drop `smtp-server` | 2 | | Drop `git -c` | 1 | | Drop the `grep` scope | 2 | | Drop `--open-files-in-pager` | 1 | | Drop `daemon` | 1 | | Drop `instaweb -d` | 1 | | Drop `instaweb --httpd` | 1 | | Drop `svn` | 1 | | Drop the `remote- ext` word | 1 | | Drop the `archimport` word | 1 | | Drop `ext::` URLs | 1 | | Case- sensitive `ext::` | 1 | | Unscoped `subcommandInline` | 1 | - **Focused tests.** `bun test tests/project-task-catalog.test.ts` reported 40 pass, 0 fail. It ran with a private HOME, HACK_HOME and DOCKER_CONFIG and `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`, and with `--timeout 60000`. The run with the default 5 s timeout hit 3 timeouts in slow existing domain tests (tracked pins, stale service definition, edit during read) while the host load average was about 106. None of the timeouts were in the changed tests. - **Gates.** Root `bun run check` passed, including `privacy-check: ok`, with only existing warnings and none in the changed files. `bunx biome check` on the changed files was clean. `bun run --cwd packages/cli typecheck` reported 0 errors. gitleaks 8.30.1 over `origin/next..HEAD` found no leaks. - **Not regenerated.** The CLI surface and consumer agent guidance did not change, so the CLI reference and plugin bundles were not regenerated. - **Full suite.** The hosted CI `test` job at this head. ## Git porcelain sweep Sources: `--git-completion-helper-all` for every command that `git --list- cmds=builtins,main,others,nohelpers` lists on Git 2.56, plus the script sources on `git/git` master (`git-send-email.perl`, `git-svn.perl`, `perl/Git/SVN.pm`, `git-instaweb.sh`, `git-p4.py`, `git-cvsimport.perl`, `git-cvsexportcommit.perl`, `git-archimport.perl`, `git- cvsserver.perl`, `git-web--browse.sh`, `git-mergetool.sh`, `git-difftool--helper.sh`, `git- submodule.sh`, `daemon.c`, `builtin/grep.c`, `imap-send.c`). Version history (2.19 and later) comes from the existing table and #463/#475. | Option or form | Runs | Status | | --- | --- | --- | | `send-email --to-cmd/--cc-cmd/--header- cmd/--sendmail-cmd` | shell command | **added** (Getopt::Long spellings) | | `send-email --smtp- server=/abs` | sendmail program | **added** (any value, fail-closed) | | `-c`/`--config-env` `sendemail.*Cmd`, `sendemail.smtpServer`, `core.sshCommand`, `core.pager`, `core.fsmonitor`, ... | config | already refused (`-c`/`--config-env` refused whatever the key) | | `grep -O<pager>` / `--open-files-in-pager=<pager>` | shell (`use_shell`) | **added**, scoped to `grep` | | `daemon --access-hook=<cmd>` | shell (`use_shell`) | **added**, scoped to `daemon` | | `instaweb -d/--httpd=<cmd>` | the daemon command line | **added**, scoped to `instaweb` | | `svn --authors-prog=<cmd>` | backticks in `Git::SVN` | **added**, scoped to `svn` (git-svn is not installed here; read from source) | | `git remote-ext <remote> <url>`, `ext::<cmd>` URLs | the URL as a command | **added**. Git's default `protocol.ext.allow=never` blocks it, but config can allow it. `EXT::` is also blocked by Git ("cannot handle remote-EXT as a builtin"). | | `git archimport <archive/branch>` | operand pasted into `tla abrowse … $limit \|` | **added**. `git archimport 'x/y;cmd #'` ran `cmd` even without tla. | | `--upload-pack`, `--receive-pack`, `--exec` (`clone`, `fetch`, `pull`, `ls-remote`, `push`, `archive`, `fetch-pack`, `send-pack`), `clone -u`, `rebase --exec/-x`, `difftool --extcmd/-x`, `submodule foreach`, `bisect run`, `filter-branch`, `rebase--helper --add-exec-commands` | shell | already modeled. `ls-remote` has no `-u` (its `--exec` is a hidden alias and is covered). | | `rebase--interactive --cmd` (2.20–2.33) | shell | in #475 (HACK-1423), not on `next` yet | | `column --command` | config lookup only | not a command | | `for-each-ref --shell`, `cat-file --batch-command` | quoting / stdin protocol | not a command | | `difftool/mergetool -t/--tool`, `web--browse -b`, `instaweb -b` | name must be a known tool or a configured `*.cmd` | config only | | `git hook run`, `clone/init --template` (template hooks run during clone), `--exec-path`, `merge/rebase/pull -s <name>` (`git-merge-<name>` on `PATH`), `transport::address` URLs (`git-remote-<transport>`) | a program file, not a string | not modeled (documented gap) | | `git config <key> <cmd>` writes (`core.pager`, `sendemail.ccCmd`, `include.path`, ...) | config read later | not modeled (existing documented gap) | | `cvsimport -d :ext:host` (runs `$CVS_RSH host cvs server`), `p4`, `cvsserver`, `cvsexportcommit`, `imap-send` (`imap.tunnel`) | env or config | not modeled. `cvsexportcommit` rev-parses its commit operands before it uses them in shell strings. | ## Gaps - **Repository aliases.** A repository alias still hides a subcommand from its scoped flags (`git se --cc-cmd cmd` with `alias.se = send-email`). This is the existing documented limit and is out of scope. - **Fail-closed collateral, documented.** - Any prefix of a refused send-email option is refused, even one Getopt::Long finds ambiguous and passes on to `format-patch` (`send-email -s`, `-t`, `--smtp`). - `--smtp-server` is refused whatever its value. - `git grep -o` is refused, because flags are case-folded; use `--only-matching`. - Subcommand names now match in any letter case at any operand, so a path named `Grep` scopes `-O`. The same applies to the existing `clone -u` and `rebase --add-exec-commands`. - **Interaction with #475 (HACK-1423, still a draft).** #475 changes `subcommandInline` to count only after the subcommand's operand. When it is rebased onto this change (or this onto it), it should apply the same after-the-operand rule to `subcommandGetoptLong`, and keep the lowercased lookup from `scopedEntries`. The two PRs touch the same lines in `evaluatesCode` and the docs paragraph, so expect a textual conflict. - **Not modeled.** Config writes and program-file runners (table above) are not modeled. ## Round 2: merged with `next`, one scoping rule (HACK-1423 + HACK-1438) The verifier found that this PR conflicted with `next` after #475 (HACK-1423) and #480 landed. The conflict was not only textual. #475 made subcommand-scoped Git flags count only **after** the operand that names the subcommand. This PR counted them at **any** operand. Its collateral test asserted that `git log -o -- Grep` is refused, which contradicts #475. **Rule chosen: #475's.** A scoped flag counts only after the operand that names its subcommand, because Git reads a subcommand's options only after the subcommand's name. This PR's lowercased subcommand lookup is kept, so `git Send-Email --cc-cmd=cmd` is still refused. What changed (merge commit 6cf5296, then a clean merge of 78487f7 in 0556dd9): - **`src/lib/task-catalog-recipe.ts`.** `scopedFlags` (from #475) and `scopedEntries` (from this PR) are now one helper, `scopedEntries({ table, positional, at })`. It returns the table entries for operands **before** argument `at`, and it looks up each operand lowercased. Both `subcommandInline` and `subcommandGetoptLong` use it, so the send-email Getopt::Long options follow the same after-the-subcommand rule as `clone -u`. #475's `--cmd` inline flag and its comments are kept. - **Tests.** The collateral test now asserts `git log -- Grep -o` is refused and `git log -o -- Grep` passes. New accepted controls where the flag comes before the subcommand word: `git rev- list --header send-email`, `git log -s -- Send-Email`, `git diff -Oorderfile -- grep`, `git push -d origin instaweb`. Every send-email spelling after `send-email` is still refused. New cases kill the two mutations that survived round 1: `git send-email --to=… -- p` and `git send-email --to=… -` pass (`--` and `-` name no option), `git push --repo=ext::cmd` is refused (`ext::` after `=`), and `git add docs/ext::notes.md` passes (`ext::` only at the start of an argument or after `=`). - **Docs (`docs/reference/ax-task-catalog.md`).** The contract table keeps both #475's rows and this PR's rows. The scoping row now lists `git rev-list --header send-email`, `git push -d origin instaweb` and `git log -o -- Grep` as accepted, and `git log -- Grep -o` as refused. The limitations paragraph uses the single after-the-subcommand rule, says the lookup ignores letter case, and drops the round-1 text ("matched at any operand ... a path named `Grep` scopes those flags to it"). The "Interaction with #475" gap and the "any operand" collateral bullet above are now resolved by this round. ### Evidence (round 2) - **Focused tests.** `bun test tests/project-task-catalog.test.ts`: 42 pass, 0 fail. Run with a private HOME, HACK_HOME and DOCKER_CONFIG and `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`. - **Mutation testing.** I applied each mutation alone, ran the focused file, and restored the source: | Mutation | Failing tests | | --- | --- | | Scoped flags count at any operand (drop the `at` check) | 5 | | Case-sensitive subcommand lookup | 2 | | `subcommandGetoptLong` counts at any operand | 1 | | `subcommandInline` counts at any operand | 4 | | Allow an empty Getopt::Long name (`--`, `-`) | 1 (survived round 1) | | `ext::` matched only at the start of an argument | 1 (survived round 1) | | `ext::` matched anywhere in an argument | 1 | | Drop `h` from the send-email exact list | 1 | | Getopt::Long without the one-dash prefix | 2 | | `index > at` instead of `index >= at` | 0, an equivalent mutation: the subcommand word itself cannot match a flag | - **Gates.** Root `bun run check` exit 0, with `privacy-check: ok`. `bun run --cwd packages/cli typecheck` exit 0, with 0 errors. gitleaks 8.30.1 `dir` over a `git log -p origin/next..HEAD` export (next = 78487f7): no leaks. The CLI surface and consumer agent guidance did not change, so nothing was regenerated. - **Full suite.** Hosted CI at 0556dd9, which contains `next` 78487f7. ## Round 3: Unicode case folding no longer bypasses the denylist (HACK-1461), merged with `next` Refs HACK-1461 The round-2 verifier found that APFS (and NTFS) look up names with Unicode case folding, but the denylist folded names with JavaScript's `toLowerCase()`. So `git ſend-email --to-cmd=cmd` (U+017F long s), `git-ſend-email`, `git inſtaweb --httpd=cmd`, `git ſvn clone --authors- prog=cmd` and `git filter-branch --tree-filter cmd` (U+FB01 fi ligature) were accepted. HACK-1461 records that two of them ran a marker command live on macOS. What changed (7483db3, on top of the merge of `next` cd35034 in 84d0379): - **`src/lib/task-catalog-recipe.ts`.** - A new `foldName` replaces `toLowerCase()` everywhere the denylist compares a name: program basenames (`programName`), Git helper porcelains (`gitSubcommand`), the subcommand-scoped tables (`scopedEntries`), the `words` check, first-operand `subcommands`, and Python REPL/trampoline modules (`isListedModule`). - `foldName` over-folds on purpose (fail-closed). It applies NFKC (`fi` becomes `fi`, `ſ` becomes `s`, the Kelvin sign becomes `K`), removes default-ignorable code points such as a zero-width space, then upper-cases before lower-casing (so `ß` becomes `ss` and dotless `ı` becomes `i`). - Option names keep ASCII-only folding. Git's parse-options and Getopt::Long compare options bytewise apart from ASCII letter case, and option values are left unfolded. - Two outright refusals in `evaluatesCode`, so the result does not depend on any one filesystem's folding table: - A listed program whose basename contains any non-ASCII character is refused whatever follows (`ſh scripts/x.sh`, `git-ſtatus`). - A Git subcommand (the first operand) that contains any non-ASCII character is refused (`git ſtatus`, `git --work-tree dir pack-refſ`). - Non-ASCII operands and values (`git commit -m 'café'`, `git log --author=Zoë`) and unlisted programs (`cat ſend-email`) still pass. - **Tests (`tests/project-task-catalog.test.ts`).** New test `HACK-1461: names that a case- insensitive filesystem folds to a listed name are refused`: - 18 refused spellings covering the long s, the fi ligature, the Kelvin sign (`Ksh -c`, `awK`), dotless i (`git ınstaweb`, `ıex`), folding after the first operand (`git log filter- branch`, `git log bıſect`, `git help ſend-email --to-cmd=…`, `git log filter-branch`, `python3 -m code`) and the non-ASCII refusals. - 8 accepted controls, plus the ASCII `filter-branch` and `send-email --to-cmd` refusals. - The collateral comment no longer claims a subcommand counts "at any operand". It now says the name is case-folded and scopes its flags from any operand that names it. - **Docs (`docs/reference/ax-task-catalog.md`).** - The refusal list describes the Unicode folding and the non-ASCII rule, and the contract table has a new row for it. - The limitations paragraph documents `help.autocorrect` next to the repository-alias limit. With autocorrect set to run the correction, `git send-emai --to-cmd=cmd` runs `send-email`, but the catalog sees an unknown subcommand and does not scope the flag to it. ### Evidence (round 3) - **Focused tests.** `bun test tests/project-task-catalog.test.ts`: 43 pass, 0 fail, run with `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`. The file's fixtures use their own private HOME, HACK_HOME and registry. - **Mutation testing.** I applied each mutation alone, then ran the HACK-1461 test (it failed every time) and a pure `isInlineCommandArgv` probe over the same 30 cases, then restored the source. | Mutation | Cases that flip to accepted | | --- | --- | | `foldName` is plain `toLowerCase()` (the pre-fix folding) | 7: `ıex`, `git log filter-branch`, `git log bıſect`, `git help ſend-email --to-cmd=id`, `git log filter-branch`, `python3 -m code`, `ſh scripts/x.sh` | | No non-ASCII listed-program refusal | 2: `ſh scripts/x.sh`, `git-ſtatus` | | No non-ASCII Git subcommand refusal | 2: `git ſtatus`, `git --work-tree dir pack-refſ` | | No upper-case step | 2: `ıex`, `git log bıſect` | | NFC instead of NFKC | 1: `python3 -m code` | | Default-ignorables kept | 1: `git log filter-branch` | | `words` and `scopedEntries` back on `toLowerCase()` | 4 | The Kelvin cases do not flip under the `toLowerCase()` mutation, because JavaScript already lowercases U+212A to `k`. They stay in the test as regression cases. - **Live macOS check (built CLI).** - Setup: I ran `bun run build`. In a scratch project with a private HOME, HACK_HOME and DOCKER_CONFIG, `.hack/tasks.json` held the HACK-1461 argv, each with a `touch <marker>` command. I then ran `./dist/hack project task list --json --path=<scratch>`. - At this head, every Unicode recipe (long s, dashed long s, fi ligature, dotless i, Kelvin) is refused as `inline_command`, as is the ASCII `send-email --to-cmd` control. The benign `lint` control gets past the inline check and is refused later as `input_path_invalid` (the scratch project is not a Git repository). - A CLI built the same way from the round-2 `task-catalog-recipe.ts` (0556dd9) let the long-s, dashed, fi and dotless-i recipes through the inline check; they too stopped only at `input_path_invalid`. - The marker file was never created. The catalog has no execution path. - **Gates.** - Root `bun run check`: exit 0, with `privacy-check: ok`. Its 34 warnings were there before this change, and none is in the changed files. - `bun run --cwd packages/cli typecheck`: 0 errors. - gitleaks 8.30.1 `dir` over a `git log -p origin/next..HEAD` export: no leaks. - The CLI surface and the consumer agent guidance did not change, so nothing was regenerated. - **Merge.** `next` cd35034 is merged in 84d0379 with no conflicts, so the branch is no longer behind. - **Full suite.** Hosted CI run 40134680598 (pull_request, headSha 7483db3) passed every check: the `test` aggregate and test shards 1/4 to 4/4, docker-e2e shards 1/4 to 4/4, secret-scan, Candidate core on Ubuntu and macOS, the Config compiler jobs, Runtime state models, runtime- images and linux-process-lifetime. While CI ran, `next` moved to 191382d (#489, #497). Neither touches this PR's files, and `git merge-tree` merges it cleanly, so GitHub shows BEHIND, not a conflict. ### Gaps (round 3) - I did not repeat the direct `git ſend-email --to-cmd` execution in this round, because the agent harness blocks a private-HOME `git` run outside its worktree. HACK-1461 already records that the bypass executes. This round shows that the catalog now refuses those argv. - `help.autocorrect` and repository aliases are documented limits. They are not detected, because the check does not read Git config. ### Round 4 (312de30): capital sharp s and non-ASCII programs The G109 verifier found a HIGH gap: `ẞh` (U+1E9E, capital sharp s) resolves to `/usr/bin/ssh` on the case-insensitive macOS filesystem, but `foldName` did not fold it, because JavaScript lowercases U+1E9E to `ß`, not `ss`. - **Folding.** `foldName` now runs NFKC, strips default-ignorables, then applies lower, upper, lower. The upper step maps `ß` to `SS`, so `ẞh` and `ßh` fold to `ssh`. - **argv[0] rule.** A recipe whose program (argv[0]) has any non-ASCII character in its file name is now refused, listed or not (`ẞh`, `/usr/bin/ẞh`, `./scripts/ünïcode.sh`, `café`). A case-insensitive filesystem may run such a name as an ASCII program that `foldName` does not predict, so the check fails closed. Reviewed scripts must use ASCII file names. Later arguments are still checked through `foldName` against the listed programs (`cat ẞh` is refused). - **Tests.** `tests/project-task-catalog.test.ts` adds `ẞh host id`, `/usr/bin/ẞh -o ProxyCommand=id x`, `cat ẞh`, `./scripts/ünïcode.sh` and `café --version` as refused cases. The accepted controls did not change. 43 pass. - **Mutations.** Restoring the old fold fails the `cat ẞh` case. Removing the argv[0] rule fails the HACK-1461 test. - **Docs.** `docs/reference/ax-task-catalog.md` documents the `ẞ` fold, the argv[0] refusal and the ASCII-name requirement. - **Gates.** Biome is clean on the changed files (complexity is back under the limit), `packages/cli` typecheck has 0 errors, `privacy:check` is ok, and gitleaks found no leaks in the commit patch. Hosted CI at 312de30 is the full-suite evidence. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
roodboi
added a commit
that referenced
this pull request
Oct 25, 2026
…ner veto and widen the recorded probe-budget case (#507) ## Summary Fix unit G111: the three HACK-1463 follow-ups from the #497 verifier. One of them was not a test problem. The carrier-state case failed under load because the storage transport refused healthy commands, so this PR includes one small product fix in `groupAbsent`. ## Root causes and fixes ### (1) Carrier-state call-site case "saw 9 inspects": a real refusal under load The count was a symptom. I ran the three carrier-state cases in a loop at load average 100-240 with diagnostics on every refusal path. About 1 run in 6 failed. Each time, the carrier had refused *before* the fault fired, at whichever recorded command was running (`storage-helper- create` or `storage-helper-start`), as `unclassified`. One failure stopped after inspection 5, another before inspection 1. The inspect count was simply wherever that refusal landed. The diagnostics traced the refusal to `groupAbsent` in `src/lib/native-compose-storage-witness- docker.ts`. A recorded command makes host identity reads (`/bin/ps`, `sysctl kern.bootsessionuuid`) through the unrecorded runner. After each read exits, the runner proves its process group is gone with `kill(-pgid, 0)`. Only `ESRCH` was accepted, and any other error refused at once, with no cause attached. On macOS, `kill(-pgid, 0)` can return `EPERM` just after the group's last member exits. A standalone probe (spawn a detached `ps`, await its exit, probe the group) saw `EPERM` once in 400 exits on this host. So a healthy read was refused, and the refusal surfaced as an `unclassified` carrier refusal at the enclosing stage. **Fix:** `EPERM` now means "not yet absent" (the group still has a member this process cannot signal). The wait continues to the same monotonic deadline: 3 s for an unrecorded command, or the 15 s settlement budget. Only `ESRCH` proves absence. If `EPERM` persists, the command still refuses at its deadline as `probe-budget`. Any other error still refuses at once. This follows the existing group-absence polls in `native-compose-file-command.ts` and `native-host-hook- runner.ts`. The carrier reference doc said "EPERM/EIO ... refuse". It now says that EPERM is not absence and is waited out to the same budget. **Test change:** the carrier-state cases now assert the reported refusal (stage and reason) *before* the inspection count. A refusal that happens before the fault then fails with its own stage and reason instead of a bare count. Only the fault reports `helper-policy-stability`, and it refuses at once. So once that refusal is confirmed, the count still pins which inspection reached the tagged call site, and the property the case proves is kept. New regression tests in `tests/native-compose-storage-witness-docker-io.test.ts` drive the real unrecorded runner with a `process.kill` spy that answers `EPERM` to the group probe: - A group that answers `EPERM` three times and then `ESRCH` completes (`exitCode 0`, exact stdout). - A group that always answers `EPERM` is never treated as absent and refuses as `probe-budget` at its 3 s budget. ### (2) Probe-budget recorded slow start: wall time is host work, not the 20 s delay The issue assumed the simulated 20 s start used 20 s of the 30 s Bun budget. It does not. The engine stubs `Date.now` and advances it on a virtual clock (`engine.elapsed`), so `SLOW_START_MS` costs no wall time. The wall time is the recorded branch's host work: three published commands, each with stopped-child `ps`/`sysctl` identity reads, private-state publications and settlement, plus `finish()` retirement. At load 120-140 that took more than 30 s. Shortening the simulated delay would not change it. **Fix:** the recorded case gets a named budget, `RECORDED_PROOF_CASE_BUDGET_MS = 120_000`, documented as a hang guard rather than a speed assertion. `SLOW_START_MS` stays at 20 s, so the "slower than the former 15 s cap" property and its mutation proof are unchanged. Biome re-wraps that one test body because its last argument is now an identifier. ### (3) Finalizer owner-confirmed refusal tagged The deferred finish's `if (!nativeComposeStorageCommandOwnerConfirmed(owner)) return refuse();` now calls `refuseObserved("storage-helper-cleanup", "private-state")`. The finalizer is registered only after all three commands settled. Only a running command can invalidate the owner, so no deadline can veto it at this point. A vetoed owner is the invocation's saved private state, reported at the carrier's own cleanup stage, as the binding check right after it is. `private-state` is outside `deadlineReasons`, so it never reads `storage-budget` past the deadline. Both reference docs say so. The new carrier case drives the real carrier against the synthetic daemon. It captures the real owner from `createNativeComposeStorageCommandOwner` and invalidates it with `invalidateNativeComposeStorageCommandOwner` after the invoke returns and before `finish()`. It asserts `{stage: storage-helper-cleanup, reason: private-state}`, a late reason of `private- state`, and refusal before the finish's binding read (48 reads, not 49) and before evidence retirement. ## Mutation proof Each mutant was applied to the source, then its focused test was run with `--timeout 60000`. The source was restored byte for byte afterwards (checked by comparing file contents). | Mutant | Killed by | Observed | |---|---|---| | Finalizer owner check back to `return refuse()` | new vetoed-owner carrier case | refusal `undefined`, late `storage-budget` | | `groupAbsent` refuses `EPERM` at once (the old code) | both EPERM io cases | first: refused; second: `unclassified` instead of `probe-budget` | | `groupAbsent` treats `EPERM` as absent | both EPERM io cases | first: answered 1 instead of 3; second: resolved instead of refusing | | Fixed 15 s cap restored for `command` probes | recorded exec/up slow-start case | proof refused | | Remove-spawned state check back to untagged `if (!same(...)) return refuse()` | carrier- state row "once its remove child is spawned" | `storage-helper-cleanup` / `unclassified`, late `storage-budget` | ## Evidence - Bun 1.4.2 (`mise x bun@1.4.2`), `bun install --frozen-lockfile`; no `bun.lock` change. - Root `bun run check`: privacy-check ok, 34 warnings, all of them complexity warnings that were already there (`invoke` is still 57). `bun run --cwd packages/cli typecheck` and root `bun run typecheck`: 0 errors. `ultracite check` on the three changed test files: clean. - gitleaks 8.30.1 (`gitleaks dir` over the `git log -p origin/next..HEAD` export): no leaks. - Focused files at the PR head, run serially with `--timeout 60000` at load average about 100: docker IO 15 pass, probe budget 6 pass, command record 34 pass, docker witness 90 pass, carrier call sites 18 pass (767 s). - Before the fix: the carrier-state cases failed 5 times in about 16 looped runs at load average 100-240 (shared host, some runs with an 18-24 worker CPU generator). Every failure was the pre-fault `unclassified` refusal described above. - **30 runs under load at the PR head c487960.** Each loop ran a `yes` CPU generator that a shell trap stopped on exit, and every run was a fresh `bun test --timeout 60000` process: - (1) the three carrier-state cases ("stopped helper carrier …"): **30/30 runs pass (90/90 cases)** with 18 generator workers at load average 70-126. Each run took 100-245 s. - (2) the recorded exec/up slow-start case: **30/30 pass** with 18 workers at load 44-72 (4-8 s per process), and **30/30 pass** with 120 workers at load 112-321. Under the heavier load each whole `bun test` process took 50-71 s, more than the former 30 s case budget. - Full suite: hosted CI run 40143860376 at the exact PR head c487960 is all green: `test` shards 1-4 and aggregate, docker-e2e shards 1-4 and aggregate, Candidate core (Linux, macOS), Config compiler (ubuntu-latest, ubuntu-24.04-arm, macos-latest, macos-15-intel), Runtime state models, linux-process-lifetime, runtime-images and secret-scan. ## Gaps - The `EPERM` root cause is shown by a standalone probe and by diagnostics on the refusal path. The diagnostics were temporary and are not committed. I did not establish exactly which macOS kernel state produces `EPERM`. The doc comment says only what was observed. - The io regression tests use a `process.kill` spy, so they prove the transport's handling of `EPERM`, not that the kernel produces it. - Other group-absence checks in the repository were not changed. Some already retry any non-`ESRCH` result (`native-compose-file-command.ts`, `native-host-hook-runner.ts`). `native- compose-vm-file-client.ts` and `native-compose-adoption-routing-execution.ts` treat a single `EPERM` as "present", which is safe but was not reviewed here for load behavior. - The carrier-state call-site cases still select inspections by order. The fixture's daemon cannot see which phase an inspect belongs to, so the cases depend on the documented inspection order, as before. ### Round 2 (ce5d368): EPERM case budget The verifier found that `"EPERM is never group absence or settlement"` in `tests/native-compose- storage-command-record.test.ts` now waits the full 15 s settlement budget before refusing (15.7 s locally, 15050 ms in hosted CI) inside a fixed 30 s case budget. Its case budget is now the named `GROUP_PROBE_REFUSAL_CASE_BUDGET_MS = 60_000`, with the reason in its comment. Both rows pass. Only the test budget changed. Follow-ups from the verifiers are ticketed: two remaining untagged finalizer refusals, and EPERM handling in the vm-file-client and adoption-routing group probes plus the 30 s sibling budgets in the probe-budget file. Refs HACK-1463 🤖 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.
Summary
Fix unit G103: the HACK-1455 follow-ups after #469 (storage diagnostics), #464 (global-state guard) and #465 (label review). Diagnostics and test infrastructure only: no carrier check, deadline, retention or recovery rule changes.
Root causes and fixes
#469: storage diagnostics
assertNativeComposeStorageBindingUnchangedcalled the helper directly, and nothing drove the real Docker carrier with a binding that changed mid-step. So any call site could go back toif (!same(before, x)) return refuse();unnoticed.New
tests/native-compose-storage-binding-call-sites.test.tsruns the realcreateNativeComposeDockerStorageXattrCarrierin process on the synthetic daemon fromtests/helpers/native-compose-storage-docker.ts. It drives an original readonly verify (inspect, journaled invoke, deferred finish) and a cold provision. Only the saved material binding is a stand-in:assertNativeComposeMaterialAuthorityis spied, and the chosen read returns the same binding with another receipt hash. The table names the read that reaches each call site and the stage it must report. The baseline case pins the total read count (53 for verify, 4 for provision), so a change to the read order fails there first.The case runs with the synthetic
dockerfirst onPATHand an isolated Docker client config, so the engine-ID admission keeps the synthetic CLI and never dials a real socket.PATHand theDOCKER_*selectors are restored infinally, and a timed-out case's carrier is aborted inafterEach.refuseFromwas untested. Newtests/native-compose-storage-command-record.test.tscases make the creation check refuse before the record is written (check 1) and after it is read back (check 2). Each asserts the issued boundary and its post-deadline reason. The carrier table's read 6 covers the same path end to end.storage-helper-start,storage-provision). The carrier table also checks five different stages.assertNativeComposeStorageCarrierStateUnchangedreplacessame(repeated.state, stopped.state)andsame(finalCarrier.state, stopped.state). A stopped carrier whose state changed is the same instability as a changed creation time or policy, so it reportsstorage-helper-policy/helper-policy-stability.assertNativeComposeStorageBindingUnchangedatstorage-helper-cleanup(the carrier's own cleanup stage). The owner-confirmed check still runs first, as before.deadlineReasons, so they never readstorage-budgetpast the deadline. The reference docs say so.fault-carrier-state-changedsynthetic-daemon fault returns anotherFinishedAton the Nth policy inspection.STORE_CASE_BUDGET_MSinstead of Bun's 5 s default. G99 / HACK-1439 has no open PR or remote branch yet, so there was nothing to merge.#464: global-state guard
The refused-teardown case could bypass the fixture gate. It now asserts that it holds the gate (
fixtureActive,!fixtureUncertain). As a plainboundedTestit fails.restoreProcessListeners' docstring claimed removal had no effect. It said that removing a leftover listener "cannot let unsettled work reach the host". That is wrong for cleanup handlers: a leftoverSIGINT/SIGTERMhandler that kills a child on Ctrl-C stops running, and the child can outlive an interrupted run.Decision: keep report-and-restore to the baseline. The guard does not call the handler or wait for what it owns. It cannot know what the handler owns or whether running it now is safe, and running another file's handler at an arbitrary point is the cross-file interference the restore exists to prevent. The leak report names the file and fails the run, so the fix belongs in that file's teardown. The docstring and the verify skill now say this.
withOwnedProcessListenersrestored only when its callback settled. On a timeout the callback never settles, so other code's listeners stayed removed. Its restore is now idempotent and registered three ways:finally;onTestFinished, which Bun runs after a timeout;afterEachruns beforeonTestFinished) would blame the timed-out test.A saved
oncelistener now comes back asonce. Bun does not exposeonce:listeners()andrawListeners()both return the bare function. So the newtests/helpers/process-once-listeners.tswraps the registration methods ofprocessand remembers the last registration kind; the preload installs it before any file loads. The guard's file-boundaryrestoreProcessListenersuses the same registry for missing listeners.#465: label review
hack_ownership_label_namespaces_block_enrollmentnow covers the dependency-cache declaration form. Without an ownership key that form reports no owner label; with each of the four ownership keys it still reports one and refuses enrollment.Mutation proof
Each mutant was applied to the source and run against its own focused test with a scratch driver; the file was restored byte for byte afterwards. The kill reason was checked in each log; for the binding call sites it is
private-statebecomingunclassified(andstorage-budgetonce late).same()+refuse()form: observe, provision (latest, admission, after), owner check, admitted, create admission, final, start admission, cleanup authority, remove admission, finalizerrepeatedstate check back toif (!same(...)) return refuse()finalCarrierstate check back to the untagged formreturn refuseFrom(error)becomesreturn refuse()"storage-helper-start"storage-provisionrow of the helper testgatedTestbecomesboundedTestonTestFinishedrestorerestoreUnsettledOwnedProcessListenerswithOwnedProcessListenersre-adds every listener as persistentrestoreProcessListenersre-adds every listener as persistenthack_ownership_label_namespaces_block_enrollment(owner-label assertion of the new form)Evidence
mise x bun@1.4.2),bun install --frozen-lockfile; nobun.lockchange.bun run check: privacy-check ok, 34 warnings, all existing complexity warnings (invoke59 -> 57).bun run --cwd packages/cli typecheck: 0 errors.gitleaks dirover thegit log -p origin/next..HEADexport): no leaks.cargo fmt --check,clippy --all-targets -D warningsandproject_contract hack_ownership_label_namespaces_block_enrollmentall pass. The TS parity scantests/hack-ownership-labels.test.tspasses (3).wf/g99-test-reliability-under-load(HACK-1439; not merged, no PR yet): clean. That branch setssetDefaultTimeout(STORE_CASE_BUDGET_MS)for the whole xattr-owner file, which agrees with the explicit budget here.test(shards 1-4), docker-e2e (shards 1-4), Linux and macOS candidate core, the Linux and macOS-arm config compilers, runtime state models, linux-process-lifetime, runtime-images and secret-scan. On the hosted runner, the 19 new carrier and listener cases took 0.06-0.9 s each.Config compiler (macos-15-intel).tests/native-compose-file-owner.test.ts, "cancellation during the last durable up arm …", hit its explicit 5000 ms timeout (5312 ms). That file and test are not changed here. In the last greennextrun (40125320831) the same case took 3071 ms, and itsdownsibling took 5418 ms under a 30 s budget. It is not on the known-flake list, so it was not rerun; it is reported as a finding.Gaps
assertNativeComposeMaterialAuthority. No real receipt publication happens between two carrier reads, and no CLI fault changes the saved binding mid-proof.onceregistry wrapsprocess.on,once,addListener,prependListenerandprependOnceListenerfor the test process. A function registered for one event both ways at once is read as its last registration kind. Without the root preload (for examplepackages/db),oncelisteners added before the helper first loads come back persistent.afterEacherror also stops Bun from running that test'sonTestFinished. So in this repository the preload restore is the one that matters;onTestFinishedcovers runs without the preload.Refs HACK-1455
🤖 Generated with Claude Code