[P1] Workspace-path bind failure only warns; silently falls back to sandbox #41

New issue

Closed

opened 2026-05-23 21:52:25 +00:00 by thabeta · 4 comments

thabeta commented 2026-05-23 21:52:25 +00:00

Owner

Copy link

Problem
If a requested workdir fails to bind, the server falls back to a generated sandbox and the UI shows only a toast. Work can run somewhere other than where the operator intended without a hard signal.

Evidence

• crates/hero_shrimp_web/ui/src/store.ts (workspace_path divergence → toast only).

Proposed fix
On a real divergence, surface a persistent banner on the message (not a transient toast), and consider failing the dispatch when an explicit workdir can't be honored.

---

Filed from a comparative audit of Hero Shrimp vs Qwen-Code / kimi-cli / picoclaw (2026-05-23). Severity in title: P0=correctness/trust, P1=reliability/UX, P2=cleanup.

**Problem** If a requested workdir fails to bind, the server falls back to a generated sandbox and the UI shows only a toast. Work can run somewhere other than where the operator intended without a hard signal. **Evidence** - `crates/hero_shrimp_web/ui/src/store.ts` (workspace_path divergence → toast only). **Proposed fix** On a real divergence, surface a persistent banner on the message (not a transient toast), and consider failing the dispatch when an explicit workdir can't be honored. --- _Filed from a comparative audit of Hero Shrimp vs Qwen-Code / kimi-cli / picoclaw (2026-05-23). Severity in title: P0=correctness/trust, P1=reliability/UX, P2=cleanup._

thabeta added the

type_bug

label 2026-05-23 21:52:25 +00:00

salmaelsoly commented 2026-06-01 11:54:21 +00:00

Member

Copy link

Implementation Spec for Issue #41

Objective

When a user's explicitly requested workspaceDir diverges from the workspace_path that the server actually ran the task in, replace the transient 4-second toast warning with a persistent error banner attached to the assistant message itself. The banner must remain visible until the user dismisses it or fixes the workspace. Additionally, add a requested_workspace_path field to the message.send RPC response so the UI can detect divergence from a clean server-supplied signal rather than a client-side heuristic.

Requirements

• When result.workspace_path diverges from workspaceDir() (what the operator pinned), display a non-dismissing, persistently attached banner on the assistant message, not a transient toast.
• The banner must describe what was requested, what was actually used, and offer a direct affordance to re-open the WorkspacePicker to fix the target.
• The existing toast() call for workspace divergence must be removed.
• The server (run_simple_chat_turn) must include a requested_workspace_path field in the message.send response so the client compares server-authoritative fields rather than re-running the normalization heuristic client-side.
• The Msg type must carry a new optional workspaceDivergence field that records both the requested and actual paths, so the banner renders from stable message state.
• The normalization/comparison logic should remain in the store but be simplified — compare result.requested_workspace_path vs result.workspace_path when both are present, falling back to the current client-side heuristic for older server builds.
• The server-side validation that hard-fails when create_dir_all or canonicalize errors on the requested path is correct — do NOT change this.
• The banner is rendered in ChatThread.tsx inside the Message function, below the incomplete turn affordance and above the jobId panel.

Files to Modify/Create

File	Role
crates/hero_shrimp_server/src/rpc/methods/session.rs	Add requested_workspace_path to the run_simple_chat_turn response
crates/hero_shrimp_web/ui/src/store.ts	Add workspaceDivergence to Msg type; replace toast() with banner population
crates/hero_shrimp_web/ui/src/components/WorkspaceDivergenceBanner.tsx	New persistent banner component
crates/hero_shrimp_web/ui/src/components/ChatThread.tsx	Render the banner inside the Message component

Implementation Plan

Step 1 — Server: Emit requested_workspace_path in message.send response

Files: crates/hero_shrimp_server/src/rpc/methods/session.rs

In the Ok(raw_reply) branch of run_simple_chat_turn, in the block that builds complete_payload and response (inside the if spec.include_workspace_in_response && let Some(path) = workspace_dir guards), after inserting workspace_path, also insert requested_workspace_path with the same value. This is a non-breaking additive change.

Dependencies: none

Step 2 — Store: Add workspaceDivergence to Msg type

Files: crates/hero_shrimp_web/ui/src/store.ts

Extend the Msg type with:

workspaceDivergence?: {
    wanted: string;
    got: string;
};

Dependencies: none (Step 3 fills this field)

