morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4615] [CLOSED] fix(dev): run exclusive commands on Windows without the native addon #6332

New issue

Closed

opened 2026-10-31 01:13:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:13:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4615
Author: @starterio-dotcom
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: claude/zen-sammet-d3db51

📝 Commits (1)

4b2a2ea fix(dev): run exclusive commands on Windows without the native addon

📊 Changes

7 files changed (+269 additions, -56 deletions)

View changed files

📝 CONTRIBUTING.md (+23 -12)
📝 native/windows/README.md (+8 -0)
📝 scripts/lib/windows-owned-command.d.mts (+18 -1)
📝 scripts/lib/windows-owned-command.mjs (+44 -3)
📝 scripts/run-exclusive-command.mjs (+49 -28)
📝 tests/process-global/runnerCancellation.test.ts (+91 -9)
📝 tests/process-global/windowsOwnedCommand.test.ts (+36 -3)

📄 Description

Summary

On Windows source checkouts, scripts/run-exclusive.mjs exited 1 before running its command whenever native/windows/build/ was absent, so npm run build:cached and npm run test:focused failed with only the loader warning. The wrapper now asks the package-owned loader for its availability fact: with a verified authority the command keeps Job Object ownership exactly as before; without it, the wrapper prints the loader's reason plus an explicit leader-only-supervision notice and runs the command.

Problem and expected behavior

Trigger: any Windows checkout without the packaged addon (every fresh worktree, every checkout that never restored the artifact from the tarball, any unpublished package.json version, non-x64 Windows). scripts/run-exclusive-command.mjs went through loadWindowsOwnedProcessClass() and createWindowsOwnedCommand() from scripts/lib/windows-owned-command.mjs (since 1b97b0f3), which require the addon, and failed before the command started:

REA Windows native controls are unavailable: Missing packaged Windows native file native/windows/build/manifest.json (ENOENT). ...

Expected: the SQLite command lock and the command itself never depended on the addon; only process-tree ownership does. The build must run, say which guarantee is reduced, and keep the owned path unchanged when the addon is present.

Change and scope

scripts/lib/windows-owned-command.mjs: loadWindowsCommandOwnership() imports src/windows/WindowsNativeLoader.ts through one tsx namespace, returns the loader's tagged WindowsNativeLoadResult, and loads the owner class only alongside a verified authority. describeUnownedWindowsCommand() composes the diagnostic: the loader reason, then a statement that the command runs without Windows Job Object ownership, that the lock is released when the command leader exits, that cancellation stops only that leader, and the README restore anchor. Declarations added in the .d.mts.
scripts/run-exclusive-command.mjs: owned path unchanged (authority passed explicitly). Unavailable: diagnostic on stderr, then cross-spawn supervision with inherited stdio; IPC/disconnect cancellation terminates the leader and keeps the lock until its close event (the pre-1b97b0f3 Windows behavior). A failed source import still fails loudly.
CONTRIBUTING.md and native/windows/README.md describe both modes and the exact npm pack rea-agents@<matching-version> restore step.
Not included: no automatic download, no environment override, no change to provider workflows (they still report native controls unavailable), no CHANGELOG edit.

Fail-closed was rejected because the restore step cannot work for an unpublished version (the loader rejects any other version's artifact), offline, or on non-x64 Windows, and the MinGW build lane does not run on Windows.

Contract and boundary impact

Semantic owner and earliest changed stage: systemWindowsNativeAuthority() remains the owner of availability; the supervisor consumes its parsed fact at command start instead of letting the owner constructor throw.
Contribution scope fit and linked maintainer decision: not applicable (developer tooling within existing boundaries; no new provider or target capability).
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: Windows developer commands only; the POSIX path is untouched.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): with the addon, Job Object ownership and cleanup retry are unchanged; without it, the lock covers the command leader only and the wrapper states so on stderr.
Generated metadata, package, or installation impact: none; .d.mts declarations only.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: tests/process-global/runnerCancellation.test.ts gains two leader-only regressions (exit code and argument preservation with the exact stderr diagnostic; lock exclusivity plus IPC SIGTERM cancellation exiting 143 and releasing the lock) and asserts the exact supervisor stderr in the existing argument tests. Owned Windows scenarios there and in tests/process-global/windowsOwnedCommand.test.ts now gate on the loader fact instead of the platform, so hosts without the artifact report them as skipped. windowsOwnedCommand.test.ts adds availability and diagnostic tests for the lib. On the base revision the leader-only tests fail because the supervisor exits 1 before spawning.
Base reproduction or other evidence: executed on Windows 11 x64 without the addon. On the base revision, wrapping a trivial node -e command exited 1 with only the loader message; after the change it printed the diagnostic and the command output and exited 0, and a failing command's exit code 3 propagated.
User-visible CLI/MCP output (if applicable): stderr of a wrapped command without the addon:

