Skip to content

Commit 778aa6e

Browse files
committed
fix(host-kit): require every recorded identity claim before a signal
judgeProcessIdentity is now the whole rule: ours only when every recorded claim (start time and/or command) was read and matches; reassigned for a contradicted start time, or a mismatching command with no start time recorded; unverified for everything else. The start-time and command reads fail independently, so one confirmed claim beside an unread one no longer lets the startup reaper, the app-log stop or the final SIGKILL signal an unconfirmed lifetime. The app-log inspect uses the same judgment as its terminate, so the two agree by construction. The recording marker check keeps one distinction on top: a fully answered mismatch is a refusal, while an unanswered read stays unprovable. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_016qAu53rMak1bXMuR5U2gF4
1 parent 9f48ee2 commit 778aa6e

7 files changed

Lines changed: 102 additions & 49 deletions

‎packages/host-kit/src/internal/host-process-stop-confirm.test.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,19 +170,21 @@ const STOP_CASES: HostCase[] = [
170170
},
171171
{
172172
// Windows hides only the command line of an elevated or other-user process, and that is the #3540
173-
// companion: it must be signaled and retained, never reported stopped.
173+
// companion. With one recorded claim unread its lifetime is unconfirmed: it is retained, never
174+
// signaled, and never reported stopped.
174175
name: 'recorded process hides its command line, as an elevated one does',
175176
identity: { command: null },
176177
...DENIED_ON_TERM,
177178
...HELD,
178-
expect: REFUSED_ON_TERM,
179+
expect: UNVERIFIED,
179180
},
180181
{
182+
// The start-time and command reads fail independently: a matching command beside a start time
183+
// the host did not answer is not a confirmed lifetime.
181184
name: 'recorded process hides its start time',
182185
identity: { startTime: null },
183-
...DENIED_ON_TERM,
184186
...HELD,
185-
expect: REFUSED_ON_TERM,
187+
expect: UNVERIFIED,
186188
},
187189
{
188190
// POSIX `hidepid`, a `ps` that times out, or a Windows process that exited while its object still

‎packages/host-kit/src/internal/host-process.test.ts‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -138,33 +138,60 @@ test('captureHostEnvironment copies the environment and reads the working direct
138138
assert.equal(source.cwd.mock.calls.length, 1);
139139
});
140140