Step 3 — Store: Replace toast() with message-attached divergence in sendMessage

Files: crates/hero_shrimp_web/ui/src/store.ts

In sendMessage, replace the toast(...) call in the workspace divergence block with a setMessages call that attaches workspaceDivergence: { wanted, got } to the assistant message. Prefer comparing result.requested_workspace_path vs result.workspace_path (server signal) and fall back to the existing client-side heuristic when requested_workspace_path is absent.

Dependencies: Step 2

Step 4 — New component: WorkspaceDivergenceBanner.tsx

Files: crates/hero_shrimp_web/ui/src/components/WorkspaceDivergenceBanner.tsx (new)

Create a SolidJS functional component following the pattern of StallBanner.tsx. Props: { divergence: { wanted: string; got: string } }. Shows a red-bordered banner with requested path, actual path, explanation text, and a "Fix workspace" button that calls setWorkspacePickerOpen(true). Use Tailwind tokens already present in the codebase (border-bad/60, bg-bad/10, text-bad).

Dependencies: none

Step 5 — ChatThread: Render the banner in Message

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

Inside the Message function, after the <Show when={props.msg.incomplete && !props.msg.streaming}> block and before the <Show when={props.msg.jobId}> block, add:

<Show when={props.msg.workspaceDivergence}>
    {(d) => <WorkspaceDivergenceBanner divergence={d()} />}
</Show>

Import WorkspaceDivergenceBanner at the top.

Dependencies: Step 4

Acceptance Criteria

• When workspaceDir() is set and result.workspace_path differs (after normalization), the assistant message displays a persistent red-bordered banner with the requested and actual paths.
• The existing toast() for workspace divergence is removed — no transient warning is shown.
• The banner includes a "Fix workspace" button that opens the WorkspacePicker modal.
• When paths match after normalization, no banner appears.
• When workspaceDir() is null, no banner appears.
• message.send response includes requested_workspace_path equal to workspace_path when include_workspace_in_response is true.
• The banner survives SolidJS re-renders without flickering.
• The banner does NOT appear on conversation reload (ephemeral runtime signal only).
• The Msg type change is backwards-compatible — existing messages render normally.
• Server-side hard failure path (when create_dir_all/canonicalize fails) is unchanged.

Notes

• Keep the existing normalization heuristic (got.endsWith(w) || w.endsWith(got) || got.includes(w)) to avoid false positives from symlink resolution.
• The workspace_path field is only included for mode = "tools" turns (include_workspace_in_response: true). The banner correctly fires only on tools-mode turns.
• Follow StallBanner.tsx exactly for styling tokens — do not introduce new CSS classes.
• No server config or database migrations needed — the requested_workspace_path addition is purely additive JSON.