REA Windows native controls are unavailable: Missing packaged Windows native file native/windows/build/manifest.json (ENOENT). Restore both native/windows/build/manifest.json and native/windows/build/rea-windows-x64.node from the rea-agents@6.5.0 tarball or build them with native/windows/README.md; no native artifact was loaded.
The artifacts command runs without Windows Job Object ownership: the command lock is released when the command leader exits, and cancellation stops only that leader, so descendants it leaves behind are not covered. Restore the artifact as described in native/windows/README.md#restore-the-artifact-in-a-source-checkout to reinstate owned supervision.

Remaining proof gaps: the owned path was not re-run locally (addon absent on this host); CI's windows-curated lane restores the artifact and runs both test files. Interactive Ctrl+C in a console without the addon was not exercised beyond the IPC cancellation test.

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 test:focused -- tests/process-global/runnerCancellation.test.ts tests/process-global/windowsOwnedCommand.test.ts (Windows 11 x64, Node 24.21.0, addon absent): 2 files passed, 12 tests passed, 9 skipped (4 POSIX-only, 5 owned Windows scenarios); the wrapper's own artifacts:skills build ran through the fallback with the diagnostic visible.
npm run check:fast: typecheck and lint passed.
npm run build:cached equivalent: exercised by the test:focused artifacts step above (3 turbo tasks successful).

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: Windows developer commands only; owned-path coverage relies on the CI Windows lane with the restored artifact.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the fallback never loads an unverified artifact (the loader still refuses mismatched files, and the wrapper relays its reason); reduced guarantees are stated on stderr rather than implied.

Author readiness

Human owner: @starterio-dotcom

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
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.

🤖 Generated with Claude Code

