morluto/rea

Fork 0

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

[PR #6245] [CLOSED] fix(javascript): keep selected-input filesystem failures typed #7221

New issue

Closed

opened 2026-11-01 21:46:16 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6245
Author: @kaoru0822-kitauji
Created: 10/22/2026
Status: ❌ Closed

Base: main ← Head: fix/6116-javascript-application-fs-errors

📝 Commits (1)

abd99a4 fix(javascript): keep selected-input filesystem failures typed

📊 Changes

3 files changed (+203 additions, -20 deletions)

View changed files

📝 src/application/javascript/JavaScriptApplicationService.ts (+43 -8)
📝 src/application/javascript/JavaScriptArtifactReconstruction.ts (+18 -10)
📝 tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts (+142 -2)

📄 Description

What broke: analyze_javascript_application / analyze-javascript-application turned filesystem failures on the selected input into a generic artifact_operation_failed / execution_failure / reason: io. A readable but unsearchable selected directory (mode 0400) only had its errno in prose. Raw realpath/lstat/access errors had no path, errno or diagnostics at all.
Fix: The selection owner now requires search permission on a selected directory and does its metadata read inside the same classifier, so selected-input EACCES/EPERM becomes the existing access_denied (path, system_code, boundary: filesystem-read). Errno failures that nothing narrower classified now keep their facts: EMFILE/ENFILE become a retryable resource_constraint, and anything else stays artifact_operation_failed/io but keeps the host message plus system_code, syscall and path diagnostics.
Risk: Low. No schema change, only existing error classes. A selected directory without search permission was already unusable, because every entry lookup failed. Failures in child entries of a searchable root keep their current behavior.

Summary

Fixes #6116
Fixes #5991

Both issues come from one place: the JavaScript application selection boundary (resolveSelectedInput → resolveFormat in JavaScriptArtifactReconstruction.ts) and the service's raw-errno fallback in JavaScriptApplicationService.ts. The selection check only tested R_OK, so a selected directory without search permission was admitted and then failed inside traversal (#6116). resolveFormat called lstat outside the classifier, and the raw fallback built new ArtifactOperationError(OPERATION, "io") with no cause, detail or diagnostics (#5991).

Problem and expected behavior

#6116 smallest trigger: a selected directory $app at mode 0400 containing index.js. Expected: the same host access denial as mode 000, with the selected path and EACCES.
#5991 smallest trigger: an errno-bearing failure from lstat (EACCES), realpath (EIO) or access (EMFILE) on the selected path. Expected: a selected-read denial stays access_denied. Other host failures keep their code, path and message instead of a bare reason: io.

Change and scope

JavaScriptArtifactReconstruction.ts: resolveSelectedInput runs realpath, then lstat, then access(R_OK | X_OK) for directories (R_OK for files), all inside the existing classifier, and returns the metadata. resolveFormat becomes the pure selectedFormat, which removes the second lstat call. Other errno codes still propagate raw.
JavaScriptApplicationService.ts: the raw FILESYSTEM_ERROR_CODES branch is replaced by hostFilesystemFailure. It does not claim a selected-read denial, because the raw failure may concern a private workspace file such as a worker transfer file. Instead it keeps the host's own facts: EMFILE/ENFILE become AnalysisResourceConstraintError("capacity", retryable) with reported_limits. Other codes stay ArtifactOperationError("io"), with detail set to the host message and diagnostics set to {system_code, syscall, path}.
CLI and MCP both call this service and use the same projectAnalysisError, so their output stays the same.
Not included:
- Child-entry failures under a searchable root, which already come back from DirectoryArtifactReader as an ArtifactReaderFailure("io") with errno and path in detail. That is #5229 / PR #5230 territory. Lifting errno into a structured field there belongs to the shared ArtifactReaderFailure owner, which every artifact operation uses.
- A permission change between the selection check and traversal. It still yields the root-traversal io failure.

Contribution scope: not applicable. This is an ordinary fix within existing boundaries.

Contract and boundary impact

Semantic owner and earliest changed stage: JavaScript application input selection (resolveSelectedInput), plus the service's fallback for unclassified host filesystem failures.
Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).
CLI and MCP/tool-catalog contract: no schema change. Existing error codes are used: access_denied, resource_constraint, and artifact_operation_failed with added detail/diagnostics.
Provider, bridge, target-format, or platform compatibility: on Windows, X_OK is treated as an existence check, so behavior there is unchanged.
Evidence, artifact, provenance, or reconstruction contract: unchanged for successful analyses.
Target selection, authority, upstream behavior, and cleanup details: no change in authority. Permissions are only checked, never changed.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added (tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts):
- Real mode-0400 selected directory → access_denied with path, system_code: EACCES and boundary: filesystem-read. After restoring 0700, the same directory analyzes successfully. Permissions are restored in finally.
- Injected selected-path faults via syncBuiltinESMExports, each asserted to fire exactly once:
  - realpath EIO → artifact_operation_failed/io with detail and diagnostics {system_code, syscall, path}.
  - access EMFILE → resource_constraint, retryable: true, resource: capacity and reported_limits.
  - lstat EACCES → access_denied.
  - A plain non-errno Error → still execution_failure with its message.