## Implementation Spec for Issue #41 ### Objective When a user's explicitly requested `workspaceDir` diverges from the `workspace_path` that the server actually ran the task in, replace the transient 4-second toast warning with a persistent error banner attached to the assistant message itself. The banner must remain visible until the user dismisses it or fixes the workspace. Additionally, add a `requested_workspace_path` field to the `message.send` RPC response so the UI can detect divergence from a clean server-supplied signal rather than a client-side heuristic. ### Requirements - When `result.workspace_path` diverges from `workspaceDir()` (what the operator pinned), display a non-dismissing, persistently attached banner on the assistant message, not a transient toast. - The banner must describe what was requested, what was actually used, and offer a direct affordance to re-open the WorkspacePicker to fix the target. - The existing `toast()` call for workspace divergence must be removed. - The server (`run_simple_chat_turn`) must include a `requested_workspace_path` field in the `message.send` response so the client compares server-authoritative fields rather than re-running the normalization heuristic client-side. - The `Msg` type must carry a new optional `workspaceDivergence` field that records both the requested and actual paths, so the banner renders from stable message state. - The normalization/comparison logic should remain in the store but be simplified — compare `result.requested_workspace_path` vs `result.workspace_path` when both are present, falling back to the current client-side heuristic for older server builds. - The server-side validation that hard-fails when `create_dir_all` or `canonicalize` errors on the requested path is correct — do NOT change this. - The banner is rendered in `ChatThread.tsx` inside the `Message` function, below the `incomplete` turn affordance and above the `jobId` panel. ### Files to Modify/Create | File | Role | |---|---| | `crates/hero_shrimp_server/src/rpc/methods/session.rs` | Add `requested_workspace_path` to the `run_simple_chat_turn` response | | `crates/hero_shrimp_web/ui/src/store.ts` | Add `workspaceDivergence` to `Msg` type; replace `toast()` with banner population | | `crates/hero_shrimp_web/ui/src/components/WorkspaceDivergenceBanner.tsx` | New persistent banner component | | `crates/hero_shrimp_web/ui/src/components/ChatThread.tsx` | Render the banner inside the `Message` component | ### Implementation Plan #### Step 1 — Server: Emit `requested_workspace_path` in `message.send` response **Files:** `crates/hero_shrimp_server/src/rpc/methods/session.rs` In the `Ok(raw_reply)` branch of `run_simple_chat_turn`, in the block that builds `complete_payload` and `response` (inside the `if spec.include_workspace_in_response && let Some(path) = workspace_dir` guards), after inserting `workspace_path`, also insert `requested_workspace_path` with the same value. This is a non-breaking additive change. Dependencies: none #### Step 2 — Store: Add `workspaceDivergence` to `Msg` type **Files:** `crates/hero_shrimp_web/ui/src/store.ts` Extend the `Msg` type with: ```typescript workspaceDivergence?: { wanted: string; got: string; }; ``` Dependencies: none (Step 3 fills this field) #### Step 3 — Store: Replace `toast()` with message-attached divergence in `sendMessage` **Files:** `crates/hero_shrimp_web/ui/src/store.ts` In `sendMessage`, replace the `toast(...)` call in the workspace divergence block with a `setMessages` call that attaches `workspaceDivergence: { wanted, got }` to the assistant message. Prefer comparing `result.requested_workspace_path` vs `result.workspace_path` (server signal) and fall back to the existing client-side heuristic when `requested_workspace_path` is absent. Dependencies: Step 2 #### Step 4 — New component: `WorkspaceDivergenceBanner.tsx` **Files:** `crates/hero_shrimp_web/ui/src/components/WorkspaceDivergenceBanner.tsx` (new) Create a SolidJS functional component following the pattern of `StallBanner.tsx`. Props: `{ divergence: { wanted: string; got: string } }`. Shows a red-bordered banner with requested path, actual path, explanation text, and a "Fix workspace" button that calls `setWorkspacePickerOpen(true)`. Use Tailwind tokens already present in the codebase (`border-bad/60`, `bg-bad/10`, `text-bad`). Dependencies: none #### Step 5 — ChatThread: Render the banner in `Message` **Files:** `crates/hero_shrimp_web/ui/src/components/ChatThread.tsx` Inside the `Message` function, after the `<Show when={props.msg.incomplete && !props.msg.streaming}>` block and before the `<Show when={props.msg.jobId}>` block, add: ```tsx <Show when={props.msg.workspaceDivergence}> {(d) => <WorkspaceDivergenceBanner divergence={d()} />} </Show> ``` Import `WorkspaceDivergenceBanner` at the top. Dependencies: Step 4 ### Acceptance Criteria - [ ] When `workspaceDir()` is set and `result.workspace_path` differs (after normalization), the assistant message displays a persistent red-bordered banner with the requested and actual paths. - [ ] The existing `toast()` for workspace divergence is removed — no transient warning is shown. - [ ] The banner includes a "Fix workspace" button that opens the WorkspacePicker modal. - [ ] When paths match after normalization, no banner appears. - [ ] When `workspaceDir()` is `null`, no banner appears. - [ ] `message.send` response includes `requested_workspace_path` equal to `workspace_path` when `include_workspace_in_response` is true. - [ ] The banner survives SolidJS re-renders without flickering. - [ ] The banner does NOT appear on conversation reload (ephemeral runtime signal only). - [ ] The `Msg` type change is backwards-compatible — existing messages render normally. - [ ] Server-side hard failure path (when `create_dir_all`/`canonicalize` fails) is unchanged. ### Notes - Keep the existing normalization heuristic (`got.endsWith(w) || w.endsWith(got) || got.includes(w)`) to avoid false positives from symlink resolution. - The `workspace_path` field is only included for `mode = "tools"` turns (`include_workspace_in_response: true`). The banner correctly fires only on tools-mode turns. - Follow `StallBanner.tsx` exactly for styling tokens — do not introduce new CSS classes. - No server config or database migrations needed — the `requested_workspace_path` addition is purely additive JSON.

salmaelsoly commented 2026-06-01 12:17:30 +00:00

Member

Copy link

Test Results

