Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/verify-session-lifecycle/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ node -e 'console.log(require(process.argv[1]).status)' "$AGENT_TTY_HOME/sessions

`ReconcileRepairsDestroyed` repairs a destroyed record in the reconcile that replaced it, right after its own write. A reconcile process that was killed between that write and the repair left `failed`, after `destroy` had printed `destroyed: true`, and every later reconcile skipped the terminal manifest (coder/agent-tty#265). A `destroy` of an ended session that failed after its marker write left the marker next to `exited` or `failed` the same way.

Now `reconcileSession` first checks a terminal manifest other than destroyed for the marker. When the marker is there, `writeMarkedDestroyed` writes destroyed from that copy, with `hostPid` and `childPid` null, the exit code kept and no failure fields, and the reconcile unlinks the socket, which a `destroy` whose write failed left, and returns. Only a `destroy` writes the marker, after it stopped the session and right before its destroyed write, so the marker means that `destroy` chose destroyed. `scanHome` skips a session with the marker, so `home list` and the dashboard picker do not count it while its manifest reads `failed`. The replay holds the repair write of the reconcile in `list` and kills that process:
Now `reconcileSession` first checks a terminal manifest other than destroyed for the marker. When the marker is there, `writeMarkedDestroyed` writes destroyed from that copy, with `hostPid` and `childPid` null, the exit code kept and no failure fields, and the reconcile unlinks the socket, which a `destroy` whose write failed left, and the old socket of an older host, and returns. Only a `destroy` writes the marker, after it stopped the session and right before its destroyed write, so the marker means that `destroy` chose destroyed. `scanHome` skips a session with the marker, so `home list` and the dashboard picker do not count it while its manifest reads `failed`. The replay holds the repair write of the reconcile in `list` and kills that process:

```sh
id=$(node --import tsx src/cli/main.ts create --json -- sh -c 'exec cat' | node -e 'let s="";process.stdin.on("data",(d)=>(s+=d)).on("end",()=>console.log(JSON.parse(s).result.sessionId))')
Expand Down
4 changes: 2 additions & 2 deletions .agents/skills/verify-session-lifecycle/coverage.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"src/host/lifecycle.ts#destroySession": "d840dd07a0e481f5",
"src/host/lifecycle.ts#isMarkedDestroyed": "d178f4a63473915c",
"src/host/lifecycle.ts#writeMarkedDestroyed": "4863c35f70699757",
"src/host/lifecycle.ts#reconcileSession": "4e5dac8144a0d9fb",
"src/host/lifecycle.ts#reconcileSession": "739c83bf945000c6",
"src/host/hostMain.ts#waitForManifestFromCreate": "fd500651f83683eb",
"src/host/hostMain.ts#isSessionCommandable": "260060f07b068ec1",
"src/host/hostMain.ts#SHUTDOWN_KILL_GRACE_MS": "51171d9c5aa6c510",
Expand Down Expand Up @@ -91,7 +91,7 @@
"src/host/hostMain.ts#runHost>handlers.resize": "d601680963309542",
"src/host/hostMain.ts#runHost>handlers.destroy": "ccff44951e711dac",
"skill:model/SessionLifecycle.tla": "f7270faba013acff",
"skill:scripts/facts.mjs": "b336aed63c9e6cc3",
"skill:scripts/facts.mjs": "82adfcc9fbc7fbb7",
"skill:scripts/check.sh": "3a47553300cb217a"
},
"callSites": {
Expand Down
2 changes: 1 addition & 1 deletion .agents/skills/verify-session-lifecycle/scripts/facts.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ function repairFacts() {
const reconcile = requireDeclaration(lifecycle, 'reconcileSession').body;
const marked = indexOfStatement(
reconcile,
/^if \(manifest !== null && isMarkedDestroyed\(sessionDirectory, manifest\)\) \{\s*if \(await writeMarkedDestroyed\(manifestFile, manifest\)\) \{\s*const socketFile = socketPath\(sessionDirectory\);\s*await unlinkIfPresent\(socketFile\);\s*await removeSocketDirectoryIfEmpty\(socketFile\);\s*\}\s*return;\s*\}$/,
/^if \(manifest !== null && isMarkedDestroyed\(sessionDirectory, manifest\)\) \{\s*if \(await writeMarkedDestroyed\(manifestFile, manifest\)\) \{\s*const socketFile = socketPath\(sessionDirectory\);\s*await unlinkIfPresent\(socketFile\);\s*await removeSocketDirectoryIfEmpty\(socketFile\);\s*await removeLegacySocketBestEffort\(socketFile\);\s*\}\s*return;\s*\}$/,
);
const markedCheck = normalize(
requireDeclaration(lifecycle, 'isMarkedDestroyed').body.getText(),
Expand Down
2 changes: 1 addition & 1 deletion src/host/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
- The `run` handler waits 30,000 ms by default. The `wait` handler needs exactly one of `exit` or `idleMs`, else it throws `INVALID_DURATION`. `waitForRender` polls every `RENDER_WAIT_POLL_INTERVAL_MS` (200 ms, `src/renderWait/limits.ts`). [hostMain.ts:runHost]
- `runHost` handles PTY exit once. If the status is `destroying` it records `destroyed`, else `exited`. Shutdown runs once. [hostMain.ts:runHost]
- `destroySession` throws `SESSION_NOT_FOUND` when there is no manifest, or when the session directory is gone at its `destroyed` write, and `SESSION_ALREADY_DESTROYED` for a destroyed session. When it records `destroyed` itself, `recordDestroyed` first writes the empty `<sessionDir>/destroyed` marker with `writeDestroyMarker`. Before it records a terminal session as `destroyed`, it calls `stopProgramGroup` with the group from `readProvenProgramGroup`, so only a group with a token gets a signal, and it throws `HOST_TIMEOUT` while that group lives. A session already recorded `destroyed` with the unshared group gets no wait, because only a process that saw the group recorded it, after its wait. A running session gets the `destroy` RPC. With `force` it gets `SIGKILL` for the program's group and, only when `hostMatches` proves the host, for the host: on Linux `judgeHost` must return `alive`, elsewhere `/bin/ps` must show `_host <sessionId>`, and on Windows the host gets no signal. It polls every 100 ms for at most 50 attempts and throws `HOST_TIMEOUT` if the status is still not terminal. [lifecycle.ts:destroySession]
- `reconcileSession` writes `destroyed` (both PIDs `null`, the exit code kept, no failure fields) over a terminal manifest other than `destroyed` when the destroy marker exists, and returns (`writeMarkedDestroyed`). Otherwise it does nothing when the manifest is missing, the status is terminal or the host can be alive: `judgeHost` with the record from `host.json` does not return `dead`. A null `hostPid` counts as a dead host. `allocateSession` writes the host PID into the first manifest, so a new session already has a `hostPid`. After the liveness check, `reconcileSession` reads the manifest again and does nothing when it is now missing or terminal. Otherwise it maps the status it read again: `running` to `failed` (failure origin `host-death`), `exiting` to `exited`, `destroying` to `destroyed` and any other status to `failed` (origin `unknown`), and sets both PIDs to `null`. Right after that write, when its status was not `destroyed` and the destroy marker exists, it writes `destroyed` again from the same copy, with both PIDs `null` and no failure fields. When the session directory is gone at any of these writes, it returns. [lifecycle.ts:reconcileSession] [lifecycle.ts:writeMarkedDestroyed]
- `reconcileSession` writes `destroyed` (both PIDs `null`, the exit code kept, no failure fields) over a terminal manifest other than `destroyed` when the destroy marker exists (`writeMarkedDestroyed`). It then unlinks the socket, removes the Home's socket directory if it is empty, removes the old socket at `legacySocketPath` when `isOwnLegacySocket` accepts it, and returns. Otherwise it does nothing when the manifest is missing, the status is terminal or the host can be alive: `judgeHost` with the record from `host.json` does not return `dead`. A null `hostPid` counts as a dead host. `allocateSession` writes the host PID into the first manifest, so a new session already has a `hostPid`. After the liveness check, `reconcileSession` reads the manifest again and does nothing when it is now missing or terminal. Otherwise it maps the status it read again: `running` to `failed` (failure origin `host-death`), `exiting` to `exited`, `destroying` to `destroyed` and any other status to `failed` (origin `unknown`), and sets both PIDs to `null`. Right after that write, when its status was not `destroyed` and the destroy marker exists, it writes `destroyed` again from the same copy, with both PIDs `null` and no failure fields. When the session directory is gone at any of these writes, it returns. [lifecycle.ts:reconcileSession] [lifecycle.ts:writeMarkedDestroyed]
- Every kill of the program's process group goes through `stopProgramGroup`. For a record with a token, `judgeProgramGroup` returns `owned` only when a process that is not a zombie (a process whose main thread exited is not one while another of its threads runs) has the program's pid as its process group and session id, and the recorded autogroup on the recorded boot. A scan that finds no live member runs once more, because a member can fork and end after the scan listed `/proc`. `stopProgramGroup` reads that process again right before the kill. `unproven` (another boot id, or only members with another session id or autogroup) and `unknown` kill nothing. `gone` (no live member) first sends the group SIGSTOP with `signalProcessGroup`, because a fork chain whose members each live about 2 ms is never in view, and a stopped member is. The judge then runs again, and a frozen group that gets no kill gets SIGCONT, also when a scan throws. So a group id that an unrelated group took, whose members no scan shows either, gets a SIGSTOP and a SIGCONT. `unknown` means that `/proc` refused the boot id, or showed a member with the program's session id but no autogroup, so `isProgramGroupAlive` reads it alive. A record without a token gets the kill of its `pid`. A `{ childPid, sessionDirectory }` group gets the kill of `childPid`, and reads alive, only while `sessionDirectory` exists, checked in the same synchronous block. The unshared group `{ unshared: true }` gets no signal and reads alive. [programGroup.ts:stopProgramGroup] [programGroup.ts:judgeProgramGroup]
- `captureProgram` records a token only on Linux, for its own child that leads its own session and has an autogroup other than the host's, within 250 ms, and only when `/proc` shows the boot id. `runHost` calls it right after `createPty` and writes the record before its first manifest write, with no `await` in between. [programGroup.ts:captureProgram] [hostMain.ts:runHost]
- `allocateSession` throws `INVALID_DIMENSIONS` for non-positive `cols` or `rows`, `STORAGE_READ_ERROR` when `cwd` is not a directory and `INVALID_INPUT` when the shell path is not a file. An empty `command` defaults to the shell path. It creates the session directory and calls `launch` only after these checks pass. [lifecycle.ts:allocateSession]
Expand Down
1 change: 1 addition & 0 deletions src/host/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,7 @@ export async function reconcileSession(
const socketFile = socketPath(sessionDirectory);
await unlinkIfPresent(socketFile);
await removeSocketDirectoryIfEmpty(socketFile);
await removeLegacySocketBestEffort(socketFile);
}
return;
}
Expand Down
14 changes: 14 additions & 0 deletions test/unit/host/lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,20 @@ describe.skipIf(process.platform === 'win32')(
await rm(dirname(oldSocketFile), { recursive: true, force: true });
});

it('a reconcile that repairs a marked session removes it', async () => {
const directory = await seedSession(
makeRecord({ status: 'failed', hostPid: null, childPid: null }),
);
writeFileSync(join(directory, 'destroyed'), '');

await reconcileSession(directory);

expect(await readManifest(manifestPath(directory))).toMatchObject({
status: 'destroyed',
});
expect(existsSync(oldSocketFile)).toBe(false);
});

it('destroy removes it when the host is dead', async () => {
await seedSession(makeRecord({ status: 'destroyed', childPid: null }));
killProbeReportsDead();
Expand Down
Loading