fix(webui): scope workspace and memory views - #5831
serrrfirat wants to merge 10 commits into
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: L
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThreads a scoped-workspace flag from serve config into session and frontend context, rewires workspace filesystem resolution for scoped workspace and memory mounts, and scopes local-dev workspace mounts and write routing. ChangesScoped projection flag plumbing
Workspace filesystem scoping
Local-dev workspace mount scoping
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ServeCommand
participant WebuiServeConfig
participant WebUiV2State
participant SessionHandler
ServeCommand->>WebuiServeConfig: with_workspace_requires_scoped_projection(...)
WebuiServeConfig->>WebUiV2State: with_workspace_requires_scoped_projection(...)
SessionHandler->>WebUiV2State: workspace_requires_scoped_projection()
SessionHandler-->>SessionHandler: features.workspace_requires_scoped_projection
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the WebUI workspace and browse filesystems from a static, project-scoped mount view to a dynamic, caller-scoped mount view, ensuring that workspace files, attachments, and persistent memory are isolated by the authenticated caller's scope (tenant, user, and project). Feedback highlights an inconsistency in the fallback strings ("_none" vs "none") used for missing optional fields between scoped_memory_target and scoped_agent_project_root. Additionally, it is recommended to preserve the original behavior of the default thread_scope() test helper to avoid unintended side effects on existing tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 2 | 0 | 2 | fbf3649c5597 |
Head: fbf3649c5597e6a54c731422d33ba5fc73987dfc
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found blocking regressions in the workspace scoping change: attachment bytes and the file browser now use a different workspace root than the runtime/tool paths that need to read or write the same files.
Findings
Blocking: 2 / Notes: 0
Blocking findings
1. ❌ [HIGH] Attachment landing no longer matches the runtime read port
Location: crates/ironclaw_reborn_composition/src/factory.rs:536
webui_workspace_filesystem() now lands /workspace/attachments/... through the scoped WebUI resolver, but build_reborn_runtime still wires attachment_read_port from local_runtime.workspace_filesystem, whose fixed view resolves /workspace to /projects/workspace. The landed files therefore sit under /projects/tenants/.../workspace while the model-side reader looks in /projects/workspace, so WebUI/OpenAI-compatible image attachments are persisted but model reads return NotFound and the image parts are skipped. Wire the runtime attachment reader to the same scoped workspace, or keep landing on the same mount the runtime reads.
2. ❌ [MEDIUM] Workspace browser is disconnected from agent tool output
Location: crates/ironclaw_reborn_composition/src/local_dev_mounts.rs:162
The standalone Workspace/Files browser now resolves /workspace to the scoped WebUI tree, but local-dev capability execution still uses local_runtime.workspace_mounts built from WORKSPACE_TARGET (/projects/workspace). Files created by the agent's coding/shell tools will remain in /projects/workspace, while the browser reads /projects/tenants/.../workspace, so agent-produced files disappear from the WebUI. The browser/project readers and the tool execution mounts need to share the same workspace target.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
|
@ironloopai review --agent reviewer |
|
@ironloopai status |
|
@ironloopai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/local_dev_mounts.rs`:
- Around line 176-195: scoped_workspace_mount_view currently rewrites only
mounts whose target matches WORKSPACE_TARGET, so if ambient_workspace_mount_view
or its callers stop including that grant the function will quietly return an
unscoped MountView. Add a fail-loud guard in scoped_workspace_mount_view that
tracks whether any mount was rewritten and then either debug-assert or return a
HostApiError when no targets match; use the existing scoped_workspace_target,
WORKSPACE_TARGET, and MountView/MountGrant flow to keep the check close to the
rewrite logic.
- Around line 201-217: The scoped_memory_target helper is using a different
missing-segment sentinel than the rest of the memory path logic, so mounts
without agent/project end up under the wrong tree. Update scoped_memory_target
to match MemoryDocumentScope::virtual_prefix() and the memory path parser by
using the shared memory path builder (or the same _none sentinel) for missing
agent_id and project_id instead of hardcoding __none__.
In
`@crates/ironclaw_reborn_composition/src/support/fs/mount_filesystem_reader.rs`:
- Around line 373-409: Add a memory-isolation-by-project test alongside
scoped_browser_reads_memory_from_existing_memory_namespace to cover the
same-user/different-project case. Use the existing scoped_memory_writer,
scope_for, MountScopedFilesystemReader, and FsMount::Memory flow, but create two
scopes for the same caller with different project_id values and verify the
second scope cannot list the first scope’s seeded MEMORY.md. This should mirror
the workspace isolation coverage and confirm scoped_memory_target keeps memory
namespaced by project.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b39bcc7-44b6-426c-a3a4-bc606f60c2fc
📒 Files selected for processing (7)
crates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/local_dev_mounts.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/support/fs/attachment_landing.rscrates/ironclaw_reborn_composition/src/support/fs/mount_filesystem_reader.rscrates/ironclaw_reborn_composition/src/support/fs/project_filesystem_reader.rs
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 2 | 0 | 2 | f5f4f439e03d |
Head: f5f4f439e03d8f30b3d4dc06045fae6e3eb61b85
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found two blocking regressions in the new scoped filesystem wiring: approval lease terms still use the unscoped workspace mount view, and WebUI memory browsing uses a non-canonical sentinel for missing project/agent scope.
Findings
Blocking: 2 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Approval leases still use unscoped workspace mounts
Location: crates/ironclaw_reborn_composition/src/runtime.rs:801
The PR scopes local-dev workspace execution mounts per run in LocalDevLoopCapabilityPortFactory::create_capability_port, but this approval lease provider is still constructed with local_runtime.workspace_mounts.clone(), the composition-time unscoped /projects/workspace view. When a workspace capability opens an approval gate, the resulting lease grant carries unscoped mount constraints while the resumed invocation context uses the scoped mounts. scoped_mount_obligation requires the lease mounts to be a subset of the invocation mounts, so approved workspace operations can be rejected after approval; in any path that consumed the lease mounts directly this would also widen workspace scope. Build the lease terms from the approval gate/run scope or original request context, and add a caller-level approval test for a non-default user/project workspace operation.
2. ❌ [MEDIUM] Memory browser points missing project scope at the wrong namespace
Location: crates/ironclaw_reborn_composition/src/local_dev_mounts.rs:215
scoped_memory_target substitutes __none__ for absent agent_id or project_id, but the memory subsystem's canonical virtual paths use _none for missing axes. For the common/default case where project_id is absent, memory writes and profile-backed documents land under /memory/.../projects/_none/..., while the WebUI browse mount now points at /memory/.../projects/__none__/..., making those documents appear missing. Use the shared memory path helper or the same _none sentinel, and cover project_id: None in the WebUI memory mount tests.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
Inline review fallback
Inline comment projection fell back to a body-only PR Review because GitHub rejected the inline payload.
Reason: Unprocessable Entity: "Line could not be resolved" - https://docs.github.com/rest/pulls/reviews#create-a-review-for-a-pull-request
IronLoop preserved the inline review comment payloads below instead of dropping them.
Inline fallback 1: crates/ironclaw_reborn_composition/src/runtime.rs:801
This provider is still using the unscoped composition-time workspace mounts, while the PR now scopes execution mounts per run in create_capability_port. Approval leases for workspace operations will carry unscoped mount constraints that are not a subset of the scoped invocation mounts at resume time, so approved operations can be rejected by the scoped-mount obligation check. Please derive these lease terms from the approval gate/run scope or original request context and add a non-default user/project approval test.
Inline fallback 2: crates/ironclaw_reborn_composition/src/local_dev_mounts.rs:215
The memory path convention uses _none for absent project/agent axes, but this new browse target uses __none__. For callers without a project, memory documents created under /memory/.../projects/_none/... will be browsed under /memory/.../projects/__none__/... and appear missing. Please reuse the memory path helper or match the _none sentinel, with a test for project_id: None.
|
@ironloopai review |
|
@ironloopai review |
github-actions
Bot
added
size: XL
|
@ironloopai review |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_reborn_composition/src/local_dev_mounts.rs (1)
210-216: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftFail closed when workspace scope lacks
project_id.
scoped_workspace_targetfalls back to fixedWORKSPACE_TARGETforproject_id: None, so an incomplete WebUIResourceScopecan still land on shared/projects/workspace. Return an error or use a caller-scoped fallback; don’t silently reintroduce the ambient workspace. As per path instructions, “Fail closed on local-only or missing required handles in production and migration-dry-run profiles.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/local_dev_mounts.rs` around lines 210 - 216, The scoped_workspace_target path currently falls back to the shared WORKSPACE_TARGET when ResourceScope.project_id is missing, which can reintroduce the ambient workspace. Update scoped_workspace_target and its callers to fail closed for missing project_id, either by returning an error or by using a caller-scoped fallback that cannot resolve to the shared /projects/workspace target. Make sure the change is applied where scoped_workspace_target and scoped_agent_project_root are used so incomplete ResourceScope values cannot silently proceed.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/local_dev_mounts.rs`:
- Around line 210-216: The scoped_workspace_target path currently falls back to
the shared WORKSPACE_TARGET when ResourceScope.project_id is missing, which can
reintroduce the ambient workspace. Update scoped_workspace_target and its
callers to fail closed for missing project_id, either by returning an error or
by using a caller-scoped fallback that cannot resolve to the shared
/projects/workspace target. Make sure the change is applied where
scoped_workspace_target and scoped_agent_project_root are used so incomplete
ResourceScope values cannot silently proceed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0b239646-fffd-43a5-883e-3888122ce056
📒 Files selected for processing (2)
crates/ironclaw_reborn_composition/src/local_dev_mounts.rscrates/ironclaw_reborn_composition/src/support/fs/mount_filesystem_reader.rs
|
🚅 Deployed to the ironclaw-pr-5831 environment in ironclaw-ci-preview
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime/approval.rs`:
- Around line 136-142: Add an inline `// silent-ok: ...` comment on the fallback
branch in `ApprovalLease::workspace_mounts` where `scoped_workspace_mount_view`
returns `Err(error)` and `self.workspace_mounts.mounts.is_empty()` is accepted;
keep the existing `tracing::debug!` call and make the comment explicitly name
the workspace-mount read/fallback so the intentional unscoped `workspace_mounts`
return is obvious.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 817d5e84-e6e9-475e-9ed9-ac5a78f3407f
📒 Files selected for processing (1)
crates/ironclaw_reborn_composition/src/runtime/approval.rs
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 0375b614b042 |
Head: 0375b614b0428e213b98df9d6e4b8d552f63f6cd
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found a blocking workspace isolation gap in the new scoped mount resolver.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [HIGH] Projectless WebUI scopes still share one workspace
Location: crates/ironclaw_reborn_composition/src/local_dev_mounts.rs:211-212
The new scoped workspace resolver falls back to the legacy /projects/workspace target whenever project_id is None. That case is still supported: WebuiServeConfig::default_project_id is optional, and create-thread keeps the caller scope unchanged when no project is requested. As a result, projectless WebUI callers still land attachments, browse/download workspace files, and receive workspace tool grants against one shared backing directory, so users without a default/requested project can see each other's files. Scope the None case by tenant/user/agent with a reserved project segment, or fail closed for projectless workspace access, and add an isolation test for project_id: None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
|
@ironloopai review |
|
@ironloopai review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_composition/src/runtime/local_dev.rs`:
- Around line 220-234: The shell capability still bypasses the new workspace
scoping, so non-owner shell paths can resolve against the shared root instead of
the per-run workspace. Update the shell grant path in the local dev runtime
setup so `SHELL_CAPABILITY_ID` uses the same scoped workspace target as
`workspace_mounts` (via the `local_dev_resource_scope_for_run` /
`scoped_workspace_mount_view` flow), or otherwise make the exemption explicit
and justified; use the existing `RefreshingLocalDevCapabilityPortConfig` and
`factory.rs` shell alias wiring to locate the integration point.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: de2f91e5-4f74-4a8c-a2f4-4df25e29bce8
📒 Files selected for processing (3)
crates/ironclaw_reborn_composition/src/local_dev_mounts.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs
998fec0 to
3e53257
Compare
3e53257 to
04d9751
Compare
04d9751 to
26ee6b7
Compare
26ee6b7 to
23f3f2c
Compare
e88428a to
6ae669b
Compare
6ae669b to
254a70d
Compare
254a70d to
54f34b0
Compare
54f34b0 to
7ebb513
Compare
github-actions
Bot
added
size: L
…-isolation # Conflicts: # crates/ironclaw_webui/frontend/src/app/app.tsx # crates/ironclaw_webui/frontend/src/app/auth.ts # crates/ironclaw_webui/frontend/src/layout/gateway-layout.tsx # crates/ironclaw_webui/frontend/src/pages/workspace/components/workspace-tree.tsx # crates/ironclaw_webui/frontend/src/pages/workspace/hooks/useWorkspaceBrowser.ts # crates/ironclaw_webui/frontend/src/pages/workspace/lib/workspace-api.test.ts # crates/ironclaw_webui/frontend/src/pages/workspace/lib/workspace-api.ts # crates/ironclaw_webui/frontend/src/pages/workspace/workspace-page.tsx # crates/ironclaw_webui/src/webui_serve.rs # crates/ironclaw_webui/src/webui_v2/handlers.rs # crates/ironclaw_webui/src/webui_v2/router.rs # crates/ironclaw_webui/tests/webui_v2_handlers_contract.rs
|
Closing to reopen on top of current |
This branch was successfully deployed
1 participant
Summary
main.Change Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy -p ironclaw_webui --all-features --all-targets -- -D warningscargo clippy -p ironclaw --all-features --all-targets -- -D warningscargo build— Not applicable: the owning crate tests and all-target clippy compiled the affected targets.cargo test --features integration— Not applicable: no database-backed behavior changed.Test Strategy
User behavior: A signed hosted user sees only their scoped Workspace/Memory projection; missing scoped roots render empty, while local operator sessions retain raw workspace fallback.
Risk areas:
Tests added or updated:
What the tests prove: Tenant/user storage prefixes are hidden from presented paths; hosted and non-operator sessions cannot fall back to shared roots; local operators keep existing fallback; source-thread workspace routes remain isolated and functional after conflict resolution.
Commands run:
pnpm typecheckpnpm test(124 files, 1,056 tests)cargo test -p ironclaw_webui --all-featurescargo test -p ironclaw workspace_projection_scope_follows_deployment_profilecargo clippy -p ironclaw_webui --all-features --all-targets -- -D warningscargo clippy -p ironclaw --all-features --all-targets -- -D warningscargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_holdcargo fmt --all -- --checkSecurity Impact
Changes file-view access presentation. Hosted/non-operator sessions now fail closed when caller identity or scoped roots are unavailable. No authentication bypass, secret handling, tool execution, or network policy is weakened.
Reborn Trust-Boundary Checklist
serde(default)fields fail closed: the new session feature defaults fail closed in the browser; no durable serde field added.Database Impact
None.
Blast Radius
WebUI session bootstrap, workspace/memory API projection, frontend workspace navigation, and CLI profile-to-WebUI configuration. Source-thread file browsing and artifact-export gating were explicitly preserved during the merge.
Rollback Plan
Revert merge commit
c3bd57214and the original scoped-projection commits, restoring prior raw-mount presentation. No schema or persisted-data rollback is required.Review Follow-Through
CI must confirm changed-line coverage after the added CLI profile-matrix test. No known follow-up is required in this PR; tool/host execution scoping remains outside this UI-only change.
Review track: C (security boundary and hosted multi-user file visibility)