[PR #5430] [CLOSED] fix(native): declare filesystem write effect for inspect_signature #6928

Closed
opened 2026-11-01 21:44:07 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5430
Author: @prashantpiyush1111
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/native-signature-filesystem-effects


📝 Commits (5)

  • 12c656f fix(native): declare filesystem write effect for inspect_signature
  • 50100e5 fix(native): declare filesystem write effect for inspect_signature
  • 5217324 test(native): cover signature filesystem write effect
  • e7e80ec test(native): cover signature filesystem capability
  • 83346d0 test(native): cover signature filesystem write effect

📊 Changes

4 files changed (+26 additions, -2 deletions)

View changed files

📝 src/contracts/toolEffects.test.ts (+6 -0)
📝 src/contracts/toolEffects.ts (+5 -1)
➕ src/native/NativeMacOSProviderMetadata.test.ts (+13 -0)
📝 src/native/NativeMacOSProviderMetadata.ts (+2 -1)

📄 Description

Summary

Fixes #5423.

Correct discovery metadata for inspect_signature: the operation already creates and writes a private temporary snapshot, but its capability descriptor and canonical tool effects omit that filesystem effect.

Problem and expected behavior

Capability discovery currently omits may_write_filesystem for inspect_signature, and TOOL_EFFECTS.inspect_signature.writesFilesystem is false. Discovery should report both effects truthfully. The existing failure when the temporary parent is unwritable remains valid; this PR does not alter error classification or make inspection succeed in that environment.

Change and scope

  • Add inspect_signature to the native capability's filesystem-write declaration.
  • Give only inspect_signature canonical effects mutatesSession, launchesProcess, and writesFilesystem; leave other nativeEvidence entries unchanged.
  • Add focused regression coverage for capability discovery and TOOL_EFFECTS.
  • Ordinary fix within existing boundaries; maintainer scope decision: not applicable.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Native macOS capability metadata and canonical tool-effects declaration; discovery/effect projection.
  • Contribution scope fit and linked maintainer decision: Ordinary metadata correction within existing provider boundaries; not applicable.
  • CLI and MCP/tool-catalog contract: capabilities --json should advertise may_write_filesystem; canonical effects expose writesFilesystem. MCP annotations remain derived from canonical effects.
  • Provider, bridge, target-format, or platform compatibility: Metadata correction for the existing native macOS inspect_signature operation only.
  • Evidence, artifact, provenance, or reconstruction contract: No change.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): No implementation or cleanup behavior changed. The existing private snapshot and its cleanup/failure handling remain unchanged.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): No generated metadata is intentionally changed. Adding writesFilesystem should not change the current MCP annotation values because this tool already launches a process and mutates session state.

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.

The checkbox describes this metadata-only patch; it does not imply that the existing runtime operation has no process or filesystem effects.

Evidence and regression coverage

  • Tests added or updated: src/native/NativeMacOSProviderMetadata.test.ts asserts inspect_signature advertises mayWriteFilesystem === true and launchesProcess === true. src/contracts/toolEffects.test.ts asserts TOOL_EFFECTS.inspect_signature.writesFilesystem === true.
  • Base reproduction or other evidence: with the source change stashed and the new tests kept, both new tests fail with expected false to be true; with the change they pass (run by an AI assistant in a Linux sandbox on an equivalent patch).
  • User-visible CLI/MCP output: after npm run build, node scripts/rea.mjs capabilities --json reports effects ["launches_process", "may_write_filesystem"] for the capability containing inspect_signature (now grouped with observe_native_ui, which has identical effects).
  • Remaining proof gaps: the macOS runtime reproduction from #5423 was not executed. Full typecheck, knip, the full test suite and check:pr were not run locally; CI is the authority for those. No change was made to error classification or unwritable-temp behavior.
  • Brittleness check: inspected the capability inventory/effect projection path, annotationsFromEffects, the native inventory test, and the docs. No golden or documentation change was needed for this metadata-only correction. No broader audit of every native tool's temporary-file behavior was done; scope remains limited to inspect_signature.

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