_{🔄 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/4615 **Author:** [@starterio-dotcom](https://github.com/starterio-dotcom) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `claude/zen-sammet-d3db51` --- ### 📝 Commits (1) - [`4b2a2ea`](https://github.com/morluto/rea/commit/4b2a2eaf9425c49d6550096ecec73ac4e92af917) fix(dev): run exclusive commands on Windows without the native addon ### 📊 Changes **7 files changed** (+269 additions, -56 deletions) <details> <summary>View changed files</summary> 📝 `CONTRIBUTING.md` (+23 -12) 📝 `native/windows/README.md` (+8 -0) 📝 `scripts/lib/windows-owned-command.d.mts` (+18 -1) 📝 `scripts/lib/windows-owned-command.mjs` (+44 -3) 📝 `scripts/run-exclusive-command.mjs` (+49 -28) 📝 `tests/process-global/runnerCancellation.test.ts` (+91 -9) 📝 `tests/process-global/windowsOwnedCommand.test.ts` (+36 -3) </details> ### 📄 Description ## Summary On Windows source checkouts, `scripts/run-exclusive.mjs` exited 1 before running its command whenever `native/windows/build/` was absent, so `npm run build:cached` and `npm run test:focused` failed with only the loader warning. The wrapper now asks the package-owned loader for its availability fact: with a verified authority the command keeps Job Object ownership exactly as before; without it, the wrapper prints the loader's reason plus an explicit leader-only-supervision notice and runs the command. ## Problem and expected behavior Trigger: any Windows checkout without the packaged addon (every fresh worktree, every checkout that never restored the artifact from the tarball, any unpublished `package.json` version, non-x64 Windows). `scripts/run-exclusive-command.mjs` went through `loadWindowsOwnedProcessClass()` and `createWindowsOwnedCommand()` from `scripts/lib/windows-owned-command.mjs` (since 1b97b0f3), which require the addon, and failed before the command started: ``` REA Windows native controls are unavailable: Missing packaged Windows native file native/windows/build/manifest.json (ENOENT). ... ``` Expected: the SQLite command lock and the command itself never depended on the addon; only process-tree ownership does. The build must run, say which guarantee is reduced, and keep the owned path unchanged when the addon is present. ## Change and scope - `scripts/lib/windows-owned-command.mjs`: `loadWindowsCommandOwnership()` imports `src/windows/WindowsNativeLoader.ts` through one tsx namespace, returns the loader's tagged `WindowsNativeLoadResult`, and loads the owner class only alongside a verified authority. `describeUnownedWindowsCommand()` composes the diagnostic: the loader reason, then a statement that the command runs without Windows Job Object ownership, that the lock is released when the command leader exits, that cancellation stops only that leader, and the README restore anchor. Declarations added in the `.d.mts`. - `scripts/run-exclusive-command.mjs`: owned path unchanged (authority passed explicitly). Unavailable: diagnostic on stderr, then cross-spawn supervision with inherited stdio; IPC/disconnect cancellation terminates the leader and keeps the lock until its close event (the pre-1b97b0f3 Windows behavior). A failed source import still fails loudly. - `CONTRIBUTING.md` and `native/windows/README.md` describe both modes and the exact `npm pack rea-agents@<matching-version>` restore step. - Not included: no automatic download, no environment override, no change to provider workflows (they still report native controls unavailable), no CHANGELOG edit. Fail-closed was rejected because the restore step cannot work for an unpublished version (the loader rejects any other version's artifact), offline, or on non-x64 Windows, and the MinGW build lane does not run on Windows. ## Contract and boundary impact - Semantic owner and earliest changed stage: `systemWindowsNativeAuthority()` remains the owner of availability; the supervisor consumes its parsed fact at command start instead of letting the owner constructor throw. - Contribution scope fit and linked maintainer decision: not applicable (developer tooling within existing boundaries; no new provider or target capability). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: Windows developer commands only; the POSIX path is untouched. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): with the addon, Job Object ownership and cleanup retry are unchanged; without it, the lock covers the command leader only and the wrapper states so on stderr. - Generated metadata, package, or installation impact: none; `.d.mts` declarations only. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/process-global/runnerCancellation.test.ts` gains two leader-only regressions (exit code and argument preservation with the exact stderr diagnostic; lock exclusivity plus IPC `SIGTERM` cancellation exiting 143 and releasing the lock) and asserts the exact supervisor stderr in the existing argument tests. Owned Windows scenarios there and in `tests/process-global/windowsOwnedCommand.test.ts` now gate on the loader fact instead of the platform, so hosts without the artifact report them as skipped. `windowsOwnedCommand.test.ts` adds availability and diagnostic tests for the lib. On the base revision the leader-only tests fail because the supervisor exits 1 before spawning. - Base reproduction or other evidence: executed on Windows 11 x64 without the addon. On the base revision, wrapping a trivial `node -e` command exited 1 with only the loader message; after the change it printed the diagnostic and the command output and exited 0, and a failing command's exit code 3 propagated. - User-visible CLI/MCP output (if applicable): stderr of a wrapped command without the addon: ``` REA Windows native controls are unavailable: Missing packaged Windows native file native/windows/build/manifest.json (ENOENT). Restore both native/windows/build/manifest.json and native/windows/build/rea-windows-x64.node from the rea-agents@6.5.0 tarball or build them with native/windows/README.md; no native artifact was loaded. The artifacts command runs without Windows Job Object ownership: the command lock is released when the command leader exits, and cancellation stops only that leader, so descendants it leaves behind are not covered. Restore the artifact as described in native/windows/README.md#restore-the-artifact-in-a-source-checkout to reinstate owned supervision. ``` - Remaining proof gaps: the owned path was not re-run locally (addon absent on this host); CI's `windows-curated` lane restores the artifact and runs both test files. Interactive Ctrl+C in a console without the addon was not exercised beyond the IPC cancellation test. 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 test:focused -- tests/process-global/runnerCancellation.test.ts tests/process-global/windowsOwnedCommand.test.ts` (Windows 11 x64, Node 24.21.0, addon absent): 2 files passed, 12 tests passed, 9 skipped (4 POSIX-only, 5 owned Windows scenarios); the wrapper's own `artifacts:skills` build ran through the fallback with the diagnostic visible. - `npm run check:fast`: typecheck and lint passed. - `npm run build:cached` equivalent: exercised by the `test:focused` artifacts step above (3 turbo tasks successful). ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Windows developer commands only; owned-path coverage relies on the CI Windows lane with the restored artifact. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the fallback never loads an unverified artifact (the loader still refuses mismatched files, and the wrapper relays its reason); reduced guarantees are stated on stderr rather than implied. ## Author readiness Human owner: @starterio-dotcom - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:13:39 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

2026-11-01 21:11:00 +00:00

[GH-ISSUE #5653] [Tracker] New issue intake by area (October 2026) #3423

bsakaguchi changed title from ~~[PR #4615] [CLOSED] fix(dev): run exclusive commands on Windows without the native addon~~ to [PR #4615] fix(dev): run exclusive commands on Windows without the native addon

2026-11-03 13:32:48 +00:00

bsakaguchi changed title from ~~[PR #4615] fix(dev): run exclusive commands on Windows without the native addon~~ to [PR #4615] [CLOSED] fix(dev): run exclusive commands on Windows without the native addon

2026-11-03 14:34:34 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6332

No description provided.

Rows
Columns