141-
test('judgeProcessIdentity reads a command mismatch as reassigned only when no start time was recorded', () => {
142-
const matches = (command: string) => command === 'recorded';
143-
const facts = (startTime: string | null, command: string | null) => ({
144-
startTime,
145-
command,
146-
zombie: false,
147-
});
148-
assert.equal(judgeProcessIdentity({ matches }, facts('S1', 'other')), 'reassigned');
149-
assert.equal(
150-
judgeProcessIdentity({ startTime: 'S1', matches }, facts('S1', 'other')),
141+
// One row per verdict rule: `ours` needs every recorded claim read and matching.
142+
test.each([
143+
['both claims read and matching', { startTime: 'S1', matches: true }, ['S1', 'recorded'], 'ours'],
144+
['only a start time recorded, read and matching', { startTime: 'S1' }, ['S1', null], 'ours'],
145+
['only a command recorded, read and matching', { matches: true }, [null, 'recorded'], 'ours'],
146+
[
147+
'a matching command beside an unread start time',
148+
{ startTime: 'S1', matches: true },
149+
[null, 'recorded'],
151150
'unverified',
152-
);
153-
assert.equal(
154-
judgeProcessIdentity({ startTime: 'S1', matches }, facts(null, 'other')),
151+
],
152+
[
153+
'a matching start time beside an unread command',
154+
{ startTime: 'S1', matches: true },
155+
['S1', null],
155156
'unverified',
156-
);
157-
assert.equal(
158-
judgeProcessIdentity({ startTime: 'S1', matches }, facts('S2', 'recorded')),
157+
],
158+
['nothing read', { startTime: 'S1', matches: true }, [null, null], 'unverified'],
159+
['nothing recorded', {}, ['S1', 'recorded'], 'unverified'],
160+
[
161+
'a recorded start time beside a mismatching command',
162+
{ startTime: 'S1', matches: true },
163+
['S1', 'other'],
164+
'unverified',
165+
],
166+
[
167+
'an unread recorded start time beside a mismatching command',
168+
{ startTime: 'S1', matches: true },
169+
[null, 'other'],
170+
'unverified',
171+
],
172+
[
173+
'a contradicted start time',
174+
{ startTime: 'S1', matches: true },
175+
['S2', 'recorded'],
159176
'reassigned',
160-
);
161-
assert.equal(judgeProcessIdentity({ startTime: 'S1', matches }, facts('S1', 'recorded')), 'ours');
162-
assert.equal(judgeProcessIdentity({ startTime: 'S1', matches }, facts(null, null)), 'unverified');
177+
],
178+
[
179+
'a mismatching command with no start time recorded',
180+
{ matches: true },
181+
['S1', 'other'],
182+
'reassigned',
183+
],
184+
] as const)('judgeProcessIdentity: %s', (_name, recorded, [startTime, command], verdict) => {
185+
const expected = {
186+
...('startTime' in recorded ? { startTime: recorded.startTime } : {}),
187+
...('matches' in recorded ? { matches: (observed: string) => observed === 'recorded' } : {}),
188+
};
189+
assert.equal(judgeProcessIdentity(expected, { startTime, command, zombie: false }), verdict);
190+
});
191+
192+
test('judgeProcessIdentity reads a zombie as exited whatever else the host says', () => {
163193
assert.equal(
164-
judgeProcessIdentity(
165-
{ startTime: 'S1', matches },
166-
{ ...facts('S1', 'recorded'), zombie: true },
167-
),
194+
judgeProcessIdentity({ startTime: 'S1' }, { startTime: 'S2', command: null, zombie: true }),
168195
'exited',
169196
);
170197
});

‎packages/host-kit/src/internal/host-process.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -730,10 +730,11 @@ export type ProcessStopBudget = Readonly<{
730730
* - `reassigned`: a readable start time contradicts the record, or, when no start time was
731731
* recorded, a readable command does: the pid now names a different process and the recorded one
732732
* is gone.
733-
* - `ours`: at least one recorded claim is confirmed by a readable fact and none is contradicted.
734-
* - `unverified`: no recorded claim is confirmed, or a start time was recorded and the command
735-
* does not match. That may be the same lifetime running something else, as after an `exec` or an
736-
* argv rewrite, so it is neither proof the recorded process is gone nor permission to signal.
733+
* - `ours`: every recorded claim was read and matches. One confirmed claim beside an unread one is
734+
* not enough, because the start-time and command reads can fail independently.
735+
* - `unverified`: everything else. A claim the host did not answer, no claim at all, or a recorded
736+
* start time beside a command that does not match (the same lifetime may have exec'd or rewritten
737+
* its argv) is neither proof the recorded process is gone nor permission to signal.
737738
*/
738739
export type ProcessIdentityVerdict = 'ours' | 'exited' | 'reassigned' | 'unverified';
739740

@@ -742,13 +743,14 @@ export function judgeProcessIdentity(
742743
facts: HostProcessIdentityFacts,
743744
): ProcessIdentityVerdict {
744745
if (facts.zombie) return 'exited';
745-
const confirmedStartTime = Boolean(expected.startTime && facts.startTime);
746-
if (confirmedStartTime && facts.startTime !== expected.startTime) return 'reassigned';
747-
if (expected.matches && facts.command && !expected.matches(facts.command)) {
748-
return expected.startTime ? 'unverified' : 'reassigned';
749-
}
750-
const confirmedCommand = Boolean(expected.matches && facts.command);
751-
return confirmedStartTime || confirmedCommand ? 'ours' : 'unverified';
746+
const { startTime, matches } = expected;
747+
if (startTime && facts.startTime && facts.startTime !== startTime) return 'reassigned';
748+
const commandMatches = facts.command === null || !matches ? undefined : matches(facts.command);
749+
if (!startTime && commandMatches === false) return 'reassigned';
750+
if (!startTime && !matches) return 'unverified';
751+
const startTimeConfirmed = !startTime || facts.startTime === startTime;
752+
const commandConfirmed = !matches || commandMatches === true;
753+
return startTimeConfirmed && commandConfirmed ? 'ours' : 'unverified';
752754
}
753755

754756
/**

‎packages/host-kit/src/internal/owned-process-reaper.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,9 @@ function appendReapOutcome(
155155

156156
/**
157157
* Stops one recorded process through the shared escalation. The record names the process by start
158-
* time and exact command, so a pid the host shows running anything else is proven gone and never
159-
* signaled, and one the host will not describe is kept rather than guessed at.
158+
* time and exact command, and both must be read and match before any signal. A pid the host shows
159+
* in another lifetime is proven gone and never signaled; one the host will not fully describe, or
160+
* that shows the recorded lifetime running another command, is kept rather than guessed at.
160161
*/
161162
async function reapOwnedProcess(
162163
record: OwnedProcessRecord,

‎packages/host-kit/src/internal/owned-process-record.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ function driveHost(): void {
4646
const command = processState.commands.get(pid);
4747
return { stdout: command ? `${pid} ${command}\n` : '', stderr: '', exitCode: 0 };
4848
}
49+
if (!processState.starts.has(pid)) return { stdout: '', stderr: 'timed out', exitCode: 1 };
4950
const state = processState.zombiePids.has(pid) ? 'Z' : 'S';
5051
return { stdout: `${pid} ${state} ${processState.starts.get(pid)}\n`, stderr: '', exitCode: 0 };
5152
});
@@ -318,3 +319,22 @@ test('startup counts a pid reassigned after SIGTERM as missing, not terminated',
318319
cleanup();
319320
}
320321
});
322+
323+
test('startup keeps a record whose start time the host did not answer, without signaling it', async () => {
324+
// The start-time and command reads fail independently: a matching command alone is not the
325+
// recorded lifetime.
326+
const { store, cleanup } = reapFixture('unread-start');
327+
hostRuns(BROWSER);
328+
processState.starts.delete(BROWSER.pid);
329+
try {
330+
store.replace({ kind: 'daemon' }, [BROWSER]);
331+
332+
const summary = await reapOwnedProcessRecordsAtStartup(store);
333+
334+
assert.deepEqual(summary.unconfirmedPids, [BROWSER.pid]);
335+
assert.deepEqual(processState.signals, []);
336+
assert.equal(store.read().length, 1);
337+
} finally {
338+
cleanup();
339+
}
340+
});