Run by an AI assistant in a Linux sandbox on this PR merged with main (Node v22.22.2, npm 10.9.7 — NOT the pinned 24.18.0 / 11.16.0):

  • npx oxfmt --check src/native src/contracts — pass
  • npm run lint — exit 0 (only pre-existing warnings; one max-lines warning on toolEffects.ts also exists on main)
  • targeted tsc over the 4 changed files — pass
  • npm run test:focused -- <new tests + nativeInventory golden> — 3 files, 28 tests passed
  • npx vitest run src/contracts src/native tests/boundary/providers/native tests/composition — 97 files passed, 1165 tests passed (14 skipped)
  • MCP tests nativeSignatureTargetBindingMcp, managedNativeVerificationAdapters — 4 passed
  • npm run verify:module-boundaries, npm run verify:architecture-guards, metadata:check, docs:check, test:python (81 OK), scan:todos, jscpd — pass

Not run: full tsc --noEmit (sandbox out of memory), knip (crashes in the sandbox on main too), full test:run, check:pr, macOS runtime repro.

Compatibility, safety, and release

  • Breaking changes or migration steps: None expected; this makes metadata truthful for existing behavior.
  • Real Hopper/Ghidra, browser, or OS coverage: The issue's macOS runtime reproduction was not executed.
  • Package or release metadata impact: None expected.
  • Security, privacy, process, or containment review: No new target interaction or filesystem operation; the existing private snapshot path and fail-closed behavior are unchanged.

Author readiness

Human owner: @prashantpiyush1111

  • 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.

Reviewer follow-up

  • Snapshot caching: isSnapshotCacheable permits caching only when mayWriteFilesystem === false, unless the descriptor explicitly sets cachePolicy: "snapshot". No cachePolicy override was found under src/native. Therefore this metadata correction can make inspect_signature ineligible for snapshot caching where it was previously eligible. I have not verified whether an active native-macos analysis profile currently caches this operation, so I am documenting this as a possible cache-eligibility change rather than claiming that runtime cache behavior was observed. This is consistent with the newly declared filesystem write.
  • CI: the latest PR-triggered workflow runs on this head are marked action_required; they did not execute. I have not represented them as passing. Maintainer approval/authorization may be required to run these workflows.

