Repository navigation
fix(destroy): name an unreadable host.json in the HOST_TIMEOUT of an unshared group - #331
Conversation
…unshared group readHostRecord reads a host.json that it cannot read as missing, so a session with a token reads its group as unshared, and destroy --force timed out with a hint about another pid namespace or another machine. That hint was wrong there. stopSessionProcesses now asks hostRecordUnreadable, which reads host.json as readHostRecord does but reports a failed read or check. When it does, the HOST_TIMEOUT message names the file and says to make it readable and run destroy again. readHostRecord and hostRecordUnreadable share readHostRecordOrThrow, which facts.mjs now covers. Also from the #327 follow-ups: - SKILL.md: thirteen runs, not eleven, explore more than 20 million distinct states (measured on the merged model; the largest is mutant-ReconcileSkipsTerminal with 88,731,772). - reconcileSession: the marker comment now says that a destroy --force that cannot read the group writes it once the host is dead, while its group may still read alive. - USAGE: until host.json is readable, gc removes an ended session, but a session whose host died while it ran stays running.
|
@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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5636b1419e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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. |
…efused Codex round 1 on #331: a host.json that is a folder, invalid JSON or an invalid record stays unusable when it is readable, so the advice to make it readable was wrong there. hostRecordUnreadable becomes hostRecordError, which returns the error. The HOST_TIMEOUT message now names that error ("Failed to read the host record at <path> (EISDIR)", or "The host record contains invalid JSON at <path>") and says to make the file readable only for EACCES or EPERM. New unit case: a read of host.json that EACCES refuses gets the advice, and a folder (EISDIR) does not.
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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. |
…ecord-hint # Conflicts: # .agents/skills/verify-session-lifecycle/SKILL.md # .agents/skills/verify-session-lifecycle/coverage.json
…ecord-hint # Conflicts: # .agents/skills/verify-session-lifecycle/coverage.json
Why
Follow-up of #327.
readHostRecordreads ahost.jsonthat it cannot read (for examplechmod 000, or a folder in its place) as missing. A session with a token then reads its group as unshared, sodestroy --forcetimes out withHOST_TIMEOUT. The message told the user that the CLI runs in another pid namespace or on another machine, which is wrong there.What changes for users
HOST_TIMEOUTmessage now names the file and the error, for example "Failed to read the host record at<session>/host.json(EACCES), so this process cannot prove which processes are the session's. Make that file readable and run destroy again." It gives that advice only when the read was refused (EACCES or EPERM). A folder (EISDIR), invalid JSON or an invalid record gets the error alone, because a readable file does not fix it. The error code, exit code anddetailsstay the same.gcremoves such a session once it has ended, but a session whose host died while it ran staysrunning, because its group reads alive. I checked both with the real CLI (below).No JSON shape changes: only the free-text
error.messageof this one case.What changes for maintainers
hostRecord.ts:readHostRecordOrThrowholds the read thatreadHostRecorddid.readHostRecordstill returnsnullon any failure, and the newhostRecordErrorreturns the error when the file exists but its read or its check fails, elsenull.facts.mjscoversreadHostRecordOrThrow.lifecycle.ts:stopSessionProcessesbuilds the hint of an unshared group withunsharedGroupHint, which askshostRecordError. It runs only after the wait has timed out.current-checkfails(25,554,382),mutant-HostChecksCreateFailed(29,705,764),mutant-HostPollsCreateFailed(45,996,296),mutant-ReusedIdReadsGone(52,902,414),current-win32(53,200,220),current-no-token(63,127,914),current,current-unseenandmutant-GcFinalCheck(64,052,288 each),current-refused(64,552,948),mutant-ShutdownChecksCommandable(69,117,970),mutant-GcSkipsNonCollectable(69,393,808) andmutant-ReconcileSkipsTerminal(88,731,772).reconcileSession: the comment on the destroy marker said thatrecordDestroyedwrites it only for a terminal manifest or once the host and the group are dead. Since fix(destroy): destroy --force ends a session whose program group it cannot read #327, adestroy --forcethat cannot read the group writes it once the host is dead, while the group can still read alive. The comment now says so. Its conclusion, that the host has ended or is ending, still holds.readHostRecord, the newreadHostRecordOrThrow,stopSessionProcessesandfacts.mjschanged.Validation
test/unit/host/lifecycle.test.tsput a folder wherehost.jsonwas (EISDIR, so the case also works as root), or make its read throw EACCES. They fail with the old message and pass now.tsc --noEmit,oxlint,oxfmt --check,check.sh --drift-only,test/unit(2,065 tests), and thelifecycleintegration tests for host.json, pid namespaces and unreadable reads (8 tests) pass.AGENT_TTY_HOME:chmod 000 host.json:destroy --forceexits withHOST_TIMEOUTand "Failed to read the host record at … (EACCES) … Make that file readable and run destroy again.", andgcremoves the session. With invalid JSON inhost.json, the message says "The host record contains invalid JSON at …" and gives no advice.chmod 000 host.json:inspectshowsrunning,gcskips it ("session host is still alive"), anddestroy --forcegives the new message.host.jsonchanges during the wait on its thread. CI on 5636b14 failed one test, "gc keeps an exited session while a fork chain that the program left runs", intest-integration (4/4). The plain destroy at its end timed out in all 3 attempts. That path never reaches the new hint. The test passed 3 of 3 runs locally and passed on main's CI of 11c0f17. I am watching for it on a3d2c4a.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh