lhumina_code/hero_shrimp

Fork 0

JSON-RPC fs.read_file fails #74

New issue

Closed

opened 2026-06-02 08:04:11 +00:00 by rawan · 4 comments

rawan commented

2026-06-02 08:04:11 +00:00

Member

The path exists, it can't be accessed
Repro:

open crew panel, ask the agents to create any app that output files
try to access the file, you'll get the error

The path exists, it can't be accessed Repro: - open crew panel, ask the agents to create any app that output files - try to access the file, you'll get the error ![image](/attachments/9e679766-5348-4b45-a755-5a77c757583b)

image.png

41 KiB

rawan commented

2026-06-02 12:24:44 +00:00

Author

Member

Implementation Spec for Issue #74 — JSON-RPC `fs.read_file` fails

Objective

Make file chips for crew/agent-created files open successfully. Today, clicking a file the crew agents produced fails with a "path can't be accessed" error even though the file exists, because the relative path is sent to fs.read_file (which resolves against the global workspace / $HOME) instead of job.file (which resolves inside the owning job's sandbox).

Root Cause (verified in code)

Crew/agent runs execute with a per-run context.workspace_dir that is the job's sandbox subdirectory, not the daemon's global config.workspace_dir. "Wrote file" cards emit abs_path = context.workspace_dir.join(path) (crates/hero_shrimp_engine/src/execution/executor/mod.rs:104-113).
In the chat/crew timeline, file paths become clickable chips via LinkedText (crates/hero_shrimp_web/ui/src/components/LinkedText.tsx). The crew form is a backtick-fenced relative path; the click handler calls setOpenFile(path) with no job id — it does not use the existing openJobFile(jobId, path) helper.
Because openFileJobId() is null, FileModal.loadFile (FileModal.tsx:22-32) skips the job.file branch (the only branch that resolves a relative path inside the job's sandbox) and falls through to fs.read_file with the relative path.
On the backend, method_fs_read_file (crates/hero_shrimp_server/src/rpc/methods/fs.rs) calls check_read_access → resolve_user_path (fs_policy.rs:241-282) resolves a bare relative path against the global config.workspace_dir then $HOME. The crew file lives in a job sandbox dir that is neither, so canonicalize fails → "can't be accessed".

This is the companion of the already-merged fs.list fix (commit 6f95dfe2), which corrected frontend method wiring but did not address job-relative file chips in LinkedText. The bug class is already documented in store.ts:440-444.

Requirements

Clicking a crew/agent-created file chip must open the file content, not an error.
File chips that originate from a job must carry the owning job id so FileModal reads via job.file (sandbox-aware) rather than fs.read_file.
Defense-in-depth: fs.read_file/check_read_access for a relative path should also try known per-job sandbox workspace roots before failing.
Absolute paths and ~/ paths must keep working exactly as before.
No regression to the security blocklist or strict mode (SHRIMP_FS_RESTRICT_TO_WORKSPACE).
Preserve the existing binary-file preview UX.

Files to Modify

crates/hero_shrimp_web/ui/src/components/LinkedText.tsx — accept optional jobId prop; call openJobFile(jobId, path) instead of setOpenFile(path).
crates/hero_shrimp_web/ui/src/store.ts — thread owning job id into the linker; confirm openJobFile is the single entry point.
Call sites rendering LinkedText for job/crew message text — pass the message's job id.
crates/hero_shrimp_server/src/rpc/fs_policy.rs — in resolve_user_path, probe known job sandbox workspace roots for relative paths before returning the non-existent $HOME candidate.
crates/hero_shrimp_web/ui/src/components/FileModal.tsx — remove the hardcoded /home/driver ~ substitution; let the backend expand ~.

Implementation Plan

Step 1: Backend — resolve relative paths against job sandbox roots

Files: crates/hero_shrimp_server/src/rpc/fs_policy.rs

In resolve_user_path (lines 241-282), after the workspace_dir probe and before the $HOME fallback, iterate known job-sandbox roots (job_artifact_dir / job_workspace_path); return the first candidate that .exists().
Keep it existence-gated so behavior for already-resolving paths is unchanged; downstream canonicalize + blocklist + strict-mode checks still run.
Dependencies: none

Step 2: Frontend — make `LinkedText` job-aware

Files: crates/hero_shrimp_web/ui/src/components/LinkedText.tsx

Add optional jobId?: string | null prop; in the path-chip onClick, call openJobFile(props.jobId ?? null, t.value); import openJobFile from ../store.
Dependencies: relies on existing openJobFile (store.ts:448-451)

Step 3: Frontend — pass job id from message-rendering call sites

Files: components rendering <LinkedText> for job/crew content (enumerate via grep)

Pass jobId={...} from the surrounding message/job context; omit for non-job surfaces.
Dependencies: Step 2

Step 4: Frontend — remove `/home/driver` hardcode in FileModal

Files: crates/hero_shrimp_web/ui/src/components/FileModal.tsx

Drop path.replace(/^~/, '/home/driver'); pass ~/... straight to fs.read_file (backend expands ~).
Dependencies: independent

Step 5: Rebuild bundled UI assets

Regenerate crates/hero_shrimp_web/static/v2/assets/app.*.js and static/index.html so the served bundle matches the TSX changes.
Dependencies: Steps 2-4

Step 6: Tests

Files: crates/hero_shrimp_server/src/rpc/fs_policy.rs

Add a unit test asserting a relative path present only under a job-sandbox root resolves, while one present nowhere still fails with the existing invalid_params error.
Confirm existing fs_policy tests still pass.

Acceptance Criteria

In the crew panel, creating an app that outputs files then clicking a produced file opens its content (no error).
Crew/job file chips carry the owning job id and read via job.file.
fs.read_file with a relative path that exists only in a job sandbox resolves; one that exists nowhere still returns the existing error.
Absolute and ~/ paths still open; ~ expands to the server's real $HOME (no /home/driver hardcode).
Binary files still show the "preview not available" notice.
Security blocklist and strict mode unchanged; all fs_policy tests pass plus the new test.
Built UI assets regenerated and committed.

Notes

The bug is documented in store.ts:440-444: job-owned files fail via fs.read_file and must use job.file. LinkedText is the one chat surface that never adopted openJobFile.
Strict mode is off by default, so it is not the cause.
The frontend fix (Steps 2-3) is the primary correctness fix; the backend fallback (Step 1) is defense-in-depth.

## Implementation Spec for Issue #74 — JSON-RPC `fs.read_file` fails ### Objective Make file chips for crew/agent-created files open successfully. Today, clicking a file the crew agents produced fails with a "path can't be accessed" error even though the file exists, because the relative path is sent to `fs.read_file` (which resolves against the global workspace / `$HOME`) instead of `job.file` (which resolves inside the owning job's sandbox). ### Root Cause (verified in code) 1. Crew/agent runs execute with a per-run `context.workspace_dir` that is the job's sandbox subdirectory, not the daemon's global `config.workspace_dir`. "Wrote file" cards emit `abs_path = context.workspace_dir.join(path)` (`crates/hero_shrimp_engine/src/execution/executor/mod.rs:104-113`). 2. In the chat/crew timeline, file paths become clickable chips via `LinkedText` (`crates/hero_shrimp_web/ui/src/components/LinkedText.tsx`). The crew form is a backtick-fenced relative path; the click handler calls `setOpenFile(path)` with no job id — it does not use the existing `openJobFile(jobId, path)` helper. 3. Because `openFileJobId()` is null, `FileModal.loadFile` (`FileModal.tsx:22-32`) skips the `job.file` branch (the only branch that resolves a relative path inside the job's sandbox) and falls through to `fs.read_file` with the relative path. 4. On the backend, `method_fs_read_file` (`crates/hero_shrimp_server/src/rpc/methods/fs.rs`) calls `check_read_access` → `resolve_user_path` (`fs_policy.rs:241-282`) resolves a bare relative path against the global `config.workspace_dir` then `$HOME`. The crew file lives in a job sandbox dir that is neither, so `canonicalize` fails → "can't be accessed". This is the companion of the already-merged `fs.list` fix (commit `6f95dfe2`), which corrected frontend method wiring but did not address job-relative file chips in `LinkedText`. The bug class is already documented in `store.ts:440-444`. ### Requirements - Clicking a crew/agent-created file chip must open the file content, not an error. - File chips that originate from a job must carry the owning job id so `FileModal` reads via `job.file` (sandbox-aware) rather than `fs.read_file`. - Defense-in-depth: `fs.read_file`/`check_read_access` for a relative path should also try known per-job sandbox workspace roots before failing. - Absolute paths and `~/` paths must keep working exactly as before. - No regression to the security blocklist or strict mode (`SHRIMP_FS_RESTRICT_TO_WORKSPACE`). - Preserve the existing binary-file preview UX. ### Files to Modify - `crates/hero_shrimp_web/ui/src/components/LinkedText.tsx` — accept optional `jobId` prop; call `openJobFile(jobId, path)` instead of `setOpenFile(path)`. - `crates/hero_shrimp_web/ui/src/store.ts` — thread owning job id into the linker; confirm `openJobFile` is the single entry point. - Call sites rendering `LinkedText` for job/crew message text — pass the message's job id. - `crates/hero_shrimp_server/src/rpc/fs_policy.rs` — in `resolve_user_path`, probe known job sandbox workspace roots for relative paths before returning the non-existent `$HOME` candidate. - `crates/hero_shrimp_web/ui/src/components/FileModal.tsx` — remove the hardcoded `/home/driver` `~` substitution; let the backend expand `~`. ### Implementation Plan #### Step 1: Backend — resolve relative paths against job sandbox roots Files: `crates/hero_shrimp_server/src/rpc/fs_policy.rs` - In `resolve_user_path` (lines 241-282), after the `workspace_dir` probe and before the `$HOME` fallback, iterate known job-sandbox roots (`job_artifact_dir` / `job_workspace_path`); return the first candidate that `.exists()`. - Keep it existence-gated so behavior for already-resolving paths is unchanged; downstream `canonicalize` + blocklist + strict-mode checks still run. Dependencies: none #### Step 2: Frontend — make `LinkedText` job-aware Files: `crates/hero_shrimp_web/ui/src/components/LinkedText.tsx` - Add optional `jobId?: string | null` prop; in the path-chip `onClick`, call `openJobFile(props.jobId ?? null, t.value)`; import `openJobFile` from `../store`. Dependencies: relies on existing `openJobFile` (`store.ts:448-451`) #### Step 3: Frontend — pass job id from message-rendering call sites Files: components rendering `<LinkedText>` for job/crew content (enumerate via grep) - Pass `jobId={...}` from the surrounding message/job context; omit for non-job surfaces. Dependencies: Step 2 #### Step 4: Frontend — remove `/home/driver` hardcode in FileModal Files: `crates/hero_shrimp_web/ui/src/components/FileModal.tsx` - Drop `path.replace(/^~/, '/home/driver')`; pass `~/...` straight to `fs.read_file` (backend expands `~`). Dependencies: independent #### Step 5: Rebuild bundled UI assets - Regenerate `crates/hero_shrimp_web/static/v2/assets/app.*.js` and `static/index.html` so the served bundle matches the TSX changes. Dependencies: Steps 2-4 #### Step 6: Tests Files: `crates/hero_shrimp_server/src/rpc/fs_policy.rs` - Add a unit test asserting a relative path present only under a job-sandbox root resolves, while one present nowhere still fails with the existing `invalid_params` error. - Confirm existing `fs_policy` tests still pass. ### Acceptance Criteria - [ ] In the crew panel, creating an app that outputs files then clicking a produced file opens its content (no error). - [ ] Crew/job file chips carry the owning job id and read via `job.file`. - [ ] `fs.read_file` with a relative path that exists only in a job sandbox resolves; one that exists nowhere still returns the existing error. - [ ] Absolute and `~/` paths still open; `~` expands to the server's real `$HOME` (no `/home/driver` hardcode). - [ ] Binary files still show the "preview not available" notice. - [ ] Security blocklist and strict mode unchanged; all `fs_policy` tests pass plus the new test. - [ ] Built UI assets regenerated and committed. ### Notes - The bug is documented in `store.ts:440-444`: job-owned files fail via `fs.read_file` and must use `job.file`. `LinkedText` is the one chat surface that never adopted `openJobFile`. - Strict mode is off by default, so it is not the cause. - The frontend fix (Steps 2-3) is the primary correctness fix; the backend fallback (Step 1) is defense-in-depth.

rawan commented

2026-06-02 12:55:48 +00:00

Author

Member

Test Results

Backend (cargo test -p hero_shrimp_server):

Total: 296
Passed: 296
Failed: 0

Includes the new unit test relative_path_resolves_against_job_sandbox in rpc::fs_policy, which asserts that a relative path present only under a job sandbox root resolves, while a relative path that exists nowhere still returns the existing invalid_params "Cannot resolve" error. All pre-existing fs_policy tests (tilde_expands_to_home, relative_paths_resolve_against_home, plain_existing_file_allowed_in_default_mode, rejects_home_ssh_subpath, strict_mode_rejects_outside_workspace) still pass.

Frontend: cargo build -p hero_shrimp_web succeeds; tsc --noEmit reports no new type errors in the touched files; vite build regenerated the bundled assets (static/assets/app.D1-tO6Ad.js, static/index.html).

## Test Results Backend (`cargo test -p hero_shrimp_server`): - Total: 296 - Passed: 296 - Failed: 0 Includes the new unit test `relative_path_resolves_against_job_sandbox` in `rpc::fs_policy`, which asserts that a relative path present only under a job sandbox root resolves, while a relative path that exists nowhere still returns the existing `invalid_params` "Cannot resolve" error. All pre-existing `fs_policy` tests (`tilde_expands_to_home`, `relative_paths_resolve_against_home`, `plain_existing_file_allowed_in_default_mode`, `rejects_home_ssh_subpath`, `strict_mode_rejects_outside_workspace`) still pass. Frontend: `cargo build -p hero_shrimp_web` succeeds; `tsc --noEmit` reports no new type errors in the touched files; `vite build` regenerated the bundled assets (`static/assets/app.D1-tO6Ad.js`, `static/index.html`).

rawan commented

2026-06-02 12:55:58 +00:00

Author

Member

Summary

Fixes fs.read_file failing for crew/agent-created files. Root cause: those files live in a per-job sandbox (<workspace>/jobs/<job_id>/...), but the clickable file chips in chat/crew text were calling fs.read_file with a bare relative path and no job id. fs.read_file only resolved relative paths against the global workspace_dir and $HOME, so the file "existed but couldn't be accessed."

Changes

Backend:

crates/hero_shrimp_server/src/rpc/fs_policy.rs - resolve_user_path now probes each live job's sandbox workspace root for a relative path (after the global workspace_dir probe, before the $HOME fallback). Existence-gated, so it never changes resolution for paths that already resolve. Added a unit test covering the job-sandbox hit and the still-failing nowhere case.

Frontend (file chips are now job-aware, routing through job.file which resolves inside the owning job's sandbox):

crates/hero_shrimp_web/ui/src/components/LinkedText.tsx - accepts an optional jobId prop; path chips now call openJobFile(jobId, path) instead of setOpenFile(path).
crates/hero_shrimp_web/ui/src/components/Markdown.tsx - threads an optional jobId through InlineRender to LinkedText and the inline-code path chip.
crates/hero_shrimp_web/ui/src/components/MessageBody.tsx - forwards jobId to Markdown.
crates/hero_shrimp_web/ui/src/components/ChatThread.tsx - passes the message's jobId to MessageBody (main body and job-segment "say").
crates/hero_shrimp_web/ui/src/components/CrewPage.tsx - passes m.job_id to the crew-message LinkedText.
crates/hero_shrimp_web/ui/src/components/JobDrawer.tsx - passes the job's id to the goal/error LinkedText.
crates/hero_shrimp_web/ui/src/components/FileModal.tsx - removed the hardcoded /home/driver substitution for ~; the path is passed through and the backend expands ~ to the server's real $HOME.
Rebuilt bundled UI assets.

The frontend job-id threading is the primary correctness fix; the backend job-sandbox probe is defense-in-depth for any chip that still arrives without a job id.

Notes

Absolute and ~/ paths keep working via fs.read_file.
Security blocklist and SHRIMP_FS_RESTRICT_TO_WORKSPACE strict mode are unchanged.
Browse-only surfaces with no job context (LibraryPage, KanbanBoard) are unchanged and rely on the backend fallback.

## Summary Fixes `fs.read_file` failing for crew/agent-created files. Root cause: those files live in a per-job sandbox (`<workspace>/jobs/<job_id>/...`), but the clickable file chips in chat/crew text were calling `fs.read_file` with a bare relative path and no job id. `fs.read_file` only resolved relative paths against the global `workspace_dir` and `$HOME`, so the file "existed but couldn't be accessed." ### Changes Backend: - `crates/hero_shrimp_server/src/rpc/fs_policy.rs` - `resolve_user_path` now probes each live job's sandbox workspace root for a relative path (after the global `workspace_dir` probe, before the `$HOME` fallback). Existence-gated, so it never changes resolution for paths that already resolve. Added a unit test covering the job-sandbox hit and the still-failing nowhere case. Frontend (file chips are now job-aware, routing through `job.file` which resolves inside the owning job's sandbox): - `crates/hero_shrimp_web/ui/src/components/LinkedText.tsx` - accepts an optional `jobId` prop; path chips now call `openJobFile(jobId, path)` instead of `setOpenFile(path)`. - `crates/hero_shrimp_web/ui/src/components/Markdown.tsx` - threads an optional `jobId` through `InlineRender` to `LinkedText` and the inline-code path chip. - `crates/hero_shrimp_web/ui/src/components/MessageBody.tsx` - forwards `jobId` to `Markdown`. - `crates/hero_shrimp_web/ui/src/components/ChatThread.tsx` - passes the message's `jobId` to `MessageBody` (main body and job-segment "say"). - `crates/hero_shrimp_web/ui/src/components/CrewPage.tsx` - passes `m.job_id` to the crew-message `LinkedText`. - `crates/hero_shrimp_web/ui/src/components/JobDrawer.tsx` - passes the job's id to the goal/error `LinkedText`. - `crates/hero_shrimp_web/ui/src/components/FileModal.tsx` - removed the hardcoded `/home/driver` substitution for `~`; the path is passed through and the backend expands `~` to the server's real `$HOME`. - Rebuilt bundled UI assets. The frontend job-id threading is the primary correctness fix; the backend job-sandbox probe is defense-in-depth for any chip that still arrives without a job id. ### Notes - Absolute and `~/` paths keep working via `fs.read_file`. - Security blocklist and `SHRIMP_FS_RESTRICT_TO_WORKSPACE` strict mode are unchanged. - Browse-only surfaces with no job context (LibraryPage, KanbanBoard) are unchanged and rely on the backend fallback.

rawan commented

2026-06-02 14:27:51 +00:00

Author

Member

Correction — actual root cause and revised fix

The earlier spec on this issue mis-diagnosed the cause (it assumed job-sandbox relative-path resolution). After reproducing against the real reply, here is what actually breaks, with evidence.

What actually fails

In a crew reply, deliverables are rendered as a table:

| File | Size | Location |
| `Student_Grades.xlsx` | 8.5 KB | `/home/rawan/hero/var/shrimp/workspace/` |
| `create_grades.py`    | 12.6 KB | `/home/rawan/hero/var/shrimp/workspace/` |

The crew panel feeds this raw text to the universal linker (LinkedText), which linkifies paths. Tested against the live link regexes:

Student_Grades.xlsx — matches no chip rule (no /) -> not clickable
create_grades.py — matches no chip rule (no /) -> not clickable
/home/rawan/hero/var/shrimp/workspace/ — matches the absolute-path rule -> the only clickable chip

So the only thing the user can click is the directory. fs.read_file on a directory returns "... is not a file". That is why every file type fails to open — it is not specific to binaries, and the files themselves exist and are readable.

Fix

Make backtick-fenced bare filenames (no directory part) clickable chips, in both the universal linker and the markdown inline-code path:

crates/hero_shrimp_web/ui/src/components/LinkedText.tsx
crates/hero_shrimp_web/ui/src/components/Markdown.tsx

Clicking resolves the bare filename against the workspace root server-side (existing resolve_user_path behaviour, unchanged), so the actual file opens. The match is gated behind a known file-extension allowlist so ordinary code talk (array.length, obj.map) and version strings (1.2) are not linkified.

No backend changes. The bundled UI assets are rebuilt.

Known caveat

A binary deliverable (e.g. the .xlsx) now reaches fs.read_file, which still rejects non-UTF-8 content with a "... is not UTF-8" message. Text deliverables open normally. Graceful binary handling (preview/download) is tracked separately and not part of this change.

## Correction — actual root cause and revised fix The earlier spec on this issue mis-diagnosed the cause (it assumed job-sandbox relative-path resolution). After reproducing against the real reply, here is what actually breaks, with evidence. ### What actually fails In a crew reply, deliverables are rendered as a table: ``` | File | Size | Location | | `Student_Grades.xlsx` | 8.5 KB | `/home/rawan/hero/var/shrimp/workspace/` | | `create_grades.py` | 12.6 KB | `/home/rawan/hero/var/shrimp/workspace/` | ``` The crew panel feeds this raw text to the universal linker (`LinkedText`), which linkifies paths. Tested against the live link regexes: - `Student_Grades.xlsx` — matches no chip rule (no `/`) -> not clickable - `create_grades.py` — matches no chip rule (no `/`) -> not clickable - `/home/rawan/hero/var/shrimp/workspace/` — matches the absolute-path rule -> the only clickable chip So the only thing the user can click is the **directory**. `fs.read_file` on a directory returns `"... is not a file"`. That is why every file type fails to open — it is not specific to binaries, and the files themselves exist and are readable. ### Fix Make backtick-fenced **bare filenames** (no directory part) clickable chips, in both the universal linker and the markdown inline-code path: - `crates/hero_shrimp_web/ui/src/components/LinkedText.tsx` - `crates/hero_shrimp_web/ui/src/components/Markdown.tsx` Clicking resolves the bare filename against the workspace root server-side (existing `resolve_user_path` behaviour, unchanged), so the actual file opens. The match is gated behind a known file-extension allowlist so ordinary code talk (`array.length`, `obj.map`) and version strings (`1.2`) are not linkified. No backend changes. The bundled UI assets are rebuilt. ### Known caveat A binary deliverable (e.g. the `.xlsx`) now reaches `fs.read_file`, which still rejects non-UTF-8 content with a `"... is not UTF-8"` message. Text deliverables open normally. Graceful binary handling (preview/download) is tracked separately and not part of this change.