From 01e00dae72e1dbac91b2f5d9997d5e0b8091ad3a Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:40:12 +0800 Subject: [PATCH] fix(process): corroborate exiting Linux ownership candidates (#7958) * fix(process): corroborate Linux exits after procfs environ read races A token read failing with ENOENT/ESRCH may race with process exit while native identity observation still sees a live PID. Recheck native lifetime and accept absence only when corroborated; keep unverifiable or reused PIDs fail closed. Refs #7790 * fix(process): recheck Linux exit after final token retry A process can exit after the final retry of a failed procfs environment read. Re-observe native identity before producing final cleanup failures, and keep PID replacement and uncertain identity fail-closed. Add five integration-facing sweep regression cases for absence, live, unavailable, replaced and zombie-replaced candidates. Refs #7790 * fix(process): bound native exit settlement for token-open races After a terminal ENOENT/ESRCH run-token read, allow a tightly bounded native-only settlement window to corroborate pending Linux process exit. Retain an explicit failure for live/unavailable or changed PID identities, without inferring exit from the token error itself. Cover delayed absence, delayed replacement and bounded live retries. Refs #7790 * fix(process): share native token exit settlement * fix(process): signal only groups with validated members * fix(process): retain lineage through exit settlement * fix(process): type mixed identity observations in exit checks Map inference collapsed a readable/absent union onto the absent overload after main tightened ProcessIdentityObservation. * fix(process): refresh groups whose members exit during revalidation A same-group child forked after the revalidation snapshot is not an empty group, so cleanup reads the table again before skipping the signal. * test(process): arm group-refresh exit from the revalidation snapshot The regression follows the table read after planning observed the launcher token, and requires a later refresh to see the forked child. * test(mcp): assert bundle path issues by field The compare_bundles rejection reports each relative path as an invalid_format issue instead of one joined diagnostic sentence. * test(mcp): assert field issues for bundle paths and Dart ABI Relative bundle paths and rejected ABI selectors are reported as input issues on those fields, not as one joined diagnostic sentence. --------- Co-authored-by: Harshit Sharma <66710144+harshitethic@users.noreply.github.com> Co-authored-by: Cursor Agent --- .../ProcessOwnership.exitScope.test.ts | 23 +- .../ProcessOwnership.nativeExit.test.ts | 784 ++++++++++++++++++ src/process/ProcessOwnership.ts | 139 ++-- src/process/ProcessOwnershipObservation.ts | 119 ++- src/process/ProcessRunTokenObservations.ts | 7 + .../boundary/cli/bundleComparisonCli.test.ts | 20 +- .../boundary/mcp/bundleComparisonMcp.test.ts | 16 +- tests/boundary/mcp/flutterMcp.test.ts | 14 +- 8 files changed, 1048 insertions(+), 74 deletions(-) diff --git a/src/process/ProcessOwnership.exitScope.test.ts b/src/process/ProcessOwnership.exitScope.test.ts index 64df933d6b..86b19d9023 100644 --- a/src/process/ProcessOwnership.exitScope.test.ts +++ b/src/process/ProcessOwnership.exitScope.test.ts @@ -34,13 +34,19 @@ const identity = ( }, }); -it.each(["readable", "retried", "reused", "retained-parent"] as const)( +it.each([ + "readable", + "retried", + "reused", + "retained-parent", + "retained-parent-esrch", +] as const)( "keeps child scope tied to its native generation after parent exit (%s)", async (mode) => { const parent = row(200, 1); const child = row(300, parent.pid); const owner = new ProcessOwnershipEvidenceOwner(); - if (mode === "retained-parent") + if (mode === "retained-parent" || mode === "retained-parent-esrch") owner.reconcile( [ { @@ -64,6 +70,7 @@ it.each(["readable", "retried", "reused", "retained-parent"] as const)( let childTokenReads = 0; let reparented = false; const host: ProcessOwnershipHost = { + platform: "linux", listProcesses: async () => [parent, child], environment: async (pid) => pid === parent.pid ? { REA_PROCESS_RUN_ID: "run" } : {}, @@ -72,9 +79,15 @@ it.each(["readable", "retried", "reused", "retained-parent"] as const)( selected.map((process) => [ process.pid, process.pid === parent.pid - ? mode === "retained-parent" - ? { state: "unavailable", reason: "EACCES" } - : { state: "readable", runId: "run" } + ? mode === "retained-parent-esrch" + ? { + state: "unavailable", + reason: "opaque failure", + code: "ESRCH", + } + : mode === "retained-parent" + ? { state: "unavailable", reason: "EACCES" } + : { state: "readable", runId: "run" } : ++childTokenReads === 1 && mode !== "readable" ? { state: "unavailable", reason: "EACCES" } : { state: "readable", runId: undefined }, diff --git a/src/process/ProcessOwnership.nativeExit.test.ts b/src/process/ProcessOwnership.nativeExit.test.ts index 4768ad5110..115e520ba4 100644 --- a/src/process/ProcessOwnership.nativeExit.test.ts +++ b/src/process/ProcessOwnership.nativeExit.test.ts @@ -1,10 +1,19 @@ +import { spawn } from "node:child_process"; +import { once } from "node:events"; + import { expect, it, vi } from "vitest"; import { cleanupOwnedProcessGroup, + verifyNoTokenOwnedProcesses, type ProcessIdentityObservation, type ProcessOwnershipHost, type ProcessTableEntry, } from "./ProcessOwnership.js"; +import { + createSystemProcessOwnershipHost, + readProcessOwnershipFacts, + settleVanishedTokenFacts, +} from "./ProcessOwnershipObservation.js"; it.each(["zombie", "absent", "live", "unavailable"] as const)( "rechecks native lifetime when a token disappears during cleanup: %s", @@ -77,3 +86,778 @@ it.each(["zombie", "absent", "live", "unavailable"] as const)( expect(signalGroup).not.toHaveBeenCalled(); }, ); + +// A failing procfs environ read does not establish exit on its own. The +// native observer must corroborate the absence before cleanup drops a PID. +it.each(["ESRCH", "ENOENT"] as const)( + "settles %s from /proc/PID/environ only after native absence confirmation", + async (code) => { + const row: ProcessTableEntry = { + pid: 900, + parentPid: 1, + processGroupId: 900, + state: "S", + command: "short-lived-fixture", + }; + let identityReads = 0; + const live: ProcessIdentityObservation = { + state: "readable", + identity: "start:900", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { state: "observed", parentPid: 1, processGroupId: 900 }, + }; + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => [row], + environment: async () => { + throw Object.assign( + new Error(code + ": no such process, open '/proc/900/environ'"), + { code }, + ); + }, + processIdentities: async (rows) => + new Map( + rows.map(({ pid }) => [ + pid, + ++identityReads < 3 + ? live + : { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace" as const, + }, + ]), + ), + signalGroup: vi.fn(), + }; + const [fact] = await readProcessOwnershipFacts([row], host); + expect(identityReads).toBe(3); + expect(fact?.token).toMatchObject({ + state: "unavailable", + reason: expect.stringContaining(code), + }); + expect(fact?.state).toBe("absent"); + expect(fact?.identityAfter).toEqual({ + state: "absent", + reason: "not-found-in-caller-pid-namespace", + }); + expect(host.signalGroup).not.toHaveBeenCalled(); + }, +); + +it.each([ + { + code: "ESRCH", + mode: "live", + expectedReads: 3, + expectedState: "same-generation", + }, + { + code: "ESRCH", + mode: "reused", + expectedReads: 3, + expectedState: "unavailable", + }, + { + code: "ESRCH", + mode: "reused-zombie", + expectedReads: 3, + expectedState: "unavailable", + }, + { + code: "ESRCH", + mode: "unknown", + expectedReads: 3, + expectedState: "same-generation", + }, + { + code: "EACCES", + mode: "absent", + expectedReads: 2, + expectedState: "same-generation", + }, +] as const)( + "does not clear unverifiable /proc environ failures: $code / $mode", + async ({ code, mode, expectedReads, expectedState }) => { + const row: ProcessTableEntry = { + pid: 901, + parentPid: 1, + processGroupId: 901, + state: "S", + command: "fixture", + }; + let identityReads = 0; + const live: ProcessIdentityObservation = { + state: "readable", + identity: "start:901", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { state: "observed", parentPid: 1, processGroupId: 901 }, + }; + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => [row], + environment: async () => { + throw Object.assign(new Error(code + ": open '/proc/901/environ'"), { + code, + }); + }, + processIdentities: async (rows) => + new Map( + rows.map(({ pid }) => { + identityReads += 1; + if (identityReads < 3) return [pid, live] as const; + switch (mode) { + case "absent": + return [ + pid, + { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace" as const, + }, + ] as const; + case "reused": + return [ + pid, + { ...live, identity: "start:replacement" }, + ] as const; + case "reused-zombie": + return [ + pid, + { + ...live, + identity: "start:zombie-replacement", + lifecycle: { + state: "observed" as const, + lifecycle: "zombie" as const, + }, + }, + ] as const; + case "unknown": + return [ + pid, + { + state: "unavailable" as const, + reason: "permission denied", + }, + ] as const; + default: + return [pid, live] as const; + } + }), + ), + signalGroup: vi.fn(), + }; + const [fact] = await readProcessOwnershipFacts([row], host); + expect(identityReads).toBe(expectedReads); + expect(fact?.state).toBe(expectedState); + expect(fact?.identityAfter.state).not.toBe("absent"); + expect(fact?.token.state).toBe("unavailable"); + expect(host.signalGroup).not.toHaveBeenCalled(); + }, +); + +const itOnLinux = process.platform === "linux" ? it : it.skip; + +itOnLinux( + "verifies a real child exit between the native identity and follow-up read", + async () => { + const child = spawn( + process.execPath, + ["-e", "setInterval(() => {}, 1_000)"], + { + stdio: "ignore", + }, + ); + if (child.pid === undefined) throw new Error("fixture child did not start"); + const pid = child.pid; + const row: ProcessTableEntry = { + pid, + parentPid: process.pid, + processGroupId: pid, + state: "S", + command: process.execPath, + }; + const systemHost = createSystemProcessOwnershipHost("linux", {}); + let nativeReads = 0; + let exitArmed = false; + const host: ProcessOwnershipHost = { + ...systemHost, + environment: async () => { + exitArmed = true; + throw Object.assign( + new Error( + "ESRCH: simulated environ-open race before native process exit", + ), + { code: "ESRCH" }, + ); + }, + processIdentities: async (...args) => { + const observations = await systemHost.processIdentities!(...args); + nativeReads += 1; + if (exitArmed) { + exitArmed = false; + child.kill("SIGTERM"); + await once(child, "exit"); + } + return observations; + }, + signalGroup: vi.fn(), + }; + try { + const [fact] = await readProcessOwnershipFacts([row], host); + expect(nativeReads).toBeGreaterThanOrEqual(3); + expect(fact?.identityBefore).toMatchObject({ + state: "readable", + identity: expect.stringMatching(/^linux-starttime:/u), + }); + expect(fact?.token).toMatchObject({ + state: "unavailable", + reason: expect.stringContaining("ESRCH"), + }); + expect(fact?.identityAfter).toEqual({ + state: "absent", + reason: "not-found-in-caller-pid-namespace", + }); + expect(fact?.state).toBe("absent"); + expect(host.signalGroup).not.toHaveBeenCalled(); + } finally { + if (child.exitCode === null && child.signalCode === null) { + child.kill("SIGKILL"); + await once(child, "exit"); + } + } + }, +); + +it.each([ + "absent", + "delayed-absent", + "live", + "reused", + "delayed-reused", + "unknown", + "reused-zombie", +] as const)( + "resolves a Linux token-read exit race after final sweep retry: %s", + async (lastIdentity) => { + const row: ProcessTableEntry = { + pid: 902, + parentPid: 1, + processGroupId: 902, + state: "S", + command: "short-lived-worker", + }; + const live: ProcessIdentityObservation = { + state: "readable", + identity: "start:902", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { state: "observed", parentPid: 1, processGroupId: 902 }, + }; + let tokenReads = 0; + let readsAfterLastToken = 0; + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => [row], // stale snapshot must never prove exit + environment: async () => { + tokenReads += 1; + throw Object.assign(new Error("ESRCH: open '/proc/902/environ'"), { + code: "ESRCH", + }); + }, + processIdentities: async (members) => + new Map( + members.map(({ pid }) => { + if (tokenReads < 4) return [pid, live] as const; + readsAfterLastToken += 1; + const delayed = lastIdentity.startsWith("delayed-"); + if (readsAfterLastToken < (delayed ? 5 : 3)) + return [pid, live] as const; + switch (lastIdentity) { + case "delayed-absent": + case "absent": + return [ + pid, + { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace", + }, + ] as const; + case "delayed-reused": + case "reused": + return [ + pid, + { ...live, identity: "start:replacement" }, + ] as const; + case "reused-zombie": + return [ + pid, + { + ...live, + identity: "start:zombie-replacement", + lifecycle: { + state: "observed" as const, + lifecycle: "zombie" as const, + }, + }, + ] as const; + case "unknown": + return [ + pid, + { + state: "unavailable" as const, + reason: "native identity permission denied", + }, + ] as const; + default: + return [pid, live] as const; + } + }), + ), + signalGroup: vi.fn(), + }; + const result = await verifyNoTokenOwnedProcesses("run-token", host); + expect(tokenReads).toBe(4); + // A live process is not repeatedly scanned without a fixed upper bound. + expect(readsAfterLastToken).toBeLessThanOrEqual(6); + if (lastIdentity === "delayed-absent" || lastIdentity === "delayed-reused") + expect(readsAfterLastToken).toBe(5); + if (lastIdentity === "absent" || lastIdentity === "delayed-absent") + expect(result).toEqual({ cleaned: true, signaled: false }); + else + expect(result).toMatchObject({ + cleaned: false, + failures: [ + { + pid: 902, + reason: lastIdentity.includes("reused") + ? "process-identity-unavailable" + : "environment-unreadable", + }, + ], + }); + expect(host.signalGroup).not.toHaveBeenCalled(); + }, +); + +it.each(["absent", "live", "unavailable", "reused", "reused-zombie"] as const)( + "corroborates group token-open races without signaling %s", + async (outcome) => { + const row: ProcessTableEntry = { + pid: 910, + parentPid: 1, + processGroupId: 910, + state: "S", + command: "fixture", + }; + let tokenRead = false; + let postTokenReads = 0; + let absent = false; + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => (absent ? [] : [row]), + environment: async () => ({}), + runTokens: async (members) => { + tokenRead = true; + return new Map( + members.map(({ pid }) => [ + pid, + { + state: "unavailable" as const, + reason: "failure details without errno", + code: "ESRCH", + }, + ]), + ); + }, + processIdentities: async (members) => + new Map( + members.map(({ pid }) => { + const later = tokenRead && ++postTokenReads >= 3; + if (later && outcome === "absent") { + absent = true; + return [ + pid, + { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace" as const, + }, + ]; + } + if (later && outcome === "unavailable") { + return [ + pid, + { state: "unavailable" as const, reason: "identity denied" }, + ]; + } + return [ + pid, + { + state: "readable" as const, + identity: + later && outcome.startsWith("reused") + ? "replacement" + : "generation-910", + lifecycle: { + state: "observed" as const, + lifecycle: + later && outcome === "reused-zombie" + ? ("zombie" as const) + : ("non-zombie" as const), + }, + relation: { + state: "observed" as const, + parentPid: 1, + processGroupId: 910, + }, + }, + ]; + }), + ), + signalGroup, + }; + const result = await cleanupOwnedProcessGroup( + { runId: "run", leaderPid: 910, processGroupId: 910 }, + host, + ); + if (outcome === "absent") + expect(result).toEqual({ cleaned: true, signaled: false }); + else + expect(result).toMatchObject({ + cleaned: false, + failures: [ + { + pid: 910, + reason: outcome.startsWith("reused") + ? "process-identity-unavailable" + : "environment-unreadable", + }, + ], + }); + expect(signalGroup).not.toHaveBeenCalled(); + }, +); + +it("honors cancellation during token-open exit settlement", async () => { + const row: ProcessTableEntry = { + pid: 911, + parentPid: 1, + processGroupId: 911, + state: "S", + command: "fixture", + }; + const identity: ProcessIdentityObservation = { + state: "readable", + identity: "generation-911", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + }; + const controller = new AbortController(); + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => [row], + environment: async () => ({}), + signalGroup: vi.fn(), + processIdentities: async () => { + controller.abort(new Error("cancelled")); + return new Map([[row.pid, identity]]); + }, + }; + await expect( + settleVanishedTokenFacts( + [ + { + process: row, + state: "same-generation", + identityBefore: identity, + identityAfter: identity, + token: { state: "unavailable", reason: "vanished", code: "ESRCH" }, + }, + ], + host, + { deadline: undefined }, + controller.signal, + ), + ).rejects.toThrow("cancelled"); +}); + +it("does not signal a stale group after its last member exits during revalidation", async () => { + const row: ProcessTableEntry = { + pid: 912, + parentPid: 1, + processGroupId: 912, + state: "S", + command: "fixture", + }; + let tokenReads = 0; + let laterReads = 0; + let absent = false; + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => (absent ? [] : [row]), + environment: async () => ({}), + runTokens: async (members) => { + tokenReads += 1; + return new Map( + members.map(({ pid }) => [ + pid, + tokenReads === 1 + ? { state: "readable" as const, runId: "run" } + : { + state: "unavailable" as const, + reason: "failure details without errno", + code: "ESRCH", + }, + ]), + ); + }, + processIdentities: async (members) => + new Map( + members.map(({ pid }) => { + if (tokenReads >= 2 && ++laterReads >= 3) { + absent = true; + return [ + pid, + { state: "absent", reason: "not-found-in-caller-pid-namespace" }, + ]; + } + return [ + pid, + { + state: "readable", + identity: "generation-912", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: 1, + processGroupId: 912, + }, + }, + ]; + }), + ), + signalGroup, + }; + expect( + await cleanupOwnedProcessGroup( + { runId: "run", leaderPid: 912, processGroupId: 912 }, + host, + ), + ).toEqual({ cleaned: true, signaled: false }); + expect(tokenReads).toBe(2); + expect(signalGroup).not.toHaveBeenCalled(); +}); + +it("revalidates a group after its observed launcher exits and a child appears", async () => { + const launcher: ProcessTableEntry = { + pid: 920, + parentPid: 1, + processGroupId: 920, + state: "S", + command: "launcher", + }; + const child: ProcessTableEntry = { + pid: 921, + parentPid: 1, + processGroupId: 920, + state: "S", + command: "child", + }; + let exited = false; + let signaled = false; + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => (signaled ? [] : exited ? [child] : [launcher]), + environment: async (pid) => { + if (pid === launcher.pid) { + exited = true; + throw Object.assign(new Error("opaque exit"), { code: "ESRCH" }); + } + return { REA_PROCESS_RUN_ID: "run" }; + }, + processIdentities: async (rows) => + new Map( + rows.map(({ pid }) => [ + pid, + pid === launcher.pid && exited + ? { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace" as const, + } + : { + state: "readable" as const, + identity: `generation-${pid}`, + lifecycle: { + state: "observed" as const, + lifecycle: "non-zombie" as const, + }, + relation: { + state: "observed" as const, + parentPid: 1, + processGroupId: 920, + }, + }, + ]), + ), + signalGroup: vi.fn(() => { + signaled = true; + }), + }; + expect( + await cleanupOwnedProcessGroup( + { runId: "run", leaderPid: 920, processGroupId: 920 }, + host, + ), + ).toEqual({ cleaned: true, signaled: true }); + expect(host.signalGroup).toHaveBeenCalledWith(920, "SIGKILL"); +}); + +it("refreshes a group when its last member exits during revalidation and a child appears", async () => { + const launcher: ProcessTableEntry = { + pid: 940, + parentPid: 1, + processGroupId: 940, + state: "S", + command: "launcher", + }; + const child: ProcessTableEntry = { + pid: 941, + parentPid: 940, + processGroupId: 940, + state: "S", + command: "child", + }; + let planningTokenRead = false; + let phase: "planning" | "snapshot" | "after-snapshot" = "planning"; + let refreshFoundChild = false; + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => { + if (phase === "after-snapshot") { + refreshFoundChild = true; + return [child]; + } + // The snapshot under test is the table read after planning has already + // observed the launcher token. Earlier planning reads stay on the launcher. + if (planningTokenRead && phase === "planning") phase = "snapshot"; + return [launcher]; + }, + environment: async (pid) => { + if (pid === launcher.pid && phase === "snapshot") { + phase = "after-snapshot"; + throw Object.assign(new Error("opaque exit"), { code: "ESRCH" }); + } + if (pid === launcher.pid) planningTokenRead = true; + return { REA_PROCESS_RUN_ID: "run" }; + }, + processIdentities: async (rows) => + new Map( + rows.map(({ pid }) => + pid === launcher.pid && phase === "after-snapshot" + ? [ + pid, + { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace" as const, + }, + ] + : [ + pid, + { + state: "readable" as const, + identity: `generation-${pid}`, + lifecycle: { + state: "observed" as const, + lifecycle: "non-zombie" as const, + }, + relation: { + state: "observed" as const, + parentPid: pid === child.pid ? launcher.pid : 1, + processGroupId: 940, + }, + }, + ], + ), + ), + signalGroup, + }; + expect( + await cleanupOwnedProcessGroup( + { runId: "run", leaderPid: 940, processGroupId: 940 }, + host, + ), + ).toEqual({ cleaned: true, signaled: true }); + expect(refreshFoundChild).toBe(true); + expect(signalGroup).toHaveBeenCalledWith(940, "SIGKILL"); +}); + +it.each([ + { code: "ESRCH", message: "opaque failure", expected: "absent" }, + { + code: undefined, + message: "ESRCH: misleading text", + expected: "same-generation", + }, +] as const)( + "uses structured token error codes instead of message text ($message)", + async ({ code, message, expected }) => { + const row: ProcessTableEntry = { + pid: 930, + parentPid: 1, + processGroupId: 930, + state: "S", + command: "fixture", + }; + let tokenRead = false; + let observedAfterToken = false; + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => [row], + environment: async () => { + tokenRead = true; + throw Object.assign( + new Error(message), + code === undefined ? {} : { code }, + ); + }, + processIdentities: async (rows) => + new Map( + rows.map(({ pid }) => { + if (tokenRead && observedAfterToken) + return [ + pid, + { + state: "absent" as const, + reason: "not-found-in-caller-pid-namespace" as const, + }, + ]; + if (tokenRead) observedAfterToken = true; + return [ + pid, + { + state: "readable" as const, + identity: "generation-930", + lifecycle: { + state: "observed" as const, + lifecycle: "non-zombie" as const, + }, + relation: { + state: "observed" as const, + parentPid: 1, + processGroupId: 930, + }, + }, + ]; + }), + ), + signalGroup: vi.fn(), + }; + const [fact] = await readProcessOwnershipFacts([row], host); + expect(fact?.state).toBe(expected); + expect(fact?.token).toMatchObject({ + state: "unavailable", + reason: message, + ...(code === undefined ? {} : { code }), + }); + }, +); diff --git a/src/process/ProcessOwnership.ts b/src/process/ProcessOwnership.ts index e6f373ff83..cc9d838adc 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -8,6 +8,7 @@ import { readNativeProcessIdentityMap, readProcessOwnershipFacts, settleExitingProcessIdentities, + settleVanishedTokenFacts, systemProcessOwnershipHost as systemHost, } from "./ProcessOwnershipObservation.js"; import { launcherIdentityFailure } from "./ProcessOwnershipIdentity.js"; @@ -201,7 +202,11 @@ export interface ProcessOwnershipHost { /** Whether one live process exposed its ownership token to the host. */ export type ProcessRunTokenObservation = | { readonly state: "readable"; readonly runId: string | undefined } - | { readonly state: "unavailable"; readonly reason: string }; + | { + readonly state: "unavailable"; + readonly reason: string; + readonly code?: string; + }; /** Narrow Windows P0 seam for bounded process-tree termination. */ export interface WindowsProcessTreeHost { @@ -1045,7 +1050,7 @@ const createOwnedCleanupPlan = async ( observedGroupId === processGroupId, ); if (members.length === 0) continue; - const failures = await processOwnershipFailures( + const { failures, validatedMembers } = await processOwnershipFailures( members, ownership, host, @@ -1054,7 +1059,7 @@ const createOwnedCleanupPlan = async ( ); if (failures.length === 0 && unverifiableSampledGroups.has(processGroupId)) unresolved.push( - ...members.map(({ pid }) => ({ + ...validatedMembers.map(({ pid }) => ({ pid, reason: "process-identity-unavailable" as const, diagnostic: @@ -1134,55 +1139,69 @@ const revalidateOwnedProcessGroup = async ( } | ProcessCleanupResult > => { - let members: readonly ProcessTableEntry[]; - let escapedProcessGroupIds: readonly number[] = []; - try { - const settled = await settleLauncherExit( + const escapedProcessGroupIds = new Set(); + for (let refresh = 0; ; refresh += 1) { + let members: readonly ProcessTableEntry[]; + try { + const settled = await settleLauncherExit( + ownership, + liveProcesses(await host.listProcesses()), + host, + exitSettlement, + ); + if ("cleaned" in settled) return settled; + for (const groupId of settled.escapedProcessGroupIds) + escapedProcessGroupIds.add(groupId); + const relationSnapshot = await readProcessRelationSnapshot( + ownership, + settled.processes, + host, + ); + if (relationSnapshot.failures.length > 0) + return cleanupValidationFailure(relationSnapshot.failures); + members = relationSnapshot.currentRelations.filter( + ({ processGroupId: observedGroupId }) => + observedGroupId === processGroupId, + ); + } catch (cause: unknown) { + return { + cleaned: false, + reason: `process ownership could not be revalidated: ${errorMessage(cause)}`, + }; + } + const { failures, validatedMembers } = await processOwnershipFailures( + members, ownership, - liveProcesses(await host.listProcesses()), host, + expectedIdentities, exitSettlement, ); - if ("cleaned" in settled) return settled; - escapedProcessGroupIds = settled.escapedProcessGroupIds; - const processes = settled.processes; - const relationSnapshot = await readProcessRelationSnapshot( - ownership, - processes, - host, - ); - if (relationSnapshot.failures.length > 0) - return cleanupValidationFailure(relationSnapshot.failures); - members = relationSnapshot.currentRelations.filter( - ({ processGroupId: observedGroupId }) => - observedGroupId === processGroupId, - ); - } catch (cause: unknown) { - return { - cleaned: false, - reason: `process ownership could not be revalidated: ${errorMessage(cause)}`, - }; + if (failures.length > 0) return cleanupValidationFailure(failures); + if ( + processGroupId !== ownership.processGroupId && + validatedMembers.length > 0 && + !validatedMembers.some(({ pid }) => pid === processGroupId) && + ownership.sweepTokenOwnedProcesses !== true + ) + return { + cleaned: false, + reason: + "descendant process-group leader identity could not be revalidated", + }; + // Exit of every row in this table does not prove the group is empty. A + // same-group child can be forked after the read and during the token + // checks, so refresh before skipping the signal. + if (validatedMembers.length > 0 || members.length === 0) + return { + empty: validatedMembers.length === 0, + escapedProcessGroupIds: [...escapedProcessGroupIds], + }; + if (refresh >= OWNED_GROUP_REVALIDATION_LIMIT) + return { + cleaned: false, + reason: `owned process group ${String(processGroupId)} members exited during revalidation after ${String(OWNED_GROUP_REVALIDATION_LIMIT)} table refreshes. This release has no runtime override.`, + }; } - const failures = await processOwnershipFailures( - members, - ownership, - host, - expectedIdentities, - exitSettlement, - ); - if (failures.length > 0) return cleanupValidationFailure(failures); - if ( - processGroupId !== ownership.processGroupId && - members.length > 0 && - !members.some(({ pid }) => pid === processGroupId) && - ownership.sweepTokenOwnedProcesses !== true - ) - return { - cleaned: false, - reason: - "descendant process-group leader identity could not be revalidated", - }; - return { empty: members.length === 0, escapedProcessGroupIds }; }; const cleanupValidationFailure = ( @@ -1282,13 +1301,21 @@ const processOwnershipFailures = async ( host: ProcessOwnershipHost, expectedIdentities: ReadonlyMap = new Map(), exitSettlement: ProcessExitSettlementBudget = { deadline: undefined }, -): Promise => { +): Promise<{ + readonly failures: readonly ProcessOwnershipValidationFailure[]; + readonly validatedMembers: readonly ProcessTableEntry[]; +}> => { const runId = ownership.runId; if (host.processIdentities !== undefined) { - const facts = await readProcessOwnershipFacts(members, host, undefined, { + const facts = await settleVanishedTokenFacts( + await readProcessOwnershipFacts(members, host, undefined, { + exitSettlement, + }), + host, exitSettlement, - }); + ); const failures: ProcessOwnershipValidationFailure[] = []; + const validatedMembers: ProcessTableEntry[] = []; for (const fact of facts) { if (processIdentityHasExited(fact.identityAfter)) continue; if (processIdentityIsExiting(fact.identityAfter)) { @@ -1361,12 +1388,17 @@ const processOwnershipFailures = async ( }); } else if (fact.token.runId !== runId) { failures.push({ pid: fact.process.pid, reason: "run-token-mismatch" }); + } else { + validatedMembers.push(fact.process); } } - return failures; + return { failures, validatedMembers }; } - return processRunTokenFailures(members, runId, host); + return { + failures: await processRunTokenFailures(members, runId, host), + validatedMembers: members, + }; }; const processRunTokenFailures = async ( @@ -1675,6 +1707,9 @@ const scanWithOwnershipEvidence = async ( facts = facts.map((fact) => retryByPid.get(fact.process.pid) ?? fact); } + // Recheck final token-open races at the shared native identity boundary. + facts = await settleVanishedTokenFacts(facts, host, exitSettlement); + const processGroupIds = new Set([ ...(relation === undefined ? [] : [relation.processGroupId]), ...(relation?.sampledProcessGroupIds ?? []), diff --git a/src/process/ProcessOwnershipObservation.ts b/src/process/ProcessOwnershipObservation.ts index d0180ae6fe..d47e501936 100644 --- a/src/process/ProcessOwnershipObservation.ts +++ b/src/process/ProcessOwnershipObservation.ts @@ -832,6 +832,77 @@ export const settleExitingProcessIdentities = async ( return identities; }; +/** Corroborate Linux token-open exit races without granting signal authority. */ +export const settleVanishedTokenFacts = async ( + facts: readonly ProcessOwnershipFact[], + host: ProcessOwnershipHost, + budget: ProcessExitSettlementBudget, + signal?: AbortSignal, +): Promise => { + if ((host.platform ?? process.platform) !== "linux") return facts; + let pending = facts + .filter( + ({ token, identityAfter }) => + token.state === "unavailable" && + (token.code === "ENOENT" || token.code === "ESRCH") && + !processIdentityHasExited(identityAfter), + ) + .map(({ process }) => process); + if (pending.length === 0) return facts; + let current = facts; + const deadline = (budget.deadline ??= + performance.now() + PROCESS_CLEANUP_VERIFICATION_GRACE_MS); + for (let attempt = 0; attempt < 4 && pending.length > 0; attempt += 1) { + signal?.throwIfAborted(); + if (attempt > 0) { + const remaining = deadline - performance.now(); + if (remaining <= 0) break; + await delay( + Math.min(40, remaining), + undefined, + signal === undefined ? undefined : { signal }, + ); + } + const confirmed = await readNativeProcessIdentityMap(pending, host, signal); + const pendingPids = new Set(pending.map(({ pid }) => pid)); + const nextPending = new Set(); + current = current.map((fact) => { + if (!pendingPids.has(fact.process.pid)) return fact; + const after = confirmed.get(fact.process.pid); + if (after?.state === "absent") + return { + ...fact, + state: "absent" as const, + identityAfter: after, + beforeExitSettlement: fact.beforeExitSettlement ?? fact, + }; + const previous = fact.identityAfter; + if (previous.state !== "readable" || after?.state !== "readable") + return fact; + if (previous.identity !== after.identity) + return { + ...fact, + state: "unavailable" as const, + identityAfter: { + state: "unavailable" as const, + reason: "process_identity_changed_during_final_token_exit_check", + }, + }; + if (processIdentityHasExited(after)) + return { + ...fact, + identityAfter: after, + beforeExitSettlement: fact.beforeExitSettlement ?? fact, + }; + nextPending.add(fact.process.pid); + return fact; + }); + pending = pending.filter(({ pid }) => nextPending.has(pid)); + } + signal?.throwIfAborted(); + return current; +}; + /** * Read identity, token, then identity for caller-provided rows. This owner * never enumerates the process table; a token is tied to a generation only @@ -889,10 +960,56 @@ export const readProcessOwnershipFacts = async ( signal, credentialPids, ); + // An ENOENT/ESRCH environment failure can race with process exit while + // /proc/PID/stat is still readable. Do not treat the token error itself as + // proof of exit: ask the native identity owner again, which corroborates a + // missing procfs entry with signal-zero ESRCH. Keep the previous live + // observation if the refresh is unreadable; PID reuse remains a mismatch. + let finalObservedAfter = observedAfter; + if ((host.platform ?? process.platform) === "linux") { + const disappearing = processes.filter(({ pid }) => { + const token = tokens.get(pid); + return ( + token?.state === "unavailable" && + (token.code === "ENOENT" || token.code === "ESRCH") && + observedAfter.get(pid)?.state === "readable" + ); + }); + if (disappearing.length > 0) { + const refreshed = await readIdentityFacts( + disappearing, + host, + "after", + signal, + credentialPids, + ); + const confirmed = new Map(observedAfter); + for (const { pid } of disappearing) { + const before = observedAfter.get(pid); + const after = refreshed.get(pid); + if (after?.state === "absent") { + confirmed.set(pid, after); + } else if ( + before?.state === "readable" && + after?.state === "readable" + ) { + // A recycled PID (including a zombie replacement) is not proof of + // the original process exit. Keep its ownership unresolved. + if (before.identity !== after.identity) + confirmed.set(pid, { + state: "unavailable", + reason: "process_identity_changed_during_environment_exit_check", + }); + else if (processIdentityHasExited(after)) confirmed.set(pid, after); + } + } + finalObservedAfter = confirmed; + } + } const identityAfter = await settleExitingProcessIdentities( processes, host, - observedAfter, + finalObservedAfter, signal, options.exitSettlement, credentialPids, diff --git a/src/process/ProcessRunTokenObservations.ts b/src/process/ProcessRunTokenObservations.ts index 893a113413..e220eda1cb 100644 --- a/src/process/ProcessRunTokenObservations.ts +++ b/src/process/ProcessRunTokenObservations.ts @@ -68,8 +68,15 @@ export async function* readProcessRunTokens( } catch (cause: unknown) { signal?.throwIfAborted(); const reason = errorMessage(cause); + const code = + cause instanceof Error && + "code" in cause && + typeof cause.code === "string" + ? cause.code + : undefined; observation = { state: "unavailable", + ...(code === undefined ? {} : { code }), reason: batchFailure === undefined ? reason diff --git a/tests/boundary/cli/bundleComparisonCli.test.ts b/tests/boundary/cli/bundleComparisonCli.test.ts index f5b97810ab..f7a2305a84 100644 --- a/tests/boundary/cli/bundleComparisonCli.test.ts +++ b/tests/boundary/cli/bundleComparisonCli.test.ts @@ -12,6 +12,7 @@ import { createEvidenceBundle, serializeEvidenceBundle, } from "../../../src/domain/evidenceBundle.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; @@ -219,11 +220,20 @@ cliTest( name: "compare_bundles", arguments: { left_bundle_path: left, right_bundle_path: right }, }); - expect(result.isError).toBe(true); - expect(result.content[0]).toMatchObject({ - text: expect.stringContaining( - `${rejectedField}: must be an absolute local filesystem path`, - ), + expect(parseMcpToolError(result).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + details: { + operation: "compare_bundles", + issues: expect.arrayContaining([ + { + path: [rejectedField], + reason: "invalid_format", + expected: "regex", + message: "must be an absolute local filesystem path", + }, + ]), + }, }); } diff --git a/tests/boundary/mcp/bundleComparisonMcp.test.ts b/tests/boundary/mcp/bundleComparisonMcp.test.ts index b80c9ccd9c..5ecb6745d8 100644 --- a/tests/boundary/mcp/bundleComparisonMcp.test.ts +++ b/tests/boundary/mcp/bundleComparisonMcp.test.ts @@ -164,11 +164,17 @@ it("requires absolute bundle paths without changing server-cwd semantics", async expect(parseMcpToolError(result).error).toMatchObject({ code: "invalid_request", category: "invalid_input", - }); - expect(result.content[0]).toMatchObject({ - text: expect.stringContaining( - `${rejectedField}: must be an absolute local filesystem path`, - ), + details: { + operation: "compare_bundles", + issues: expect.arrayContaining([ + { + path: [rejectedField], + reason: "invalid_format", + expected: "regex", + message: "must be an absolute local filesystem path", + }, + ]), + }, }); } expect( diff --git a/tests/boundary/mcp/flutterMcp.test.ts b/tests/boundary/mcp/flutterMcp.test.ts index bc9b6a078c..bbad29fea5 100644 --- a/tests/boundary/mcp/flutterMcp.test.ts +++ b/tests/boundary/mcp/flutterMcp.test.ts @@ -14,6 +14,7 @@ import { flutterResultSchemas } from "../../../src/domain/flutter/flutterBuildAn import { aotFixtureImage } from "../../../src/flutter/FlutterBuild.fixture.js"; import { createFlutterApk } from "../../fixtures/flutterApk.js"; import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; it.each([ { label: "ARM64-V8A", paired: false }, @@ -255,12 +256,13 @@ it("advertises and enforces the same Unicode selector grammar through MCP", asyn name: "inspect_dart_aot", arguments: arguments_, }); - expect(refused.isError).toBe(true); - expect(refused.content).toContainEqual({ - type: "text", - text: expect.stringContaining( - "abi: ABI must be the exact lib/ directory label", - ), + expect(parseMcpToolError(refused).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + details: { + operation: "inspect_dart_aot", + issues: [{ path: ["abi"] }], + }, }); } });