[PR #552] [MERGED] fix(platform): accept absolute paths for any host and stop blaming the ceiling for a missing grant #2068

New issue

Closed

opened 2026-10-16 21:13:54 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:54 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/552
Author: @morluto
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: fix/windows-scenario-paths-and-permission-remediation

📝 Commits (1)

72bd8e1 fix(platform): accept absolute paths for any host and stop blaming the ceiling for a missing grant

📊 Changes

10 files changed (+219 additions, -13 deletions)

View changed files

📝 docs/product-catalog.json (+2 -2)
📝 docs/verification/managed-conformance-manifest.json (+2 -2)
📝 skills/reverse-engineer-anything/SKILL.md (+1 -1)
📝 src/application/PermissionAuthority.ts (+11 -3)
➕ src/application/PermissionAuthorityRemediation.test.ts (+99 -0)
📝 src/domain/analysisErrorPresentation.ts (+2 -0)
📝 src/domain/errors.ts (+13 -1)
📝 src/domain/processScenario.test.ts (+61 -0)
📝 src/domain/processScenario.ts (+27 -3)
📝 src/generatedMcpToolCatalog.ts (+1 -1)

📄 Description

Fixes #548 and #545.

#548 — process scenarios were unexpressable on Windows

processScenarioSchema required a leading / on executable, working_directory and filesystem_roots, so validation rejected every Windows scenario before anything else could run. Process capture is one of the few inherently cross-platform capabilities REA has, so this made capture_process_scenario and compare_process_captures unusable there.

The suggested fix was subtly wrong, so this does something else. The report proposes node:path's isAbsolute, which answers for the machine doing the checking: on Linux, C:\tools is not absolute. That works on Windows but silently breaks everywhere a Windows scenario is validated by another host — replay, comparison, or a test asserting on a fixture. A boundary schema must not vary with its evaluator, so the predicate accepts the POSIX form, a Windows drive path and a UNC share on every host, and is exported and unit-tested directly.

The rejection message now names the constraint, so a rejected path is actionable rather than surfacing as a generic "invalid request".

Two things I checked rather than assumed:

Whether this only moved the failure downstream. Containment is enforced by isPathWithinRoot, which already uses host-aware path.relative, and ProcessCaptureAuthority canonicalises both sides before comparing. Execution was never POSIX-only — validation really was the sole blocker.
Whether the path field inside checkpoints needed the same treatment. It is an HTTP path, not a filesystem path, so startsWith("/") is correct there and is deliberately unchanged.

#545 — a denial that blamed the wrong thing

When a scope is inside the administrator ceiling but no grant was ever issued, the error still told the caller to "Add the exact missing scope beneath the administrator ceiling". That advice cannot work: the ceiling already permits the operation. The missing payload is computed against candidate grants, not the ceiling, so it describes what a grant must cover — hence the report's observation that entries in missing appear verbatim in ceiling.

PermissionRemediation gains a grant variant that says the ceiling already covers the scope and names the two real routes (a project grant, or the documented auto-grant setting). PermissionAuthority selects it whenever the denial is not a ceiling violation.

Preserved deliberately: a genuine ceiling violation still says configure; an explicitly requested elicit/restart is still honoured; missing is unchanged, because it is accurate — it just was not being read as "what a grant must cover".

The executables directory-vs-exact containment semantics are noted and not changed here. That is a real documentation gap, but it is a contract question about what a scope means, not a bug, and folding it into a fix for two unrelated defects would make the change harder to review. Worth its own issue.

Tests

processScenario.test.ts: POSIX, Windows-drive (both separators) and UNC paths all validate on a Linux host, Windows filesystem_roots accepted, five relative/empty forms still rejected, and the actionable message asserted.
PermissionAuthorityRemediation.test.ts: covered scope and an empty request both resolve to grant; a genuine ceiling violation still resolves to configure; an explicit elicit is untouched.

Full suite green (1987 passed) with the regenerated catalog; typecheck, lint and prettier clean.

_{🔄 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/552 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/windows-scenario-paths-and-permission-remediation` --- ### 📝 Commits (1) - [`72bd8e1`](https://github.com/morluto/rea/commit/72bd8e109c27e81ed409368cda8bfdcd6bf561b1) fix(platform): accept absolute paths for any host and stop blaming the ceiling for a missing grant ### 📊 Changes **10 files changed** (+219 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `docs/product-catalog.json` (+2 -2) 📝 `docs/verification/managed-conformance-manifest.json` (+2 -2) 📝 `skills/reverse-engineer-anything/SKILL.md` (+1 -1) 📝 `src/application/PermissionAuthority.ts` (+11 -3) ➕ `src/application/PermissionAuthorityRemediation.test.ts` (+99 -0) 📝 `src/domain/analysisErrorPresentation.ts` (+2 -0) 📝 `src/domain/errors.ts` (+13 -1) 📝 `src/domain/processScenario.test.ts` (+61 -0) 📝 `src/domain/processScenario.ts` (+27 -3) 📝 `src/generatedMcpToolCatalog.ts` (+1 -1) </details> ### 📄 Description Fixes #548 and #545. ## #548 — process scenarios were unexpressable on Windows `processScenarioSchema` required a leading `/` on `executable`, `working_directory` and `filesystem_roots`, so validation rejected every Windows scenario before anything else could run. Process capture is one of the few inherently cross-platform capabilities REA has, so this made `capture_process_scenario` and `compare_process_captures` unusable there. **The suggested fix was subtly wrong, so this does something else.** The report proposes `node:path`'s `isAbsolute`, which answers for the machine doing the checking: on Linux, `C:\tools` is not absolute. That works on Windows but silently breaks everywhere a Windows scenario is validated by another host — replay, comparison, or a test asserting on a fixture. A boundary schema must not vary with its evaluator, so the predicate accepts the POSIX form, a Windows drive path and a UNC share on every host, and is exported and unit-tested directly. The rejection message now names the constraint, so a rejected path is actionable rather than surfacing as a generic "invalid request". Two things I checked rather than assumed: - **Whether this only moved the failure downstream.** Containment is enforced by `isPathWithinRoot`, which already uses host-aware `path.relative`, and `ProcessCaptureAuthority` canonicalises both sides before comparing. Execution was never POSIX-only — validation really was the sole blocker. - **Whether the `path` field inside `checkpoints` needed the same treatment.** It is an HTTP path, not a filesystem path, so `startsWith("/")` is correct there and is deliberately unchanged. ## #545 — a denial that blamed the wrong thing When a scope is inside the administrator ceiling but no grant was ever issued, the error still told the caller to "Add the exact missing scope beneath the administrator ceiling". That advice cannot work: the ceiling already permits the operation. The `missing` payload is computed against candidate *grants*, not the ceiling, so it describes what a grant must cover — hence the report's observation that entries in `missing` appear verbatim in `ceiling`. `PermissionRemediation` gains a `grant` variant that says the ceiling already covers the scope and names the two real routes (a project grant, or the documented auto-grant setting). `PermissionAuthority` selects it whenever the denial is not a ceiling violation. Preserved deliberately: a genuine ceiling violation still says `configure`; an explicitly requested `elicit`/`restart` is still honoured; `missing` is unchanged, because it is accurate — it just was not being read as "what a grant must cover". ## On the report's related sharp edge The `executables` directory-vs-exact containment semantics are noted and **not** changed here. That is a real documentation gap, but it is a contract question about what a scope means, not a bug, and folding it into a fix for two unrelated defects would make the change harder to review. Worth its own issue. ## Tests - `processScenario.test.ts`: POSIX, Windows-drive (both separators) and UNC paths all validate **on a Linux host**, Windows `filesystem_roots` accepted, five relative/empty forms still rejected, and the actionable message asserted. - `PermissionAuthorityRemediation.test.ts`: covered scope and an *empty* request both resolve to `grant`; a genuine ceiling violation still resolves to `configure`; an explicit `elicit` is untouched. Full suite green (1987 passed) with the regenerated catalog; typecheck, lint and prettier clean. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:13:54 +00:00