‎src/platform-runtime-app-log-process.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,14 +407,15 @@ function markerIdentity(marker: AppLogProcessMarker): ProcessStopIdentity {
407407
return { startTime: marker.startTime, matches: (command) => command === marker.command };
408408
}
409409

410-
/** Owned only while both recorded facts are confirmed, under the shared identity rule. */
410+
/** Judged by the same identity rule the stop applies, so inspect and terminate agree. */
411411
async function inspectOwnedProcess(marker: AppLogProcessMarker): Promise<AppLogProcessOwnership> {
412412
if (!isProcessAlive(marker.pid)) return 'missing';
413-
const facts = await readProcessIdentityFacts(marker.pid);
414-
const verdict = judgeProcessIdentity(markerIdentity(marker), facts);
413+
const verdict = judgeProcessIdentity(
414+
markerIdentity(marker),
415+
await readProcessIdentityFacts(marker.pid),
416+
);
415417
if (verdict === 'exited') return 'missing';
416-
const bothConfirmed = facts.startTime !== null && facts.command !== null;
417-
return verdict === 'ours' && bothConfirmed ? 'owned-alive' : 'ownership-lost';
418+
return verdict === 'ours' ? 'owned-alive' : 'ownership-lost';
418419
}
419420

420421
/**

‎src/platform-runtime-screen-recording-process-host.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,10 @@ async function readOwnershipEvidence(
133133
if (verdict === 'exited') return 'gone';
134134
if (marker.startTime.length === 0 || marker.command.length === 0) return 'not-ours';
135135
if (verdict === 'reassigned') return 'not-ours';
136-
// A recording marker is ours only while both recorded facts are confirmed, and a readable fact
137-
// that does not match is a refusal.
138-
if (facts.startTime === null || facts.command === null) return 'unprovable';
139-
return verdict === 'ours' ? 'ours' : 'not-ours';
136+
if (verdict === 'ours') return 'ours';
137+
// The host answered both facts and they do not name the marker: a refusal that also holds the
138+
// spawned child's handle back, unlike a host that did not answer at all.
139+
return facts.startTime !== null && facts.command !== null ? 'not-ours' : 'unprovable';
140140
}
141141

142142
export async function terminateManagedProcessSet(

0 commit comments

Comments
 (0)