[PR #4525] [CLOSED] fix(setup): report an uninspectable MSIX private copy at the write recheck #6287

Closed
opened 2026-10-31 01:13:18 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4525
Author: @ranmori
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/setup-msix-recheck-inspection


📝 Commits (1)

  • cddbb1a fix(setup): report an uninspectable MSIX private copy at the write recheck

📊 Changes

5 files changed (+102 additions, -12 deletions)

View changed files

📝 src/application/ClaudeDesktopConfigLocation.test.ts (+65 -0)
📝 src/application/MsixPackageStorage.ts (+16 -7)
📝 src/application/SetupClientConfiguration.ts (+13 -5)
📝 src/application/SetupClients.ts (+2 -0)
📝 src/application/SetupTypes.ts (+6 -0)

📄 Description

Summary

Fixes #4378.

Before writing the AppData claude_desktop_config.json it selected, setup rechecks whether the MSIX package has since created its private copy. packageReadTargetChanged treated every lstat failure other than ENOENT/ENOTDIR as a new private copy, so an EACCES or EPERM inspection failure was reported as target_changed: "The client began reading a different configuration file after the plan was shown". That message has no path or error, and its "rerun setup" advice cannot help with an access problem.

This PR reports that case as its own failure, target_unknown, with the private path, the error, and access-repair guidance. Writes stay blocked in both cases.

Problem and expected behavior

Trigger (source-derived, as in the issue): Windows Claude Desktop MSIX, AppData configuration selected with a direct process view, and the final lstat of the package-private path fails with a non-absence error before persistence.

  • Before: { status: "failed", reason: "target_changed" } and the "began reading a different configuration file" message.
  • After: { status: "failed", reason: "target_unknown", failure: { path, detail } } and:
    <private path> could not be inspected after the plan was shown, so REA cannot establish which configuration file the client reads: <error>. No change was made. Repair access to that path, then rerun setup.

Unchanged: a private copy that is actually observed still returns target_changed; ENOENT/ENOTDIR still keep AppData; a plan that already chose the private copy is not rechecked.

Change and scope

  • MsixPackageStorage.packageReadTargetChanged returns Result<boolean, { path, detail }> instead of boolean, and takes an optional inspect function (default lstat) in the same injection style as packagePrivateLauncherBlocker and redirectedWriteBlocker.
  • SetupClientConfiguration.persistClientConfiguration maps the error to the new target_unknown result before any backup or write.
  • SetupTypes.ClientConfigurationResult gains a target_unknown variant carrying failure: { path, detail }, shaped like the existing rollback variant.
  • SetupClients.failedConfigurationMessage renders it. The wording mirrors the access-repair guidance initial discovery already gives in ClaudeDesktopConfigLocation.

Not included: doctor and uninstall, which the issue does not establish as affected.

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/application/MsixPackageStorage.ts, the setup persistence recheck.
  • CLI and MCP/tool-catalog contract: setup JSON output can now contain reason: "target_unknown" with a failure object in clients.<name>, in the case that previously reported target_changed. No tool schema changes.
  • Provider, bridge, target-format, or platform compatibility: Windows MSIX clients only.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Process execution, authorization, cleanup, or containment impact: none. The write gate is unchanged.
  • Generated metadata, package, or installation impact: none.

Evidence and regression coverage

  • Tests added: a read-target recheck before writing AppData block in src/application/ClaudeDesktopConfigLocation.test.ts. It covers an observed private copy (changed), ENOENT and ENOTDIR (unchanged), EACCES and EPERM (inspection failure with path and detail), a plan that already chose the private copy (not rechecked), and the rendered message.
  • Base reproduction: executed. With the source changes stashed, all 7 new tests fail on the base; with them, all 24 tests in the file pass.
  • Existing coverage: tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts, including refuses a write once the package reads a newer private copy (target_changed), passes unchanged.
  • Remaining proof gaps: the mapping in persistClientConfiguration is covered by type checking and review, not by a test. A real-filesystem EACCES at that point could not be produced portably; on Windows an over-long or invalid component yields ENOENT or a TypeError. No real ACL failure on an MSIX install was exercised.

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

Windows 11 x64, Node 24.13.0, base 9a82046b:

  • npx vitest run --project services src/application/ClaudeDesktopConfigLocation.test.ts: 24 passed (7 new). On the base, the same 7 fail.
  • npx vitest run tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts src/application: 1,095 passed, 11 failed. The same 11 fail on this host without the change, in Setup.planning, Doctor, LinuxHopper, MacHopper, MacHopperPaths and BinarySessionLiveness; they appear to be Windows-host path assumptions.
  • npm run check:fast: typecheck and lint passed (also in the pre-push hook).
  • npx oxfmt --check src/application: passed.

Not run: npm run check, npm run check:pr, npm run docs:check, and an applied setup against a real Claude Desktop install.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. A caller that switched on reason === "target_changed" will now see target_unknown for inspection failures, which is the correction this issue asks for.
  • Real Hopper/Ghidra, browser, or OS coverage: unit and boundary tests on Windows 11 only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the message includes the private config path and the filesystem error text, as initial discovery already does. No configuration contents are read.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • 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/4525 **Author:** [@ranmori](https://github.com/ranmori) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/setup-msix-recheck-inspection` --- ### 📝 Commits (1) - [`cddbb1a`](https://github.com/morluto/rea/commit/cddbb1a995ac84c70dcb851b81e5b3cac29a60d5) fix(setup): report an uninspectable MSIX private copy at the write recheck ### 📊 Changes **5 files changed** (+102 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClaudeDesktopConfigLocation.test.ts` (+65 -0) 📝 `src/application/MsixPackageStorage.ts` (+16 -7) 📝 `src/application/SetupClientConfiguration.ts` (+13 -5) 📝 `src/application/SetupClients.ts` (+2 -0) 📝 `src/application/SetupTypes.ts` (+6 -0) </details> ### 📄 Description ## Summary Fixes #4378. Before writing the AppData `claude_desktop_config.json` it selected, setup rechecks whether the MSIX package has since created its private copy. `packageReadTargetChanged` treated every `lstat` failure other than `ENOENT`/`ENOTDIR` as a new private copy, so an `EACCES` or `EPERM` inspection failure was reported as `target_changed`: "The client began reading a different configuration file after the plan was shown". That message has no path or error, and its "rerun setup" advice cannot help with an access problem. This PR reports that case as its own failure, `target_unknown`, with the private path, the error, and access-repair guidance. Writes stay blocked in both cases. ## Problem and expected behavior Trigger (source-derived, as in the issue): Windows Claude Desktop MSIX, AppData configuration selected with a `direct` process view, and the final `lstat` of the package-private path fails with a non-absence error before persistence. - Before: `{ status: "failed", reason: "target_changed" }` and the "began reading a different configuration file" message. - After: `{ status: "failed", reason: "target_unknown", failure: { path, detail } }` and: `<private path> could not be inspected after the plan was shown, so REA cannot establish which configuration file the client reads: <error>. No change was made. Repair access to that path, then rerun setup.` Unchanged: a private copy that is actually observed still returns `target_changed`; `ENOENT`/`ENOTDIR` still keep AppData; a plan that already chose the private copy is not rechecked. ## Change and scope - `MsixPackageStorage.packageReadTargetChanged` returns `Result<boolean, { path, detail }>` instead of `boolean`, and takes an optional `inspect` function (default `lstat`) in the same injection style as `packagePrivateLauncherBlocker` and `redirectedWriteBlocker`. - `SetupClientConfiguration.persistClientConfiguration` maps the error to the new `target_unknown` result before any backup or write. - `SetupTypes.ClientConfigurationResult` gains a `target_unknown` variant carrying `failure: { path, detail }`, shaped like the existing `rollback` variant. - `SetupClients.failedConfigurationMessage` renders it. The wording mirrors the access-repair guidance initial discovery already gives in `ClaudeDesktopConfigLocation`. Not included: doctor and uninstall, which the issue does not establish as affected. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/application/MsixPackageStorage.ts`, the setup persistence recheck. - CLI and MCP/tool-catalog contract: setup JSON output can now contain `reason: "target_unknown"` with a `failure` object in `clients.<name>`, in the case that previously reported `target_changed`. No tool schema changes. - Provider, bridge, target-format, or platform compatibility: Windows MSIX clients only. - Evidence, artifact, provenance, or reconstruction contract: none. - Process execution, authorization, cleanup, or containment impact: none. The write gate is unchanged. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Tests added: a `read-target recheck before writing AppData` block in `src/application/ClaudeDesktopConfigLocation.test.ts`. It covers an observed private copy (changed), `ENOENT` and `ENOTDIR` (unchanged), `EACCES` and `EPERM` (inspection failure with path and detail), a plan that already chose the private copy (not rechecked), and the rendered message. - Base reproduction: executed. With the source changes stashed, all 7 new tests fail on the base; with them, all 24 tests in the file pass. - Existing coverage: `tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts`, including `refuses a write once the package reads a newer private copy` (`target_changed`), passes unchanged. - Remaining proof gaps: the mapping in `persistClientConfiguration` is covered by type checking and review, not by a test. A real-filesystem `EACCES` at that point could not be produced portably; on Windows an over-long or invalid component yields `ENOENT` or a `TypeError`. No real ACL failure on an MSIX install was exercised. 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 Windows 11 x64, Node 24.13.0, base `9a82046b`: - `npx vitest run --project services src/application/ClaudeDesktopConfigLocation.test.ts`: 24 passed (7 new). On the base, the same 7 fail. - `npx vitest run tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts src/application`: 1,095 passed, 11 failed. The same 11 fail on this host without the change, in `Setup.planning`, `Doctor`, `LinuxHopper`, `MacHopper`, `MacHopperPaths` and `BinarySessionLiveness`; they appear to be Windows-host path assumptions. - `npm run check:fast`: typecheck and lint passed (also in the pre-push hook). - `npx oxfmt --check src/application`: passed. Not run: `npm run check`, `npm run check:pr`, `npm run docs:check`, and an applied setup against a real Claude Desktop install. ## Compatibility, safety, and release - Breaking changes or migration steps: none. A caller that switched on `reason === "target_changed"` will now see `target_unknown` for inspection failures, which is the correction this issue asks for. - Real Hopper/Ghidra, browser, or OS coverage: unit and boundary tests on Windows 11 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the message includes the private config path and the filesystem error text, as initial discovery already does. No configuration contents are read. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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-31 01:13:18 +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#6287
No description provided.