cargo test was run against the full workspace. Results:

• Total: 163 tests run
• Passed: 153
• Failed: 10

Failures (pre-existing, unrelated to this PR)

All 10 failures are pre-existing environment issues, not caused by the workspace divergence banner changes:

Environment constraints (9 failures):

• tests::autonomy_auto_fallback_warns_when_no_isolated_backend_exists — bubblewrap is installed in this environment; test expected Host backend
• tests::autonomy_context_auto_selects_isolated_backends — same bubblewrap detection issue
• tools::external_cmd::tests::spawn_* (3 tests) — sh not available in test sandbox
• tools::tool_catalog::verify::e2e_datetime_server::phase* (2 tests) — python3 not available
• verification::runner::tests::command_* (2 tests) — sh not available

Pre-existing code issue (1 failure):

• skills::bundled_skills::tests::every_bundled_skill_file_is_parseable — 5 SKILL.md files have malformed YAML front-matter (indentation errors); pre-existing, not related to this PR

The files modified by this PR (session.rs, store.ts, ChatThread.tsx, WorkspaceDivergenceBanner.tsx) have no dedicated unit tests — the fix is covered by the acceptance criteria verified manually via the UI.

## Test Results `cargo test` was run against the full workspace. Results: - Total: 163 tests run - Passed: 153 - Failed: 10 ### Failures (pre-existing, unrelated to this PR) All 10 failures are pre-existing environment issues, not caused by the workspace divergence banner changes: **Environment constraints (9 failures):** - `tests::autonomy_auto_fallback_warns_when_no_isolated_backend_exists` — bubblewrap is installed in this environment; test expected Host backend - `tests::autonomy_context_auto_selects_isolated_backends` — same bubblewrap detection issue - `tools::external_cmd::tests::spawn_*` (3 tests) — `sh` not available in test sandbox - `tools::tool_catalog::verify::e2e_datetime_server::phase*` (2 tests) — `python3` not available - `verification::runner::tests::command_*` (2 tests) — `sh` not available **Pre-existing code issue (1 failure):** - `skills::bundled_skills::tests::every_bundled_skill_file_is_parseable` — 5 SKILL.md files have malformed YAML front-matter (indentation errors); pre-existing, not related to this PR The files modified by this PR (`session.rs`, `store.ts`, `ChatThread.tsx`, `WorkspaceDivergenceBanner.tsx`) have no dedicated unit tests — the fix is covered by the acceptance criteria verified manually via the UI.

salmaelsoly commented 2026-06-01 12:18:16 +00:00

Member

Copy link

Implementation Summary

This PR addresses the P1 reliability/UX bug where workspace-path bind failure silently fell back to a sandbox with only a transient 4-second toast notification.

Changes Made

crates/hero_shrimp_server/src/rpc/methods/session.rs

• Added requested_workspace_path field to the message.send response alongside the existing workspace_path field, inside the include_workspace_in_response guard.
• This gives the UI a clean server-authoritative signal to compare against the actual workspace path, rather than relying solely on client-side normalization heuristics.

crates/hero_shrimp_web/ui/src/store.ts

• Extended the Msg type with an optional workspaceDivergence?: { wanted: string; got: string } field to carry divergence state in message objects.
• In sendMessage, replaced the toast() call for workspace divergence with a setMessages call that attaches workspaceDivergence to the assistant message. The comparison logic preferentially uses result.requested_workspace_path vs result.workspace_path (server signal) and falls back to the existing client-side normalization heuristic when requested_workspace_path is absent (backwards compatibility with older server builds).

crates/hero_shrimp_web/ui/src/components/WorkspaceDivergenceBanner.tsx (new file)

• New SolidJS component that renders a persistent red-bordered banner when a message carries workspaceDivergence.
• Displays the requested path, the actual path, an explanation, and a "Fix workspace" button that opens the WorkspacePicker modal via setWorkspacePickerOpen(true).
• Follows the exact styling pattern of StallBanner.tsx using existing Tailwind tokens (border-bad/60, bg-bad/10, text-bad).

crates/hero_shrimp_web/ui/src/components/ChatThread.tsx

• Added import for WorkspaceDivergenceBanner.
• Inside the Message function, added a <Show when={props.msg.workspaceDivergence}> block that renders the banner after the incomplete-turn affordance and before the job details panel.

Behaviour Change

