[PR #620] [CLOSED] fix(process): derive provider detach from the configured platform #2121

New issue

Closed

opened 2026-10-16 21:14:35 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:14:35 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/620
Author: @aniruddhaadak80
Created: 10/5/2026
Status: ❌ Closed

Base: main ← Head: fix/process-detach-from-configured-platform

📝 Commits (1)

6df945e fix(process): derive provider detach from the configured platform

📊 Changes

3 files changed (+62 additions, -1 deletions)

View changed files

📝 src/ghidra/GhidraLauncher.ts (+1 -0)
📝 src/process/ProviderProcess.ts (+6 -1)
📝 tests/boundary/process/providerProcess.test.ts (+55 -0)

📄 Description

Summary

spawnOwnedProviderProcess decided whether to detach the child — and therefore whether a POSIX process group actually exists — from the ambient process.platform. GhidraHeadlessLauncher, which calls it, had already resolved a possibly-different configured platform and used that value for its command, windowsVerbatimArguments, environment, cleanup, and its ownership_kind manifest entry.

Two sources of truth for one fact. On a POSIX host configured for Windows, the manifest recorded posix-process-group with process_group_id equal to the launcher pid while no such group was ever created. REA's own token-verified seam cannot corroborate that claim, and ProcessOwnership.ts swallows the resulting ESRCH and still returns cleaned: true — so unverifiable cleanup reports as verified, which AGENTS.md's Boundary Contracts forbid.

Fixes #560

Problem and expected behavior

Smallest trigger: GhidraHeadlessLauncher configured with an explicit platform that differs from the host it runs on. The violated invariant: When a transformation loses information, preserve the reported value and an explicit unknown rather than guessing a canonical identity — the ownership manifest here asserts a process group that the spawn never created.

Observed before this change, driving the real launcher with platform: "linux" on a win32 host:

ownership.json: {"pid":20984,"process_group_id":20984,
                 "ownership_kind":"posix-process-group", ...}
ambient platform: win32          -> detached: false -> no group led by 20984 exists
observeOwnedProcessLineage: {"status":"unavailable",
                             "reason":"owned launcher is not live"}

The manifest claims a POSIX process group; the spawn created no group; the token-verified seam says it cannot confirm it.

Expected: the detach decision, the claimed group, and the ownership_kind manifest entry all derive from the same configured platform, so a manifest can never describe a group that was not created — and the token-verified observation reports the disagreement honestly instead of asserting a group that does not exist.

Change and scope

OwnedProviderProcessSpawnOptions gains an optional platform, defaulted with options.platform ?? process.platform — the same resolution idiom GhidraLauncher already uses at its own line 79. detached follows the resolved value. GhidraLauncher passes the platform it already resolved into spawnOwnedProviderProcess, so the spawn and the manifest are derived from one value.

The field is additive and optional: BridgeLauncher and LinuxPrivateDisplayProbe call sites omit it and keep ambient behavior, with no signature churn.

Not included: the broader half of #560. Six evidence builders stamp platform: process.platform, and that is correct — src/domain/browserObservation.ts:51-79 rejects every endpoint except literal loopback (127.0.0.1/[::1]), pinned by browserObservation.test.ts:39-46 rejecting http://192.168.1.2:9222 and http://localhost:9222. The producer is provably the same host, so those stamps are honest and are deliberately untouched. OwnedProcessGroup.processGroupId also stays a required number: making it optional to stop a win32-configured spawn from claiming pid would break OwnedProcessGroup and every consumer (cleanupOwnedProcessGroup, observeOwnedProcessGroupWithHost, selectCapturedProcessGroupIds, PlaywrightElectronActiveProvider, HopperCleanup), and the Windows path never reads it — cleanupStartedProcess calls cleanupWindowsProcessTree(leaderPid) there.

Contract and boundary impact

Semantic owner and earliest changed stage: src/process/ProviderProcess.ts — the owned-provider spawn step of the shared process lifecycle primitive, now parameterized by the caller's configured platform.
CLI and MCP/tool-catalog contract: unchanged. No tool added, removed, or reshaped; no schema change; TOOL_CONTRACTS and generated catalog untouched. rea doctor and capability inventory output is unchanged for correctly configured hosts.
Provider, bridge, target-format, or platform compatibility: unchanged for every real host/provider pair. Callers that omit the new field behave exactly as before. Only the previously-inconsistent case — an explicitly configured platform differing from the ambient host — changes, and it changes to the internally consistent answer.
Evidence, artifact, provenance, or reconstruction contract: unchanged for real deployments. The ownership manifest's ownership_kind and process_group_id now describe the process group that actually exists; the schema and field names are untouched.
Process execution, authorization, cleanup, or containment impact: yes, and this is the point. A win32-configured spawn on a POSIX host is now correctly left non-detached, so no POSIX group is claimed. Its ownership token's group claim consequently reports unavailable rather than a false verified. No cleanup path regresses: nothing calls cleanupOwnedProcessGroup on that path, and real host/provider pairs (Windows host with Windows target, Linux host with Linux target) behave identically to before.
Generated metadata (docs/product-catalog.json), package, or installation impact: none. No contract signature changed.

Evidence and regression coverage

Tests added or updated: tests/boundary/process/providerProcess.test.ts gains a provider process configured-platform spawning block using the file's existing real-spawn fixture pattern and observeOwnedProcessLineage. It spawns once configured win32 and once configured linux, then asserts the win32 spawn reports unavailable: "owned launcher process-group identity did not match" (no group exists) and the linux spawn reports status: "verified" with processGroupId equal to its own pid (it leads the group it claims). It is it.skipIf(process.platform === "win32"), matching the file's existing platform-gating convention.
Base reproduction or other evidence: the real GhidraHeadlessLauncher driven with platform: "linux" on this win32 host emitted ownership_kind: "posix-process-group" with process_group_id equal to the launcher pid while detached was false and observeOwnedProcessLineage returned unavailable: "owned launcher is not live" — REA's own seam contradicting the manifest it wrote.
User-visible CLI/MCP output: none. This corrects an internal ownership claim; no CLI or MCP surface changes.
Remaining proof gaps: the pre-fix state is only reachable on a POSIX host, and this machine is Windows-only (no WSL distro, no Docker). The new test therefore reports as skipped here and its assertions were not executed locally. CI runs test:ci:shard on ubuntu-latest, so the lane that proves it is Linux; see "Validation performed" for what was and was not confirmed on this host.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run check:fast → 2/2 tasks successful, 0 errors, 98 warnings (unchanged baseline)
npx vitest run --project process-boundary tests/boundary/process/providerProcess.test.ts → 1 failed, 14 passed, 4 skipped. The failure is allocates a private runtime root and removes it idempotently throwing PrivateRuntimeRootUnavailableError — this host has no native Windows DACL authority. Confirmed inherited by stashing all changes and re-running: 1 failed, 14 passed, 3 skipped. The 4th skip is the new platform-gated test.
Regression proof: stashing only src/process/ProviderProcess.ts and src/ghidra/GhidraLauncher.ts while keeping the test makes npm run check:fast exit 2 with error TS2353: ... 'platform' does not exist in type 'OwnedProviderProcessSpawnOptions' at the two new call sites — the test cannot compile, let alone pass, without the seam. The runtime assertion inversion could not be observed locally for the reason above and is left to the Linux CI lane.

Compatibility, safety, and release

Breaking changes or migration steps: none. The new option is optional and defaults to ambient behavior, so every existing call site is unaffected.
Real Hopper/Ghidra, browser, or OS coverage: none performed. No Ghidra installation and no POSIX host are available here, so npm run verify:hopper:linux and real-JDK Ghidra launches were not run. This change does not touch a provider's analysis path — it fixes how a launched child is placed in the process group its own manifest describes.
Package or release metadata impact: none. CHANGELOG.md is release-please generated from PR titles and is not edited by hand.
Security, privacy, process, or containment review: this change tightens containment rather than loosening it. A caller that explicitly configures a non-ambient platform previously got a manifest claiming cleanup authority it did not have; it now gets the truthful answer, which may be unavailable for group-based cleanup. For every real host/target pair the detach decision and the manifest agree exactly as before, so no real deployment changes behavior. No new privileges, no new I/O, no new authority.

Review checklist

The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/620 **Author:** [@aniruddhaadak80](https://github.com/aniruddhaadak80) **Created:** 10/5/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/process-detach-from-configured-platform` --- ### 📝 Commits (1) - [`6df945e`](https://github.com/morluto/rea/commit/6df945ef358a41eb6d75f37deea8d140c1f31cf4) fix(process): derive provider detach from the configured platform ### 📊 Changes **3 files changed** (+62 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/ghidra/GhidraLauncher.ts` (+1 -0) 📝 `src/process/ProviderProcess.ts` (+6 -1) 📝 `tests/boundary/process/providerProcess.test.ts` (+55 -0) </details> ### 📄 Description ## Summary `spawnOwnedProviderProcess` decided whether to detach the child — and therefore whether a POSIX process group actually exists — from the ambient `process.platform`. `GhidraHeadlessLauncher`, which calls it, had already resolved a possibly-different *configured* platform and used that value for its command, `windowsVerbatimArguments`, environment, cleanup, and its `ownership_kind` manifest entry. Two sources of truth for one fact. On a POSIX host configured for Windows, the manifest recorded `posix-process-group` with `process_group_id` equal to the launcher pid while no such group was ever created. REA's own token-verified seam cannot corroborate that claim, and `ProcessOwnership.ts` swallows the resulting `ESRCH` and still returns `cleaned: true` — so unverifiable cleanup reports as verified, which AGENTS.md's Boundary Contracts forbid. Fixes #560 ## Problem and expected behavior Smallest trigger: `GhidraHeadlessLauncher` configured with an explicit `platform` that differs from the host it runs on. The violated invariant: *When a transformation loses information, preserve the reported value and an explicit unknown rather than guessing a canonical identity* — the ownership manifest here asserts a process group that the spawn never created. Observed before this change, driving the real launcher with `platform: "linux"` on a `win32` host: ``` ownership.json: {"pid":20984,"process_group_id":20984, "ownership_kind":"posix-process-group", ...} ambient platform: win32 -> detached: false -> no group led by 20984 exists observeOwnedProcessLineage: {"status":"unavailable", "reason":"owned launcher is not live"} ``` The manifest claims a POSIX process group; the spawn created no group; the token-verified seam says it cannot confirm it. Expected: the detach decision, the claimed group, and the `ownership_kind` manifest entry all derive from the same configured platform, so a manifest can never describe a group that was not created — and the token-verified observation reports the disagreement honestly instead of asserting a group that does not exist. ## Change and scope `OwnedProviderProcessSpawnOptions` gains an optional `platform`, defaulted with `options.platform ?? process.platform` — the same resolution idiom `GhidraLauncher` already uses at its own line 79. `detached` follows the resolved value. `GhidraLauncher` passes the `platform` it already resolved into `spawnOwnedProviderProcess`, so the spawn and the manifest are derived from one value. The field is additive and optional: `BridgeLauncher` and `LinuxPrivateDisplayProbe` call sites omit it and keep ambient behavior, with no signature churn. Not included: the broader half of #560. Six evidence builders stamp `platform: process.platform`, and that is correct — `src/domain/browserObservation.ts:51-79` rejects every endpoint except literal loopback (`127.0.0.1`/`[::1]`), pinned by `browserObservation.test.ts:39-46` rejecting `http://192.168.1.2:9222` and `http://localhost:9222`. The producer is provably the same host, so those stamps are honest and are deliberately untouched. `OwnedProcessGroup.processGroupId` also stays a required `number`: making it optional to stop a `win32`-configured spawn from claiming `pid` would break `OwnedProcessGroup` and every consumer (`cleanupOwnedProcessGroup`, `observeOwnedProcessGroupWithHost`, `selectCapturedProcessGroupIds`, `PlaywrightElectronActiveProvider`, `HopperCleanup`), and the Windows path never reads it — `cleanupStartedProcess` calls `cleanupWindowsProcessTree(leaderPid)` there. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/process/ProviderProcess.ts` — the owned-provider spawn step of the shared process lifecycle primitive, now parameterized by the caller's configured platform. - CLI and MCP/tool-catalog contract: unchanged. No tool added, removed, or reshaped; no schema change; `TOOL_CONTRACTS` and generated catalog untouched. `rea doctor` and capability inventory output is unchanged for correctly configured hosts. - Provider, bridge, target-format, or platform compatibility: unchanged for every real host/provider pair. Callers that omit the new field behave exactly as before. Only the previously-inconsistent case — an explicitly configured platform differing from the ambient host — changes, and it changes to the internally consistent answer. - Evidence, artifact, provenance, or reconstruction contract: unchanged for real deployments. The ownership manifest's `ownership_kind` and `process_group_id` now describe the process group that actually exists; the schema and field names are untouched. - Process execution, authorization, cleanup, or containment impact: yes, and this is the point. A `win32`-configured spawn on a POSIX host is now correctly left non-detached, so no POSIX group is claimed. Its ownership token's group claim consequently reports `unavailable` rather than a false `verified`. No cleanup path regresses: nothing calls `cleanupOwnedProcessGroup` on that path, and real host/provider pairs (Windows host with Windows target, Linux host with Linux target) behave identically to before. - Generated metadata (`docs/product-catalog.json`), package, or installation impact: none. No contract signature changed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/process/providerProcess.test.ts` gains a `provider process configured-platform spawning` block using the file's existing real-spawn fixture pattern and `observeOwnedProcessLineage`. It spawns once configured `win32` and once configured `linux`, then asserts the `win32` spawn reports `unavailable: "owned launcher process-group identity did not match"` (no group exists) and the `linux` spawn reports `status: "verified"` with `processGroupId` equal to its own pid (it leads the group it claims). It is `it.skipIf(process.platform === "win32")`, matching the file's existing platform-gating convention. - Base reproduction or other evidence: the real `GhidraHeadlessLauncher` driven with `platform: "linux"` on this `win32` host emitted `ownership_kind: "posix-process-group"` with `process_group_id` equal to the launcher pid while `detached` was false and `observeOwnedProcessLineage` returned `unavailable: "owned launcher is not live"` — REA's own seam contradicting the manifest it wrote. - User-visible CLI/MCP output: none. This corrects an internal ownership claim; no CLI or MCP surface changes. - Remaining proof gaps: the pre-fix state is only reachable on a POSIX host, and this machine is Windows-only (no WSL distro, no Docker). The new test therefore reports as **skipped** here and its assertions were not executed locally. CI runs `test:ci:shard` on `ubuntu-latest`, so the lane that proves it is Linux; see "Validation performed" for what was and was not confirmed on this host. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run check:fast` → 2/2 tasks successful, 0 errors, 98 warnings (unchanged baseline) - `npx vitest run --project process-boundary tests/boundary/process/providerProcess.test.ts` → 1 failed, 14 passed, 4 skipped. The failure is `allocates a private runtime root and removes it idempotently` throwing `PrivateRuntimeRootUnavailableError` — this host has no native Windows DACL authority. Confirmed inherited by stashing all changes and re-running: 1 failed, 14 passed, 3 skipped. The 4th skip is the new platform-gated test. - Regression proof: stashing only `src/process/ProviderProcess.ts` and `src/ghidra/GhidraLauncher.ts` while keeping the test makes `npm run check:fast` exit 2 with `error TS2353: ... 'platform' does not exist in type 'OwnedProviderProcessSpawnOptions'` at the two new call sites — the test cannot compile, let alone pass, without the seam. The *runtime* assertion inversion could not be observed locally for the reason above and is left to the Linux CI lane. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The new option is optional and defaults to ambient behavior, so every existing call site is unaffected. - Real Hopper/Ghidra, browser, or OS coverage: none performed. No Ghidra installation and no POSIX host are available here, so `npm run verify:hopper:linux` and real-JDK Ghidra launches were not run. This change does not touch a provider's analysis path — it fixes how a launched child is placed in the process group its own manifest describes. - Package or release metadata impact: none. `CHANGELOG.md` is release-please generated from PR titles and is not edited by hand. - Security, privacy, process, or containment review: this change tightens containment rather than loosening it. A caller that explicitly configures a non-ambient platform previously got a manifest claiming cleanup authority it did not have; it now gets the truthful answer, which may be `unavailable` for group-based cleanup. For every real host/target pair the detach decision and the manifest agree exactly as before, so no real deployment changes behavior. No new privileges, no new I/O, no new authority. ## Review checklist - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:14:35 +00:00