🔄 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/5430 **Author:** [@prashantpiyush1111](https://github.com/prashantpiyush1111) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/native-signature-filesystem-effects` --- ### 📝 Commits (5) - [`12c656f`](https://github.com/morluto/rea/commit/12c656f0944ede40d72ed16cc27cc7d6c9f53f43) fix(native): declare filesystem write effect for inspect_signature - [`50100e5`](https://github.com/morluto/rea/commit/50100e53c89aef48fbfa580c529388948d58061d) fix(native): declare filesystem write effect for inspect_signature - [`5217324`](https://github.com/morluto/rea/commit/52173242195371fb5df4c0281b8a8b665c34a741) test(native): cover signature filesystem write effect - [`e7e80ec`](https://github.com/morluto/rea/commit/e7e80ec40f4883f914ef06ffabd970ef03ec8e6c) test(native): cover signature filesystem capability - [`83346d0`](https://github.com/morluto/rea/commit/83346d00be8fbd60812054b2f72732d0fbf1d559) test(native): cover signature filesystem write effect ### 📊 Changes **4 files changed** (+26 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/contracts/toolEffects.test.ts` (+6 -0) 📝 `src/contracts/toolEffects.ts` (+5 -1) ➕ `src/native/NativeMacOSProviderMetadata.test.ts` (+13 -0) 📝 `src/native/NativeMacOSProviderMetadata.ts` (+2 -1) </details> ### 📄 Description ## Summary Fixes #5423. Correct discovery metadata for `inspect_signature`: the operation already creates and writes a private temporary snapshot, but its capability descriptor and canonical tool effects omit that filesystem effect. ## Problem and expected behavior Capability discovery currently omits `may_write_filesystem` for `inspect_signature`, and `TOOL_EFFECTS.inspect_signature.writesFilesystem` is false. Discovery should report both effects truthfully. The existing failure when the temporary parent is unwritable remains valid; this PR does not alter error classification or make inspection succeed in that environment. ## Change and scope - Add `inspect_signature` to the native capability's filesystem-write declaration. - Give only `inspect_signature` canonical effects `mutatesSession`, `launchesProcess`, and `writesFilesystem`; leave other `nativeEvidence` entries unchanged. - Add focused regression coverage for capability discovery and `TOOL_EFFECTS`. - Ordinary fix within existing boundaries; maintainer scope decision: not applicable. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: Native macOS capability metadata and canonical tool-effects declaration; discovery/effect projection. - Contribution scope fit and linked maintainer decision: Ordinary metadata correction within existing provider boundaries; not applicable. - CLI and MCP/tool-catalog contract: `capabilities --json` should advertise `may_write_filesystem`; canonical effects expose `writesFilesystem`. MCP annotations remain derived from canonical effects. - Provider, bridge, target-format, or platform compatibility: Metadata correction for the existing native macOS `inspect_signature` operation only. - Evidence, artifact, provenance, or reconstruction contract: No change. - Target selection, authority, upstream behavior, and cleanup details (if applicable): No implementation or cleanup behavior changed. The existing private snapshot and its cleanup/failure handling remain unchanged. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): No generated metadata is intentionally changed. Adding `writesFilesystem` should not change the current MCP annotation values because this tool already launches a process and mutates session state. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] 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. The checkbox describes this metadata-only patch; it does not imply that the existing runtime operation has no process or filesystem effects. ## Evidence and regression coverage - Tests added or updated: `src/native/NativeMacOSProviderMetadata.test.ts` asserts `inspect_signature` advertises `mayWriteFilesystem === true` and `launchesProcess === true`. `src/contracts/toolEffects.test.ts` asserts `TOOL_EFFECTS.inspect_signature.writesFilesystem === true`. - Base reproduction or other evidence: with the source change stashed and the new tests kept, both new tests fail with `expected false to be true`; with the change they pass (run by an AI assistant in a Linux sandbox on an equivalent patch). - User-visible CLI/MCP output: after `npm run build`, `node scripts/rea.mjs capabilities --json` reports effects `["launches_process", "may_write_filesystem"]` for the capability containing `inspect_signature` (now grouped with `observe_native_ui`, which has identical effects). - Remaining proof gaps: the macOS runtime reproduction from #5423 was not executed. Full typecheck, knip, the full test suite and `check:pr` were not run locally; CI is the authority for those. No change was made to error classification or unwritable-temp behavior. - Brittleness check: inspected the capability inventory/effect projection path, `annotationsFromEffects`, the native inventory test, and the docs. No golden or documentation change was needed for this metadata-only correction. No broader audit of every native tool's temporary-file behavior was done; scope remains limited to `inspect_signature`. 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 Run by an AI assistant in a Linux sandbox on this PR merged with main (Node v22.22.2, npm 10.9.7 — NOT the pinned 24.18.0 / 11.16.0): - `npx oxfmt --check src/native src/contracts` — pass - `npm run lint` — exit 0 (only pre-existing warnings; one `max-lines` warning on `toolEffects.ts` also exists on main) - targeted `tsc` over the 4 changed files — pass - `npm run test:focused -- <new tests + nativeInventory golden>` — 3 files, 28 tests passed - `npx vitest run src/contracts src/native tests/boundary/providers/native tests/composition` — 97 files passed, 1165 tests passed (14 skipped) - MCP tests `nativeSignatureTargetBindingMcp`, `managedNativeVerificationAdapters` — 4 passed - `npm run verify:module-boundaries`, `npm run verify:architecture-guards`, `metadata:check`, `docs:check`, `test:python` (81 OK), `scan:todos`, `jscpd` — pass Not run: full `tsc --noEmit` (sandbox out of memory), `knip` (crashes in the sandbox on main too), full `test:run`, `check:pr`, macOS runtime repro. ## Compatibility, safety, and release - Breaking changes or migration steps: None expected; this makes metadata truthful for existing behavior. - Real Hopper/Ghidra, browser, or OS coverage: The issue's macOS runtime reproduction was not executed. - Package or release metadata impact: None expected. - Security, privacy, process, or containment review: No new target interaction or filesystem operation; the existing private snapshot path and fail-closed behavior are unchanged. ## Author readiness Human owner: @prashantpiyush1111 - [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. - [ ] 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. ## Reviewer follow-up - Snapshot caching: `isSnapshotCacheable` permits caching only when `mayWriteFilesystem === false`, unless the descriptor explicitly sets `cachePolicy: "snapshot"`. No `cachePolicy` override was found under `src/native`. Therefore this metadata correction can make `inspect_signature` ineligible for snapshot caching where it was previously eligible. I have not verified whether an active `native-macos` analysis profile currently caches this operation, so I am documenting this as a possible cache-eligibility change rather than claiming that runtime cache behavior was observed. This is consistent with the newly declared filesystem write. - CI: the latest PR-triggered workflow runs on this head are marked `action_required`; they did not execute. I have not represented them as passing. Maintainer approval/authorization may be required to run these workflows. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:44:07 +00:00
Sign in to join this conversation.
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#6928
No description provided.