- With only the two source files reverted, the four errno/permission tests fail and the non-errno control passes.
- The existing mode-000, missing, ELOOP and ENAMETOOLONG selection tests are unchanged and pass.
Base reproduction (built CLI, base d77bc0e8a, macOS arm64, non-root, isolated config/state, real mode bits, restored in a trap):
- 0700 → exit 0
- 0400 → exit 1, {"code":"artifact_operation_failed","category":"execution_failure","details":{"reason":"io","detail":"Could not inspect entry at <app>/index.js (EACCES): …"}}
- 0000 → access_denied
- 0700 → exit 0
After the fix (same script, rebuilt CLI): 0400 → exit 1, {"code":"access_denied","category":"unavailable","details":{"operation":"analyze_javascript_application","path":"<app>","system_code":"EACCES","boundary":"filesystem-read"}}. Mode 000 is unchanged, and 0700 succeeds before and after.

MCP output (the in-memory MCP repro from #5991, run as a temporary uncommitted file): on base, all three faults returned bare reason: io. With the fix:

{"method":"lstat","code":"EACCES","result":{"code":"access_denied","category":"unavailable","details":{"path":"<root>","system_code":"EACCES","boundary":"filesystem-read"}}}
{"method":"realpath","code":"EIO","result":{"code":"artifact_operation_failed","category":"execution_failure","details":{"reason":"io","detail":"EIO: selected path realpath failed","diagnostics":{"system_code":"EIO","syscall":"realpath","path":"<root>"}}}}
{"method":"access","code":"EMFILE","result":{"code":"resource_constraint","category":"resource_constraint","retryable":true,"details":{"resource":"capacity","reported_limits":{"system_code":"EMFILE","syscall":"access","path":"<root>"}}}}

The readable and mode-000 controls are unchanged.

Remaining proof gaps:
- EIO, EMFILE and the post-selection EACCES were injected, not produced naturally by the host.
- Windows was not run.
- Failures from private worker workspace files were covered from the source only, through the same fallback. No test was run for them.
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

Built-CLI mode 0700/0400/0000/0700 repro: on base, 0400 gave artifact_operation_failed. With the fix it gives access_denied with path and EACCES, and the controls are unchanged.
#5991 in-memory MCP repro: on base, 5/5 passed as written (bug present). With the fix, the 3 assertions that encode the bug fail and the outputs match the ones above.
vitest run tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts: 12 passed. On base source, the 4 new errno/permission tests fail.
npm run test:focused with these 10 files: 158 passed.
- JavaScript application selection, service, reconstruction and workflows
- targetDiagnostics
- MCP electronMcp and javascriptAnalysisCancellation
- CLI javascriptApplicationJsonOutput and javascriptApplicationProgress
- javascriptApplicationCancellation
npm run check:fast: passed.
npm run format:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Callers that handled artifact_operation_failed for these cases now get the more specific existing codes listed above.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only. No external provider is involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: diagnostics add the host-reported path of the failed syscall, which can be a private temp workspace path. No credentials are involved.

Author readiness

Human owner: @kaoru0822-kitauji

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/6245 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6116-javascript-application-fs-errors` --- ### 📝 Commits (1) - [`abd99a4`](https://github.com/morluto/rea/commit/abd99a4ce7ce75c20df19245a4244e4fddd2e86a) fix(javascript): keep selected-input filesystem failures typed ### 📊 Changes **3 files changed** (+203 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptApplicationService.ts` (+43 -8) 📝 `src/application/javascript/JavaScriptArtifactReconstruction.ts` (+18 -10) 📝 `tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts` (+142 -2) </details> ### 📄 Description **What broke:** `analyze_javascript_application` / `analyze-javascript-application` turned filesystem failures on the selected input into a generic `artifact_operation_failed` / `execution_failure` / `reason: io`. A readable but unsearchable selected directory (mode 0400) only had its errno in prose. Raw `realpath`/`lstat`/`access` errors had no path, errno or diagnostics at all. **Fix:** The selection owner now requires search permission on a selected directory and does its metadata read inside the same classifier, so selected-input `EACCES`/`EPERM` becomes the existing `access_denied` (path, `system_code`, `boundary: filesystem-read`). Errno failures that nothing narrower classified now keep their facts: `EMFILE`/`ENFILE` become a retryable `resource_constraint`, and anything else stays `artifact_operation_failed`/`io` but keeps the host message plus `system_code`, `syscall` and `path` diagnostics. **Risk:** Low. No schema change, only existing error classes. A selected directory without search permission was already unusable, because every entry lookup failed. Failures in child entries of a searchable root keep their current behavior. ## Summary Fixes #6116 Fixes #5991 Both issues come from one place: the JavaScript application selection boundary (`resolveSelectedInput` → `resolveFormat` in `JavaScriptArtifactReconstruction.ts`) and the service's raw-errno fallback in `JavaScriptApplicationService.ts`. The selection check only tested `R_OK`, so a selected directory without search permission was admitted and then failed inside traversal (#6116). `resolveFormat` called `lstat` outside the classifier, and the raw fallback built `new ArtifactOperationError(OPERATION, "io")` with no cause, detail or diagnostics (#5991). ## Problem and expected behavior - #6116 smallest trigger: a selected directory `$app` at mode 0400 containing `index.js`. Expected: the same host access denial as mode 000, with the selected path and `EACCES`. - #5991 smallest trigger: an errno-bearing failure from `lstat` (EACCES), `realpath` (EIO) or `access` (EMFILE) on the selected path. Expected: a selected-read denial stays `access_denied`. Other host failures keep their code, path and message instead of a bare `reason: io`. ## Change and scope - `JavaScriptArtifactReconstruction.ts`: `resolveSelectedInput` runs `realpath`, then `lstat`, then `access(R_OK | X_OK)` for directories (`R_OK` for files), all inside the existing classifier, and returns the metadata. `resolveFormat` becomes the pure `selectedFormat`, which removes the second `lstat` call. Other errno codes still propagate raw. - `JavaScriptApplicationService.ts`: the raw `FILESYSTEM_ERROR_CODES` branch is replaced by `hostFilesystemFailure`. It does not claim a selected-read denial, because the raw failure may concern a private workspace file such as a worker transfer file. Instead it keeps the host's own facts: `EMFILE`/`ENFILE` become `AnalysisResourceConstraintError("capacity", retryable)` with `reported_limits`. Other codes stay `ArtifactOperationError("io")`, with `detail` set to the host message and `diagnostics` set to `{system_code, syscall, path}`. - CLI and MCP both call this service and use the same `projectAnalysisError`, so their output stays the same. - Not included: - Child-entry failures under a searchable root, which already come back from `DirectoryArtifactReader` as an `ArtifactReaderFailure("io")` with errno and path in `detail`. That is #5229 / PR #5230 territory. Lifting errno into a structured field there belongs to the shared `ArtifactReaderFailure` owner, which every artifact operation uses. - A permission change between the selection check and traversal. It still yields the root-traversal `io` failure. Contribution scope: not applicable. This is an ordinary fix within existing boundaries. ## Contract and boundary impact - Semantic owner and earliest changed stage: JavaScript application input selection (`resolveSelectedInput`), plus the service's fallback for unclassified host filesystem failures. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: no schema change. Existing error codes are used: `access_denied`, `resource_constraint`, and `artifact_operation_failed` with added `detail`/`diagnostics`. - Provider, bridge, target-format, or platform compatibility: on Windows, `X_OK` is treated as an existence check, so behavior there is unchanged. - Evidence, artifact, provenance, or reconstruction contract: unchanged for successful analyses. - Target selection, authority, upstream behavior, and cleanup details: no change in authority. Permissions are only checked, never changed. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added (`tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts`): - Real mode-0400 selected directory → `access_denied` with `path`, `system_code: EACCES` and `boundary: filesystem-read`. After restoring 0700, the same directory analyzes successfully. Permissions are restored in `finally`. - Injected selected-path faults via `syncBuiltinESMExports`, each asserted to fire exactly once: - `realpath` EIO → `artifact_operation_failed`/`io` with `detail` and `diagnostics {system_code, syscall, path}`. - `access` EMFILE → `resource_constraint`, `retryable: true`, `resource: capacity` and `reported_limits`. - `lstat` EACCES → `access_denied`. - A plain non-errno `Error` → still `execution_failure` with its message. - With only the two source files reverted, the four errno/permission tests fail and the non-errno control passes. - The existing mode-000, missing, ELOOP and ENAMETOOLONG selection tests are unchanged and pass. - Base reproduction (built CLI, base `d77bc0e8a`, macOS arm64, non-root, isolated config/state, real mode bits, restored in a trap): - 0700 → exit 0 - 0400 → exit 1, `{"code":"artifact_operation_failed","category":"execution_failure","details":{"reason":"io","detail":"Could not inspect entry at <app>/index.js (EACCES): …"}}` - 0000 → `access_denied` - 0700 → exit 0 - After the fix (same script, rebuilt CLI): 0400 → exit 1, `{"code":"access_denied","category":"unavailable","details":{"operation":"analyze_javascript_application","path":"<app>","system_code":"EACCES","boundary":"filesystem-read"}}`. Mode 000 is unchanged, and 0700 succeeds before and after. - MCP output (the in-memory MCP repro from #5991, run as a temporary uncommitted file): on base, all three faults returned bare `reason: io`. With the fix: ```json {"method":"lstat","code":"EACCES","result":{"code":"access_denied","category":"unavailable","details":{"path":"<root>","system_code":"EACCES","boundary":"filesystem-read"}}} {"method":"realpath","code":"EIO","result":{"code":"artifact_operation_failed","category":"execution_failure","details":{"reason":"io","detail":"EIO: selected path realpath failed","diagnostics":{"system_code":"EIO","syscall":"realpath","path":"<root>"}}}} {"method":"access","code":"EMFILE","result":{"code":"resource_constraint","category":"resource_constraint","retryable":true,"details":{"resource":"capacity","reported_limits":{"system_code":"EMFILE","syscall":"access","path":"<root>"}}}} ``` The readable and mode-000 controls are unchanged. - Remaining proof gaps: - EIO, EMFILE and the post-selection EACCES were injected, not produced naturally by the host. - Windows was not run. - Failures from private worker workspace files were covered from the source only, through the same fallback. No test was run for them. - [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 - Built-CLI mode 0700/0400/0000/0700 repro: on base, 0400 gave `artifact_operation_failed`. With the fix it gives `access_denied` with path and `EACCES`, and the controls are unchanged. - #5991 in-memory MCP repro: on base, 5/5 passed as written (bug present). With the fix, the 3 assertions that encode the bug fail and the outputs match the ones above. - `vitest run tests/boundary/filesystem/javascriptApplicationInputSelection.test.ts`: 12 passed. On base source, the 4 new errno/permission tests fail. - `npm run test:focused` with these 10 files: 158 passed. - JavaScript application selection, service, reconstruction and workflows - `targetDiagnostics` - MCP `electronMcp` and `javascriptAnalysisCancellation` - CLI `javascriptApplicationJsonOutput` and `javascriptApplicationProgress` - `javascriptApplicationCancellation` - `npm run check:fast`: passed. - `npm run format:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that handled `artifact_operation_failed` for these cases now get the more specific existing codes listed above. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only. No external provider is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: diagnostics add the host-reported path of the failed syscall, which can be a private temp workspace path. No credentials are involved. ## Author readiness Human owner: @kaoru0822-kitauji - [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. 🤖 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-11-01 21:46:16 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:39:55 +00:00

[PR #7221] [MERGED] fix(artifacts): explain recovery for existing DMG attachments #7725

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#7221

No description provided.

Rows
Columns