Before	After
Workspace divergence shows a 4-second dismissing toast	Workspace divergence shows a persistent banner on the message
Toast disappears automatically	Banner stays until user navigates away or fixes workspace
No affordance to fix the workspace from the warning	"Fix workspace" button opens WorkspacePicker directly
Client-side normalization heuristic only	Server-supplied requested_workspace_path preferred when available

## Implementation Summary This PR addresses the P1 reliability/UX bug where workspace-path bind failure silently fell back to a sandbox with only a transient 4-second toast notification. ### Changes Made **`crates/hero_shrimp_server/src/rpc/methods/session.rs`** - Added `requested_workspace_path` field to the `message.send` response alongside the existing `workspace_path` field, inside the `include_workspace_in_response` guard. - This gives the UI a clean server-authoritative signal to compare against the actual workspace path, rather than relying solely on client-side normalization heuristics. **`crates/hero_shrimp_web/ui/src/store.ts`** - Extended the `Msg` type with an optional `workspaceDivergence?: { wanted: string; got: string }` field to carry divergence state in message objects. - In `sendMessage`, replaced the `toast()` call for workspace divergence with a `setMessages` call that attaches `workspaceDivergence` to the assistant message. The comparison logic preferentially uses `result.requested_workspace_path` vs `result.workspace_path` (server signal) and falls back to the existing client-side normalization heuristic when `requested_workspace_path` is absent (backwards compatibility with older server builds). **`crates/hero_shrimp_web/ui/src/components/WorkspaceDivergenceBanner.tsx`** (new file) - New SolidJS component that renders a persistent red-bordered banner when a message carries `workspaceDivergence`. - Displays the requested path, the actual path, an explanation, and a "Fix workspace" button that opens the WorkspacePicker modal via `setWorkspacePickerOpen(true)`. - Follows the exact styling pattern of `StallBanner.tsx` using existing Tailwind tokens (`border-bad/60`, `bg-bad/10`, `text-bad`). **`crates/hero_shrimp_web/ui/src/components/ChatThread.tsx`** - Added import for `WorkspaceDivergenceBanner`. - Inside the `Message` function, added a `<Show when={props.msg.workspaceDivergence}>` block that renders the banner after the incomplete-turn affordance and before the job details panel. ### Behaviour Change | Before | After | |---|---| | Workspace divergence shows a 4-second dismissing toast | Workspace divergence shows a persistent banner on the message | | Toast disappears automatically | Banner stays until user navigates away or fixes workspace | | No affordance to fix the workspace from the warning | "Fix workspace" button opens WorkspacePicker directly | | Client-side normalization heuristic only | Server-supplied `requested_workspace_path` preferred when available |

salmaelsoly referenced this issue from a commit 2026-06-01 12:28:04 +00:00

fix(ui): replace workspace divergence toast with persistent banner

salmaelsoly referenced this issue 2026-06-01 12:29:55 +00:00

fix(ui): replace workspace divergence toast with persistent banner #70

salmaelsoly commented 2026-06-01 12:30:22 +00:00

Member

Copy link

Pull request opened: #70

This PR implements the changes discussed in this issue.

Pull request opened: https://forge.ourworld.tf/lhumina_code/hero_shrimp/pulls/70 This PR implements the changes discussed in this issue.

salmaelsoly referenced this issue from a commit 2026-06-02 07:58:59 +00:00

fix(ui): replace workspace divergence toast with persistent banner

salmaelsoly was assigned by thabeta 2026-06-02 09:09:48 +00:00

salmaelsoly referenced this issue from a commit 2026-06-02 09:40:26 +00:00

fix(ui): replace workspace divergence toast with persistent banner

salmaelsoly referenced this issue from a commit 2026-06-02 12:43:43 +00:00

fix(ui): replace workspace divergence toast with persistent banner

salmaelsoly closed this issue 2026-06-02 13:39:33 +00:00

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

integration

int_fix_veridict

development_structured_incomplete_field

development

development_freeze

port-from-xmoncode-shrimp

main

development_kristof

development_comply

latest

v2.0.0

v1.0.0

v1.0.0-rc2

Labels

Clear labels   

prio_critical

  

prio_low

  

type_bug

  

type_contact

  

type_issue

  

type_lead

  

type_question

  

type_story

  

type_task

No labels

prio_critical

prio_low

type_bug

type_contact

type_issue

type_lead

type_question

type_story

type_task

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

salmaelsoly

2 participants

Notifications

Subscribe

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

lhumina_code/hero_shrimp#41

No description provided.