Repository navigation
fix(host): stop the marker watch once the host has a new parent, give long TLC runs more workers - #330
Conversation
… 60 s A process of the same user that takes the creator's pid keeps the probe answering, so the watch ticked for as long as that process lived. On Linux and macOS the host is the creator's child and gets a new parent when the creator exits, and the watch should stop then. It should also stop 60 s after it started, as on Windows, where the parent pid stays. Both new cases fail on the head. The tests now set the host's parent pid, because runHost runs in the test process. The zombie case gives way to the new-parent case: a zombie creator also gives the host a new parent.
…after 60 s A process of the same user that took the creator's pid kept the watch's probe answering, so the watch ticked at 10 Hz for that process's life. The host is the creator's child, and Linux and macOS give it a new parent when the creator exits, also while the creator is a zombie and before its pid can be reused. So off Windows, a process.ppid other than the creator's pid ends the watch, and the /proc zombie read goes. On Windows the probe still decides. The watch also stops after 60 s in any case. facts.mjs reads the new watch, and SKILL.md states the bound as a residual.
A share of CI's lifecycle-model job took as long as its longest run, which TLC checked with 1 worker: 43 min for mutant-ReconcileSkipsTerminal. The runs that expect pass now share the CPUs, each with at least 1 worker. A run that expects a violation keeps 1 worker, so its trace stays the shortest.
The ci.yml comment of #328 said that check.sh waits for each batch of CHECK_JOBS runs. On Linux it keeps a pool and starts a new run as soon as one ends; only the bash 3.2 of macOS waits for each batch. SKILL.md still said that CI runs the check in four shares, and that it takes about an hour. Both now describe the 8 shares, CHECK_JOBS 12 and the TLC workers of the runs that expect pass.
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
# Conflicts: # .agents/skills/verify-session-lifecycle/SKILL.md # .agents/skills/verify-session-lifecycle/coverage.json
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Follow-up to #324 and #328. Merged with main after #327.
What changed
src/host/hostMain.ts).isProcessAliveanswering. The watch then ticks at 10 Hz for that process's life. The host stays correct, because no marker comes after a successful create.process.ppidother than the creator's pid ends the watch. The/proczombie read goes.isProcessAlivestill decides, as before.CREATE_FAILED_WATCH_MAX_MS(60 s) ends the watch on every platform. Acreateends within about 20 s of the host's start-up check. SKILL.md lists the new residual: acreatethat writes its marker more than 60 s later, for example after a SIGSTOP, leaves the host serving./proc. The parent pid needs no/procand also covers macOS, so I used it.check.shgives the runs that expect pass more TLC workers. These runs explore the whole model. They now share the CPUs:CPUS / min(passes, CHECK_JOBS)workers each, at least 1. A run that expects a violation keeps 1 worker, so its trace stays the shortest. In CI a share has 4 vCPUs and 1 or 2 such runs, so each gets 4 or 2 workers. Before, the longest run (mutant-ReconcileSkipsTerminal) took 43 min with 1 worker and set the time of its share.check.shwaits for each batch ofCHECK_JOBSruns. That was wrong: on Linux it keeps a pool (wait -n) and starts a new run as soon as one ends. Only the bash 3.2 of macOS waits for each batch.CHECK_JOBS=12, the pool and the TLC workers.coverage.jsonsync for the coveredrunHost,facts.mjsandcheck.shchanges.facts.mjs(pollsCreateFailed) reads the new watch.No CLI JSON, schema, manifest or event-log change.
Validation
test/unit/host/hostMarker.test.ts(new parent, 60 s bound). Both fail on the code of 99cb181 and pass after 89ff040. The tests set the host's parent pid through aprocess.ppidgetter, becauserunHostruns in the test process. The zombie case goes: a zombie creator also gives the host a new parent.tsc --noEmit,oxlint src test vitest.config.tsandoxfmt --check .are clean, andvitest run test/unitpasses 2065 tests.vitest run --maxWorkers=1 --retry=2 test/integration/lifecycle.test.ts test/integration/cli.test.tspassed 148 tests before the merge. After the merge, lifecycle passed. One cli test (uses --home instead of AGENT_TTY_HOME) hit its 5 s timeout while 90 TLC runs loaded the machine.cli.test.tsalone then passed 77 of 77.facts.mjs: I broke the new watch by hand (without theppidcheck, without the deadline) andpollsCreateFailedturned false each time. After the merge, all 41 facts are true.check.sh --update-coverageafter the merge with fix(destroy): destroy --force ends a session whose program group it cannot read #327: all 90 runs end as expected. Distinct states are unchanged:current64,052,288,current-no-token63,127,914,current-win3253,200,220. (Before the merge, all 85 runs also ended as expected.)lifecycle-modelshares took 11 to 33 min (share 1, withcurrentandcurrent-unseen, took 33 min). With 1 TLC worker per run they took 15 to 43 min on test: trial of the 8-share lifecycle-model job on #324 (do not merge) #329 and 16 to 36 min in the merge group of fix: on Windows, stop a late host after a failed create in more cases, and name the session in text mode #324. The ci.yml comment and SKILL.md give these times and name the model of fix: on Windows, stop a late host after a failed create in more cases, and name the session in text mode #324.lifecycle-modelshares took 9 to 38 min (24.6 / 9.2 / 13.7 / 11.4 / 37.8 / 32.4 / 31.7 / 15.4), within the 55 min timeout.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh