Make session scratch an operable, confined agent filesystem - #1007
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 6 Skipped Deployments
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAttachment routing now supports provider-and-scratch delivery, bounded scratch twins, provider-file correlation, secure scratch lifecycle management, confined finite commands, rollback-safe staging, liveness reporting, and stricter continuation validation. ChangesAttachment, scratch, and command execution
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
softmarshmallow
marked this pull request as ready for review
July 30, 2026 05:01
coderabbitai
Bot
added
ai
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
test/desktop-agent-chat-image-drop.md (1)
43-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the additional behaviors into separate manual cases.
Large-raster provider-limit handling and generic
.txtrouting are independent from the standard image-drop flow. Move them into separately named test cases with their own IDs.Based on coding guidelines, “Ensure each manual test case file covers only one independent behavior—if a test case covers multiple independent behaviors, split it into separate files.”
🤖 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 `@test/desktop-agent-chat-image-drop.md` around lines 43 - 49, Split the large high-resolution image/provider-limit scenario and the plain .txt generic-file routing scenario from the standard image-drop manual test. Place each independent behavior in its own separately named test file with a unique test-case ID, preserving the existing steps and expected outcomes.Source: Coding guidelines
packages/grida-ai-agent/src/session/scratch.ts (1)
251-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging the swallowed
readdirerror.Failing closed is right, but an
EACCES/EIOis currently indistinguishable from "scratch was swept" — the model just seesavailable: falsewith no host-side trace.♻️ Suggested tweak
- } catch { + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + console.warn(`[agent] scratch listing failed for ${scratchDir}:`, err); + } return new Set(); }🤖 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 `@packages/grida-ai-agent/src/session/scratch.ts` around lines 251 - 262, Update listScratchFilePaths to capture the readdir error and log it through the module’s existing logging mechanism before returning the empty Set, preserving the current failing-closed behavior for all read failures.packages/grida-ai-agent/src/runtime/index.ts (1)
418-435: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: the "reverse order" intent doesn't survive
Promise.allSettled.
[...created].reverse()only orders task creation; allunlinkcalls start concurrently. Harmless for independent files — either drop thereverse()or make the loop sequential if the ordering is meant to be meaningful.🤖 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 `@packages/grida-ai-agent/src/runtime/index.ts` around lines 418 - 435, Update rollbackScratchSeeds so its cleanup behavior matches the intended ordering: either remove the unnecessary reverse ordering and keep parallel Promise.allSettled cleanup, or replace it with sequential unlink processing when reverse deletion order is required. Preserve ENOENT handling and aggregate cleanup failures.
🤖 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 `@packages/grida-ai-agent/src/runtime/message-view.test.ts`:
- Around line 249-252: Strengthen the assertions in the message-view test around
the marker text so each attachment’s name is verified together with its expected
availability value. Replace the independent contains checks near the live.bin
and expired.bin assertions with object-level or paired assertions that cannot
pass when the availability values are swapped.
In `@packages/grida-ai-agent/src/runtime/runtime.live.test.ts`:
- Around line 296-315: Update the commandInput selection in the test to search
run_command chunks for the invocation whose parsed input command is “cp”, rather
than choosing the first shell invocation. Keep the existing commandTrace,
approval assertion, and path assertions anchored to that selected cp command.
---
Nitpick comments:
In `@packages/grida-ai-agent/src/runtime/index.ts`:
- Around line 418-435: Update rollbackScratchSeeds so its cleanup behavior
matches the intended ordering: either remove the unnecessary reverse ordering
and keep parallel Promise.allSettled cleanup, or replace it with sequential
unlink processing when reverse deletion order is required. Preserve ENOENT
handling and aggregate cleanup failures.
In `@packages/grida-ai-agent/src/session/scratch.ts`:
- Around line 251-262: Update listScratchFilePaths to capture the readdir error
and log it through the module’s existing logging mechanism before returning the
empty Set, preserving the current failing-closed behavior for all read failures.
In `@test/desktop-agent-chat-image-drop.md`:
- Around line 43-49: Split the large high-resolution image/provider-limit
scenario and the plain .txt generic-file routing scenario from the standard
image-drop manual test. Place each independent behavior in its own separately
named test file with a unique test-case ID, preserving the existing steps and
expected outcomes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f7e43ad-2ad6-4117-93cb-2861950156a6
📒 Files selected for processing (42)
docs/wg/ai/agent/compositor.mddocs/wg/ai/agent/scratch.mddocs/wg/ai/agent/vision.mdeditor/kits/agent-chat/group-parts.tseditor/kits/agent-chat/message.tsxeditor/lib/agent-chat/build-agent-send.test.tseditor/lib/agent-chat/build-agent-send.tseditor/lib/agent-chat/file-attachment.test.tseditor/lib/agent-chat/file-attachment.tseditor/lib/agent-chat/image-attachment.tseditor/lib/agent-chat/input-resource-policy.test.tseditor/lib/agent-chat/input-resource-policy.tseditor/lib/agent-chat/input-resource-router.test.tseditor/lib/agent-chat/input-resource-router.tseditor/lib/agent-chat/use-turn-queue-controller.tseditor/scaffolds/desktop/shared/agent-composer-input.tsxeditor/scaffolds/desktop/shared/agent-composer-resource-id.test.tseditor/scaffolds/desktop/shared/agent-composer-resource-id.tspackages/grida-ai-agent/src/agent/image-gen-capability.test.tspackages/grida-ai-agent/src/agent/index.tspackages/grida-ai-agent/src/agent/scratch-capability.test.tspackages/grida-ai-agent/src/http/routes/agent.test.tspackages/grida-ai-agent/src/prompts.tspackages/grida-ai-agent/src/protocol/context.tspackages/grida-ai-agent/src/runtime/index.tspackages/grida-ai-agent/src/runtime/message-view.test.tspackages/grida-ai-agent/src/runtime/message-view.tspackages/grida-ai-agent/src/runtime/run-agent.tspackages/grida-ai-agent/src/runtime/run-input.test.tspackages/grida-ai-agent/src/runtime/run-input.tspackages/grida-ai-agent/src/runtime/runtime.live.test.tspackages/grida-ai-agent/src/runtime/workspace-agent-bindings.test.tspackages/grida-ai-agent/src/runtime/workspace-agent-bindings.tspackages/grida-ai-agent/src/session/scratch.test.tspackages/grida-ai-agent/src/session/scratch.tspackages/grida-ai-agent/src/tools/index.tspackages/grida-ai-agent/src/tools/run-command.tstest/desktop-agent-chat-busy-image-blocked.mdtest/desktop-agent-chat-image-drop.mdtest/desktop-agent-chat-image-paste.mdtest/desktop-agent-chat-multi-image.mdtest/desktop-agent-chat-screenshot-drop.md
softmarshmallow
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/grida-ai-agent/src/runtime/runtime.live.test.ts (1)
119-121: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the alternate auth directory as
secrets_roottoo.When
LIVE_AUTH_DIRis active,AuthStorereads credentials from that directory, butAgentRuntimestill receivessecrets_root: baseDiron Line 136. Because this host injectsrunUnsandboxedShell, a model command can read the real auth files outside the configured secret-root guard. Derive oneauthDirand use it for bothAuthStoreandsecrets_root; keepbaseDirfor the temporary database and scratch.Proposed fix
- const auth = new AuthStore( - PROVIDER_KEY ? baseDir : (LIVE_AUTH_DIR ?? baseDir) - ); + const authDir = PROVIDER_KEY ? baseDir : (LIVE_AUTH_DIR ?? baseDir); + const auth = new AuthStore(authDir); ... - secrets_root: baseDir, + secrets_root: authDir,🤖 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 `@packages/grida-ai-agent/src/runtime/runtime.live.test.ts` around lines 119 - 121, Define a shared authDir from PROVIDER_KEY, LIVE_AUTH_DIR, and baseDir, then pass it to both AuthStore and AgentRuntime’s secrets_root. Continue using baseDir for the temporary database and scratch paths, and update the existing AuthStore construction accordingly.test/desktop-agent-chat-image-drop.md (1)
29-53: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the byte-exact assertion objectively verifiable.
The procedure asks for a byte-for-byte copy but never compares the original and copied bytes. Add a concrete check, such as matching
sha256sumvalues or exact byte counts, so a transformed provider preview cannot falsely pass the manual test.🤖 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 `@test/desktop-agent-chat-image-drop.md` around lines 29 - 53, Update the “Use the attachment's scratch path” step in the documented test procedure to require an objective byte-level comparison between the original attachment and `dropped-copy`, preferably by matching their `sha256sum` values. Keep the existing filename-extension and byte-count checks, and make the expected result explicitly require identical hashes.
🧹 Nitpick comments (3)
editor/lib/agent-chat/input-resource-policy.ts (2)
391-412: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
scratch-unavailableconflates two different rejection causes.if (!scratch) return { reason: "scratch-unavailable" }; and the new binary-tools guard both return the same
"scratch-unavailable"reason:if (scratch.binaryTools === false && requiresScratchBinaryTools(resource)) { return { reason: "scratch-unavailable" }; }Downstream code that maps
UnavailableReasonto a user-facing message (e.g.resourceLoweringNotice/resourcePreparationNoticeinagent-composer-input.tsx) can't distinguish "no scratch binding at all" from "scratch exists but this file type needs binary tools that aren't available." A dedicated reason would preserve the diagnostic signal already captured byUnavailableReason.♻️ Suggested new reason
export type UnavailableReason = | "representation-unavailable" | "reference-capability-unavailable" | "provider-capability-unavailable" | "scratch-unavailable" + | "scratch-binary-tools-required" | "file-too-large" | "scratch-file-count-exceeded" | "scratch-budget-exceeded" | "draft-operable-copy-budget-exceeded" | "directory-cannot-be-attached" | "directory-reference-required"; ... if (scratch.binaryTools === false && requiresScratchBinaryTools(resource)) { - return { reason: "scratch-unavailable" }; + return { reason: "scratch-binary-tools-required" }; }🤖 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 `@editor/lib/agent-chat/input-resource-policy.ts` around lines 391 - 412, Update the binary-tools guard in scratchRoute to return a distinct UnavailableReason from the scratch-missing case, while keeping if (!scratch) mapped to "scratch-unavailable". Add the new reason to the UnavailableReason definition and update downstream notice mappings such as resourceLoweringNotice and resourcePreparationNotice to provide the appropriate message for this specific rejection.
459-486: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep
.tsoff the text-only extension allowlist unless the MIME is TypeScript.
video/mp2tMPEG-2 Transport Stream segments use the.tsextension, so a bare.tswithoutapplication/typescriptMIME should not fall through as text-safe. Remove.tsfrom the fallback extension allowlist, or gate it byapplication/typescript.🤖 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 `@editor/lib/agent-chat/input-resource-policy.ts` around lines 459 - 486, Update requiresScratchBinaryTools so the fallback filename-extension allowlist no longer treats .ts as text-safe; only classify .ts as safe when the parsed MIME is application/typescript. Preserve the existing MIME checks and all other extension handling.packages/grida-daemon/src/shell/runner.ts (1)
349-371: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
finishnow signals the process group on every normal exit and blocks on a poll wait.Correct for revoking background descendants, but note the added latency is folded into
duration_ms(reported to the model) andprocessGroupExiststreats a non-ESRCHerror (e.g.EPERM) as "still alive", so such a case polls until the full grace deadline instead of returning promptly.♻️ Optional: capture duration before the teardown wait
const finish = async ( exitCode: number | null, exitSignal: NodeJS.Signals | null, spawnError?: Error ) => { if (settled) return; settled = true; cleanupListeners(); + const durationMs = Date.now() - startedAt;- duration_ms: Date.now() - startedAt, + duration_ms: durationMs,🤖 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 `@packages/grida-daemon/src/shell/runner.ts` around lines 349 - 371, Update finish so duration_ms is captured before the process-group teardown wait, excluding polling latency from the reported command duration. Adjust processGroupExists to avoid treating non-ESRCH errors such as EPERM as evidence that the group remains alive; return promptly for those errors while preserving normal existence detection.
🤖 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 `@packages/grida-ai-agent/src/runtime/sandbox-repro.live.test.ts`:
- Line 243: Update the comment adjacent to the shell_executor configuration to
remove references to the obsolete shell_execution_allowed boolean and describe
that the desktop supplies an OS-confined shell executor.
In `@packages/grida-ai-agent/src/session/scratch.ts`:
- Around line 602-609: Update sweepScratch to wrap each removeAuthorityEntrySync
call in its own try/catch, logging a warning for failures while continuing to
process the remaining session entries. Preserve the existing fail-closed
authority validation performed before the deletion loop.
In `@test/desktop-agent-chat-large-raster-drop.md`:
- Around line 27-32: Update the large-raster test scenario to use a
deterministic oversized image fixture with recorded pixel dimensions and byte
size, rather than the variable “over 5 MB or 2000 pixels” criterion. Ensure the
fixture is known to exercise the bounded provider representation while
preserving the expected successful thumbnail and image-description outcomes.
---
Outside diff comments:
In `@packages/grida-ai-agent/src/runtime/runtime.live.test.ts`:
- Around line 119-121: Define a shared authDir from PROVIDER_KEY, LIVE_AUTH_DIR,
and baseDir, then pass it to both AuthStore and AgentRuntime’s secrets_root.
Continue using baseDir for the temporary database and scratch paths, and update
the existing AuthStore construction accordingly.
In `@test/desktop-agent-chat-image-drop.md`:
- Around line 29-53: Update the “Use the attachment's scratch path” step in the
documented test procedure to require an objective byte-level comparison between
the original attachment and `dropped-copy`, preferably by matching their
`sha256sum` values. Keep the existing filename-extension and byte-count checks,
and make the expected result explicitly require identical hashes.
---
Nitpick comments:
In `@editor/lib/agent-chat/input-resource-policy.ts`:
- Around line 391-412: Update the binary-tools guard in scratchRoute to return a
distinct UnavailableReason from the scratch-missing case, while keeping if
(!scratch) mapped to "scratch-unavailable". Add the new reason to the
UnavailableReason definition and update downstream notice mappings such as
resourceLoweringNotice and resourcePreparationNotice to provide the appropriate
message for this specific rejection.
- Around line 459-486: Update requiresScratchBinaryTools so the fallback
filename-extension allowlist no longer treats .ts as text-safe; only classify
.ts as safe when the parsed MIME is application/typescript. Preserve the
existing MIME checks and all other extension handling.
In `@packages/grida-daemon/src/shell/runner.ts`:
- Around line 349-371: Update finish so duration_ms is captured before the
process-group teardown wait, excluding polling latency from the reported command
duration. Adjust processGroupExists to avoid treating non-ESRCH errors such as
EPERM as evidence that the group remains alive; return promptly for those errors
while preserving normal existence detection.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97b79955-f263-4bb3-9a86-f989dfc0eb9e
📒 Files selected for processing (61)
SECURITY.mddesktop/README.mddesktop/docs/agent-authority.mddesktop/src/agent-sidecar-channel.test.tsdesktop/src/agent-sidecar-channel.tsdesktop/src/agent-sidecar-network.test.tsdesktop/src/agent-sidecar-network.tsdesktop/src/agent-sidecar.tsdesktop/src/main/agent-command-host.srt.test.tsdesktop/src/main/agent-command-host.test.tsdesktop/src/main/agent-command-host.tsdesktop/src/main/agent-network-host.test.tsdesktop/src/main/agent-network-host.tsdesktop/src/main/agent-sidecar-supervisor.tsdesktop/src/main/sandbox/manager.tsdesktop/src/preload-contract.test.tsdesktop/src/preload.tsdocs/wg/ai/agent/scratch.mddocs/wg/desktop/agent-security.mdeditor/lib/agent-chat/input-resource-policy.test.tseditor/lib/agent-chat/input-resource-policy.tseditor/lib/agent-chat/input-resource-router.tseditor/lib/desktop/bridge.test.tseditor/lib/desktop/bridge.tseditor/scaffolds/desktop/shared/agent-composer-input.tsxpackages/grida-ai-agent/README.mdpackages/grida-ai-agent/src/__public-api__.test.tspackages/grida-ai-agent/src/e2e-approval-resume.test.tspackages/grida-ai-agent/src/http/routes/agent.test.tspackages/grida-ai-agent/src/http/routes/sessions-lifecycle.test.tspackages/grida-ai-agent/src/http/routes/sessions.tspackages/grida-ai-agent/src/prompts.tspackages/grida-ai-agent/src/runtime/command-backend.test.tspackages/grida-ai-agent/src/runtime/command-backend.tspackages/grida-ai-agent/src/runtime/image-gen.live.test.tspackages/grida-ai-agent/src/runtime/index.tspackages/grida-ai-agent/src/runtime/message-view.test.tspackages/grida-ai-agent/src/runtime/run-agent.tspackages/grida-ai-agent/src/runtime/run-input.test.tspackages/grida-ai-agent/src/runtime/run-input.tspackages/grida-ai-agent/src/runtime/runtime.live.test.tspackages/grida-ai-agent/src/runtime/runtime.test.tspackages/grida-ai-agent/src/runtime/sandbox-repro.live.test.tspackages/grida-ai-agent/src/runtime/scratch.live.test.tspackages/grida-ai-agent/src/runtime/workspace-agent-bindings.test.tspackages/grida-ai-agent/src/runtime/workspace-agent-bindings.tspackages/grida-ai-agent/src/server.test.tspackages/grida-ai-agent/src/server.tspackages/grida-ai-agent/src/session/scratch.test.tspackages/grida-ai-agent/src/session/scratch.tspackages/grida-ai-agent/src/session/store.tspackages/grida-ai-agent/src/tools/run-command.test.tspackages/grida-ai-agent/src/tools/run-command.tspackages/grida-daemon/src/__public-api__.test.tspackages/grida-daemon/src/server.tspackages/grida-daemon/src/shell/runner.test.tspackages/grida-daemon/src/shell/runner.tspackages/grida-desktop-bridge/src/index.tstest/desktop-agent-chat-generic-file-drop.mdtest/desktop-agent-chat-image-drop.mdtest/desktop-agent-chat-large-raster-drop.md
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/wg/ai/agent/scratch.md
- packages/grida-ai-agent/src/runtime/run-agent.ts
- packages/grida-ai-agent/src/prompts.ts
- packages/grida-ai-agent/src/runtime/run-input.test.ts
- editor/lib/agent-chat/input-resource-router.ts
coderabbitai
Bot
removed
ai
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 `@editor/lib/agent-chat/input-resource-policy.ts`:
- Around line 484-486: Update the structured-text filename fallback regex in the
resource policy to include the .ts extension alongside the existing .tsx entry,
so TypeScript files with empty or unknown MIME types remain allowed without
binary tools.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 55e4bea1-d76e-412b-9782-df1c1ca5e3f8
📒 Files selected for processing (12)
editor/lib/agent-chat/input-resource-policy.test.tseditor/lib/agent-chat/input-resource-policy.tseditor/lib/agent-chat/input-resource-router.test.tseditor/scaffolds/desktop/shared/agent-composer-input.tsxpackages/grida-ai-agent/src/runtime/runtime.live.test.tspackages/grida-ai-agent/src/runtime/sandbox-repro.live.test.tspackages/grida-ai-agent/src/session/scratch.test.tspackages/grida-ai-agent/src/session/scratch.tspackages/grida-daemon/src/shell/runner.test.tspackages/grida-daemon/src/shell/runner.tstest/desktop-agent-chat-image-drop.mdtest/desktop-agent-chat-large-raster-drop.md
🚧 Files skipped from review as they are similar to previous changes (2)
- test/desktop-agent-chat-large-raster-drop.md
- packages/grida-ai-agent/src/runtime/sandbox-repro.live.test.ts
coderabbitai
Bot
added
ai
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #916
What changed
AgentFsroot for workspace-bound Desktop sessionsview_image, structured filesystem tools, and optional command workavailablestate from the ephemeral filesystemcp/mvwithout approval while keeping workspace promotion under the normal supervision policyCodex grounding
This intentionally generalizes several filesystem-backed Codex patterns; it is not a claim that Codex has one universal attachment filesystem.
Research against the local
openai/codexclone atfea2d8b18f5ffound:Image/LocalImage(and audio), but no genericFilevariant;LocalImagereads its original path and becomes provider-nativeinput_imagecontent (source, lowering)$CODEX_HOME/attachments/<uuid>(source)setsid(2)descendants are not a hard-revocation guaranteeGrida's result is therefore a coherent per-session scratch contract: incoming operable copies, generated output, structured filesystem access, cleanup, and command confinement share one host-owned authority.
The proprietary Codex Desktop frontend is not in that repository, so this comparison establishes the open-source core/TUI/app-server behavior, not undocumented frontend upload preprocessing.
Capability and compatibility behavior
view_image.This PR shows image thumbnails, attachment filename chips, and tool media. It does not add a user-facing scratch browser or “Reveal in Finder” action.
Security and lifecycle
run_commandis exposed only through a host-injectedShellExecutor; a boolean sandbox claim cannot enable raw spawnuserData, and re-allows only the exact workspace, own-session scratch, and private command tempcommand.abortedacknowledgement and the model-pump settlement barrier prevent a replacement turn or session deletion from overlapping cleanupsetsid(2)escape remains documented rather than overstatedValidation
pnpm fmt:checkpnpm lint— 0 errors; 3 existing Vitest todo warnings@grida/daemon,@grida/agent,@grida/desktop-bridgeuserDatadenied)Manual verification
Verified in the running Desktop app:
The corresponding manual cases are:
test/desktop-agent-chat-image-drop.mdtest/desktop-agent-chat-large-raster-drop.mdtest/desktop-agent-chat-generic-file-drop.md