morluto/rea

Fork 0

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

[PR #4143] feat(windows): capture selected windows through owned helpers #6082

New issue

Open

opened 2026-10-31 01:11:43 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4143
Author: @N0zoM1z0
Created: 10/15/2026
Status: 🔄 Open

Base: main ← Head: feat/windows-window-capture-2183

📝 Commits (5)

61b6408 feat(windows): capture selected windows with bounded owned helpers
bb9df94 fix(windows): preserve capture input, environment and frame bounds
62a20ff fix(windows): validate screenshot headers with the canonical PNG parser
dcccb4f fix(windows): admit only the capture facet inside the worker
283292e fix(windows): preserve unresolved readiness and cleanup ownership

📊 Changes

53 files changed (+4608 additions, -179 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/references/native-and-artifacts.md (+12 -0)
📝 .github/workflows/ci.yml (+10 -0)
📝 docs/native-investigation.md (+53 -0)
📝 docs/testing.md (+16 -0)
📝 knip.json (+1 -0)
📝 native/windows/README.md (+23 -0)
📝 native/windows/src/addon.cc (+20 -2)
📝 native/windows/src/authority.hpp (+56 -5)
📝 native/windows/src/filesystem.cc (+3 -13)
📝 native/windows/src/process.cc (+9 -0)
➕ native/windows/src/window_capture.cc (+662 -0)
📝 scripts/build-windows-native.mjs (+12 -1)
📝 scripts/bundle-runtime.mjs (+10 -1)
📝 scripts/verify-windows-native.mjs (+1 -0)
➕ scripts/verify-windows-window-capture.mjs (+614 -0)
📝 src/application/CapabilityInventory.ts (+25 -0)
➕ src/application/CapabilityInventory.windowsWindowCapture.test.ts (+56 -0)
📝 src/application/binary/BinarySessionStatus.ts (+3 -0)
📝 src/cli/utilityCommands.ts (+48 -0)
📝 src/cliCommandNames.ts (+1 -0)

...and 33 more files

📄 Description

Summary

Fixes #2183. Continues the implementation from #2199 by @SoGeRiEn, with real Windows validation and corrections to rendering, resource ownership and delivery.

Problem and expected behavior

The original raw SendMessageTimeout(WM_PRINT) cross-process HDC was invalid in the target: the fixture observed GetObjectType=0, FillRect=0, Win32 error 6. It returned a valid all-black PNG and DPI-scaled dimensions. The new workflow must return pixels from the exact selected PE application's client area, preserve identity and fail within a bounded helper lifetime.

Change and scope

Use PrintWindow(PW_CLIENTONLY) inside one owned Windows Job Object helper, with a 5 s deadline, 64 MiB JavaScript heap and 256 MiB job commit limit. Reuse process supervision and retained cleanup owners.
Preserve exact PID/HWND, process start identity and executable file identity/SHA-256 checks. Keep DPI changes thread-local. Use copied ANSI caption length and preserve the fixed-stream capacity reason when WIC wraps it.
Return a retained metadata summary by default over MCP; explicit image views reuse that frame. CLI and detail: complete return full Evidence. Captures remain live; stable native admission is cached, frames are not reused for a new capture.
Bound source pixels, PNG bytes and JSON/base64 expansion before allocation. Add a real Windows GUI acceptance lane to CI.

Maintainer scope decision: @N0zoM1z0, the human owner and maintainer, explicitly selected this narrow feature from #2183 and authorized implementation, testing, PR and merge. This PR records that decision and the declared target-paint effects. No desktop selection, window enumeration, input, target launch or elevation is exposed.

Contract and boundary impact

Owner: Windows provider adapter and shared CLI/MCP Evidence contracts. New tool: capture_windows_window_screenshot; retained image view: windows-window / screenshot.
Windows x64 only, PE application target. macOS native UI contracts remain separate. Packaged worker bundles its schema dependency to reduce cold helper startup.
PrintWindow can execute existing target paint code and cause target UI/filesystem/network effects; effect metadata and limitations report this. Only the owned helper is terminated on timeout/cancellation. Failed cleanup keeps its owner for retry.
Generated catalogs and skills are regenerated from authored inputs; no binaries, screenshots, profiles, credentials or provider registration information are committed.

Evidence and regression coverage

Executed on a real Windows x64 interactive desktop, including a non-elevated, non-administrator token:

Source-owned GUI: independently compare decoded pixels with expected blue/green frames; verify physical source dimensions, fresh repaint, immutable retained frames, CLI/MCP contracts, ANSI captions, wrong PID, destroyed HWND, identical bytes in a different file, source/PNG limits, smaller-image retry, written-RPC cancellation/recovery and zero runtime leftovers.
Windows Character Map: exact owned PID/HWND, non-empty PNG, matching observed client dimensions, both public transports and unchanged executable digest. Observed MCP calls took about 0.75–1.28 s; cold one-shot CLI calls took 13.7–19.9 s including CLI/process startup.
Noisy 512-pixel frame: 793,362 PNG bytes; default MCP summary 8,421 bytes versus 2,123,115 bytes for explicit image delivery (about 252× less wire data).

Coverage limits: the new post-render bounds check refuses resizing, but the resize race was not deterministically forced locally. These runs do not prove complete rendering for GPU/protected/minimized/elevated windows or physical-display contents. ANSI capture passed; this host did not reproduce the documented DBCS length overestimate. Cancellation verifies a request written to the transport, not an observed blocked paint handler. Fresh Windows npm installs timed out during setup; the local packaged-runtime runs used unchanged packed CLI/MCP/helper/native leaves with reused dependencies whose versions match the lockfile. They are not fresh-install verification. CI separately exercises the complete built checkout and native artifact.

Observed noisy-frame MCP summary excerpt (other Evidence fields omitted):

{"normalized_result":{"kind":"summary","summary":{"screenshot":{"width":512,"height":512,"bytes":793362}}}}

Reading its parent_evidence_id with inspect_analysis_view and view: {"kind":"windows-window","facet":"screenshot"} returns the retained PNG without another capture.

Review follow-up and CI

bb9df948c sends a bounded UTF-8 JSON request over supervised stdin, passes the provider's selected environment, requires native protocol-stdin support, and refuses dimensions changed during rendering. Regressions cover a request larger than the Windows command-line limit, real input delivery, blocked-reader timeout, early-close cleanup and the complete provider environment path. The final packed runtime/native artifact passed the non-admin Windows matrix again.

First CI run: Windows GUI/package and all other selected lanes passed. Source tests failed because this PR omitted the new tool from the destructive-effects audit's expected list; the Portable tests aggregate reported that same failure. The list is corrected and its six checks pass locally. The corrected 62a20fff1 CI run passed every selected lane. Current-head CI is rerunning after the additional worker-admission fix below.

62a20fff1 removes the duplicate PNG-header interpretation and uses readPngHeaderDimensions. Both malformed IHDR-length and high-bit chunk-type cases were accepted before the fix and rejected afterward; 40 focused parser/provider/delivery checks pass.

dcccb4fca keeps full filesystem/process admission in the parent and validates only the artifact/capture facet inside the supervised worker. A real Windows TEMP/TMP-is-a-file case failed before this change and passes afterward; the new case is in the Windows CI verifier. The complete non-admin GUI/CLI/MCP/Character Map matrix passed with the new packed runtime. One earlier Character Map CLI attempt failed before exit diagnostics were retained; an unchanged-artifact retry with added diagnostics passed. Its cause remains unknown and is not classified as an infrastructure failure or a fixed product bug.

283292edd preserves failed readiness even when retryable, and reports session-close cleanup under combined selected-window resources. Both regressions failed before the change; 54 focused provider/loader/helper/lazy-client checks pass afterward. Capability declarations remain snapshots: a fresh declaration reports availability only after actual admission succeeds.

Validation performed

npm run check:fast — passed on the updated main base.
npm run test:local -- eight relevant source files — 127 passed initially; 53 focused follow-up checks passed (overlapping existing cases).
npm run test:focused -- CLI/MCP/runtime bundle boundaries — 5 passed; follow-up stdin/MCP checks passed 20 tests (18 owned-command, 2 MCP).
Maximum duplicated MCP response including escaped title/path metadata — passed.
npm run build:windows-native and npm run verify:windows-native:artifact — passed.
npm run docs:check — passed, including 5 README consistency tests.
npm run verify:test-discovery — 1,355 files discovered exactly once.
npm run knip, changed-file formatting and git diff --check — passed.
Real CLI/MCP GUI acceptance described above — passed against the final packed runtime/native artifact; the Windows CI lane also exercises the complete built checkout.

Compatibility, safety, and release

Additive capability; no migration. Output limits: 4,194,304 source pixels, maximum dimension 4,096 (default 2,048), retained PNG 3 MiB. Incomplete painting remains an explicit limitation. Screenshots and titles can contain application data and are retained through existing Evidence/session semantics. Local measurements use bounded CPU affinity and helper memory controls.

Author readiness

Human owner: @N0zoM1z0

The owner explicitly authorized recording these declarations on their behalf for this work.

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.

_{🔄 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/4143 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/15/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/windows-window-capture-2183` --- ### 📝 Commits (5) - [`61b6408`](https://github.com/morluto/rea/commit/61b6408bb7da9668d249bba241822573d1bf3764) feat(windows): capture selected windows with bounded owned helpers - [`bb9df94`](https://github.com/morluto/rea/commit/bb9df948ccce8e2b465388b51d3e491ce25ef3be) fix(windows): preserve capture input, environment and frame bounds - [`62a20ff`](https://github.com/morluto/rea/commit/62a20fff19fafc69eb896c74c35b590c3c3fed5c) fix(windows): validate screenshot headers with the canonical PNG parser - [`dcccb4f`](https://github.com/morluto/rea/commit/dcccb4fca780a1522cdefec1c059a199a2c37694) fix(windows): admit only the capture facet inside the worker - [`283292e`](https://github.com/morluto/rea/commit/283292eddac84ea5d4a044d44bac42df96ac4a4d) fix(windows): preserve unresolved readiness and cleanup ownership ### 📊 Changes **53 files changed** (+4608 additions, -179 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/references/native-and-artifacts.md` (+12 -0) 📝 `.github/workflows/ci.yml` (+10 -0) 📝 `docs/native-investigation.md` (+53 -0) 📝 `docs/testing.md` (+16 -0) 📝 `knip.json` (+1 -0) 📝 `native/windows/README.md` (+23 -0) 📝 `native/windows/src/addon.cc` (+20 -2) 📝 `native/windows/src/authority.hpp` (+56 -5) 📝 `native/windows/src/filesystem.cc` (+3 -13) 📝 `native/windows/src/process.cc` (+9 -0) ➕ `native/windows/src/window_capture.cc` (+662 -0) 📝 `scripts/build-windows-native.mjs` (+12 -1) 📝 `scripts/bundle-runtime.mjs` (+10 -1) 📝 `scripts/verify-windows-native.mjs` (+1 -0) ➕ `scripts/verify-windows-window-capture.mjs` (+614 -0) 📝 `src/application/CapabilityInventory.ts` (+25 -0) ➕ `src/application/CapabilityInventory.windowsWindowCapture.test.ts` (+56 -0) 📝 `src/application/binary/BinarySessionStatus.ts` (+3 -0) 📝 `src/cli/utilityCommands.ts` (+48 -0) 📝 `src/cliCommandNames.ts` (+1 -0) _...and 33 more files_ </details> ### 📄 Description ## Summary Fixes #2183. Continues the implementation from #2199 by @SoGeRiEn, with real Windows validation and corrections to rendering, resource ownership and delivery. ## Problem and expected behavior The original raw `SendMessageTimeout(WM_PRINT)` cross-process HDC was invalid in the target: the fixture observed `GetObjectType=0`, `FillRect=0`, Win32 error 6. It returned a valid all-black PNG and DPI-scaled dimensions. The new workflow must return pixels from the exact selected PE application's client area, preserve identity and fail within a bounded helper lifetime. ## Change and scope - Use `PrintWindow(PW_CLIENTONLY)` inside one owned Windows Job Object helper, with a 5 s deadline, 64 MiB JavaScript heap and 256 MiB job commit limit. Reuse process supervision and retained cleanup owners. - Preserve exact PID/HWND, process start identity and executable file identity/SHA-256 checks. Keep DPI changes thread-local. Use copied ANSI caption length and preserve the fixed-stream capacity reason when WIC wraps it. - Return a retained metadata summary by default over MCP; explicit image views reuse that frame. CLI and `detail: complete` return full Evidence. Captures remain live; stable native admission is cached, frames are not reused for a new capture. - Bound source pixels, PNG bytes and JSON/base64 expansion before allocation. Add a real Windows GUI acceptance lane to CI. Maintainer scope decision: @N0zoM1z0, the human owner and maintainer, explicitly selected this narrow feature from #2183 and authorized implementation, testing, PR and merge. This PR records that decision and the declared target-paint effects. No desktop selection, window enumeration, input, target launch or elevation is exposed. ## Contract and boundary impact - Owner: Windows provider adapter and shared CLI/MCP Evidence contracts. New tool: `capture_windows_window_screenshot`; retained image view: `windows-window` / `screenshot`. - Windows x64 only, PE application target. macOS native UI contracts remain separate. Packaged worker bundles its schema dependency to reduce cold helper startup. - `PrintWindow` can execute existing target paint code and cause target UI/filesystem/network effects; effect metadata and limitations report this. Only the owned helper is terminated on timeout/cancellation. Failed cleanup keeps its owner for retry. - Generated catalogs and skills are regenerated from authored inputs; no binaries, screenshots, profiles, credentials or provider registration information are committed. ## Evidence and regression coverage Executed on a real Windows x64 interactive desktop, including a non-elevated, non-administrator token: - Source-owned GUI: independently compare decoded pixels with expected blue/green frames; verify physical source dimensions, fresh repaint, immutable retained frames, CLI/MCP contracts, ANSI captions, wrong PID, destroyed HWND, identical bytes in a different file, source/PNG limits, smaller-image retry, written-RPC cancellation/recovery and zero runtime leftovers. - Windows Character Map: exact owned PID/HWND, non-empty PNG, matching observed client dimensions, both public transports and unchanged executable digest. Observed MCP calls took about 0.75–1.28 s; cold one-shot CLI calls took 13.7–19.9 s including CLI/process startup. - Noisy 512-pixel frame: 793,362 PNG bytes; default MCP summary 8,421 bytes versus 2,123,115 bytes for explicit image delivery (about 252× less wire data). Coverage limits: the new post-render bounds check refuses resizing, but the resize race was not deterministically forced locally. These runs do not prove complete rendering for GPU/protected/minimized/elevated windows or physical-display contents. ANSI capture passed; this host did not reproduce the documented DBCS length overestimate. Cancellation verifies a request written to the transport, not an observed blocked paint handler. Fresh Windows npm installs timed out during setup; the local packaged-runtime runs used unchanged packed CLI/MCP/helper/native leaves with reused dependencies whose versions match the lockfile. They are not fresh-install verification. CI separately exercises the complete built checkout and native artifact. Observed noisy-frame MCP summary excerpt (other Evidence fields omitted): ```json {"normalized_result":{"kind":"summary","summary":{"screenshot":{"width":512,"height":512,"bytes":793362}}}} ``` Reading its `parent_evidence_id` with `inspect_analysis_view` and `view: {"kind":"windows-window","facet":"screenshot"}` returns the retained PNG without another capture. ## Review follow-up and CI `bb9df948c` sends a bounded UTF-8 JSON request over supervised stdin, passes the provider's selected environment, requires native protocol-stdin support, and refuses dimensions changed during rendering. Regressions cover a request larger than the Windows command-line limit, real input delivery, blocked-reader timeout, early-close cleanup and the complete provider environment path. The final packed runtime/native artifact passed the non-admin Windows matrix again. First CI run: Windows GUI/package and all other selected lanes passed. Source tests failed because this PR omitted the new tool from the destructive-effects audit's expected list; the Portable tests aggregate reported that same failure. The list is corrected and its six checks pass locally. The corrected `62a20fff1` CI run passed every selected lane. Current-head CI is rerunning after the additional worker-admission fix below. `62a20fff1` removes the duplicate PNG-header interpretation and uses `readPngHeaderDimensions`. Both malformed IHDR-length and high-bit chunk-type cases were accepted before the fix and rejected afterward; 40 focused parser/provider/delivery checks pass. `dcccb4fca` keeps full filesystem/process admission in the parent and validates only the artifact/capture facet inside the supervised worker. A real Windows TEMP/TMP-is-a-file case failed before this change and passes afterward; the new case is in the Windows CI verifier. The complete non-admin GUI/CLI/MCP/Character Map matrix passed with the new packed runtime. One earlier Character Map CLI attempt failed before exit diagnostics were retained; an unchanged-artifact retry with added diagnostics passed. Its cause remains unknown and is not classified as an infrastructure failure or a fixed product bug. `283292edd` preserves failed readiness even when retryable, and reports session-close cleanup under combined selected-window resources. Both regressions failed before the change; 54 focused provider/loader/helper/lazy-client checks pass afterward. Capability declarations remain snapshots: a fresh declaration reports availability only after actual admission succeeds. ## Validation performed - `npm run check:fast` — passed on the updated main base. - `npm run test:local --` eight relevant source files — 127 passed initially; 53 focused follow-up checks passed (overlapping existing cases). - `npm run test:focused --` CLI/MCP/runtime bundle boundaries — 5 passed; follow-up stdin/MCP checks passed 20 tests (18 owned-command, 2 MCP). - Maximum duplicated MCP response including escaped title/path metadata — passed. - `npm run build:windows-native` and `npm run verify:windows-native:artifact` — passed. - `npm run docs:check` — passed, including 5 README consistency tests. - `npm run verify:test-discovery` — 1,355 files discovered exactly once. - `npm run knip`, changed-file formatting and `git diff --check` — passed. - Real CLI/MCP GUI acceptance described above — passed against the final packed runtime/native artifact; the Windows CI lane also exercises the complete built checkout. ## Compatibility, safety, and release Additive capability; no migration. Output limits: 4,194,304 source pixels, maximum dimension 4,096 (default 2,048), retained PNG 3 MiB. Incomplete painting remains an explicit limitation. Screenshots and titles can contain application data and are retained through existing Evidence/session semantics. Local measurements use bounded CPU affinity and helper memory controls. ## Author readiness Human owner: @N0zoM1z0 The owner explicitly authorized recording these declarations on their behalf for this work. - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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 added the

pull-request

label

2026-10-31 01:11:43 +00:00

bsakaguchi referenced this issue

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

[PR #6149] [MERGED] fix(javascript): preserve source-map preparation capability errors #7164