Refactor workspace AI tool surface and add codemode runtime#22
Conversation
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe PR integrates ChangesAI Tooling Overhaul: Codemode Integration, Normalized Contracts, Catalog Runtime
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over AIThread,CodemodeRuntime: Per-turn tool configuration
participant AIThread as AIThread.beforeTurn
participant TurnConfig as createAIThreadTurnToolConfig
participant Catalog as AIThreadToolCatalog
participant CodemodeRuntime as CodemodeRuntime (LOADER)
end
AIThread->>TurnConfig: { env, ctx, workspace, getThreadContext, canMutate, timeZone }
TurnConfig->>Catalog: getActiveToolNames(canMutate)
Catalog-->>TurnConfig: string[] of active tool names
TurnConfig->>Catalog: getCodemodeTools(canMutate)
Catalog-->>TurnConfig: filtered ToolSet for codemode sandbox
TurnConfig->>CodemodeRuntime: install codemode_execute(sandbox backend + tool subset)
TurnConfig-->>AIThread: { activeTools, tools } spread into TurnConfig
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/workspaces/ai/workspace-kernel-ai-read.ts (1)
206-225: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winEmpty files incorrectly treated as failed.
Line 206 uses a falsy check
projection?.content &&which will fail for empty files that havecontent: ""(empty string). An empty but successfully extracted file withstatus: "ready"andcontent: ""will bypass lines 212-218, fall through to line 221, andgetFileReadStatuswill return"failed"at line 246, incorrectly marking the empty file as failed.🐛 Proposed fix to handle empty files correctly
- if (projection?.content && isReadableProjectionStatus(projection.status)) { + if (projection && projection.content !== null && isReadableProjectionStatus(projection.status)) { const page = pageWorkspaceAiMarkdown(projection.content, { limit: input.contentLimit, offset: input.contentOffset, });🤖 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 `@src/features/workspaces/ai/workspace-kernel-ai-read.ts` around lines 206 - 225, The condition on line 206 using `projection?.content &&` performs a falsy check that incorrectly treats empty strings as missing content, causing empty files to fall through to the getFileReadStatus call which incorrectly marks them as failed. Change the condition to explicitly check if projection.content is not null and not undefined (rather than relying on truthiness), so that empty files with a valid "ready" status are properly handled in the first return block instead of being misclassified as failed in the second return statement.
🧹 Nitpick comments (1)
src/features/workspaces/ai/web-tools.ts (1)
31-58: 🗄️ Data Integrity & Integration | 🔵 TrivialAdd output schemas to web_markdown and web_links tools for consistency and discoverability.
The
web_markdownandweb_linkstools return structured objects{ content, truncated }and{ items, truncated }respectively, but lackoutputSchemadefinitions. Other tools in the codebase (seetime_get_currentandtime_calculate_relativeintime-tools.ts) include output schemas to expose their contract to tool metadata and inspector systems. Without these schemas, the new structured web result contract is not discoverable.Suggested schema wiring
+const webMarkdownOutputSchema = z.object({ + content: z.string(), + truncated: z.boolean(), +}); + +const webLinksOutputSchema = z.object({ + items: z.array(z.string()), + truncated: z.boolean(), +}); + export function createAIThreadWebTools(env: Env): ToolSet { const browser = env.BROWSER as unknown as QuickActionBinding; return { web_markdown: tool({ description: "Load a public webpage and return its rendered content as Markdown.", inputSchema: browserPageInputSchema, inputExamples: browserPageInputExamples, + outputSchema: webMarkdownOutputSchema, strict: true, execute: async ({ url }) => { const safeUrl = assertPublicHttpUrl(url); return truncateMarkdown( @@ web_links: tool({ description: "Load a public webpage and return its rendered links.", inputSchema: browserPageInputSchema, inputExamples: browserPageInputExamples, + outputSchema: webLinksOutputSchema, strict: true, execute: async ({ url }) => {🤖 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 `@src/features/workspaces/ai/web-tools.ts` around lines 31 - 58, The web_markdown and web_links tools are missing outputSchema definitions that describe their return values, making their output contract not discoverable by tool metadata and inspector systems. Add an outputSchema property to both the web_markdown tool and web_links tool definitions that describes their respective return structures: web_markdown returns { content, truncated } and web_links returns { items, truncated }. Follow the same pattern used in other tools like time_get_current and time_calculate_relative from time-tools.ts to ensure consistency across the codebase.
🤖 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 `@src/features/workspaces/ai/ai-thread-runtime.ts`:
- Around line 84-106: Remove the unsafe type assertion `as WorkspaceFsLike` on
the input.workspace parameter being passed to createWorkspaceStateBackend in the
createAIThreadTurnToolConfig function. Instead, update the input parameter type
in the function signature to accept WorkspaceFsLike instead of WorkspaceLike,
ensuring the workspace object is known to implement all 16 required methods that
createWorkspaceStateBackend expects, rather than just the 8 methods provided by
WorkspaceLike.
In `@src/features/workspaces/ai/time-tools.ts`:
- Around line 14-15: The createRelativeOffsetFieldSchema function creates a Zod
schema with only a minimum bound (.min(0)) but no maximum bound, allowing
arbitrarily large offset values that can produce invalid Date objects when used
at line 81. Add a .max() constraint to the z.number().int() chain to limit the
offset values to a safe range that will not produce invalid dates, ensuring
dates remain within valid bounds when calculations are performed.
In `@src/features/workspaces/ai/web-tools.ts`:
- Around line 77-85: The truncateLinks function inefficiently serializes the
entire items array to JSON before checking if it exceeds
MAX_BROWSER_RESULT_CHARS, creating a large intermediate string in memory even
when truncation is needed. Instead of calling JSON.stringify on the full array
upfront, incrementally build the JSON representation by adding items one at a
time and stop accumulating once the length would exceed the character limit.
This avoids allocating unnecessary memory for pages with many links.
---
Outside diff comments:
In `@src/features/workspaces/ai/workspace-kernel-ai-read.ts`:
- Around line 206-225: The condition on line 206 using `projection?.content &&`
performs a falsy check that incorrectly treats empty strings as missing content,
causing empty files to fall through to the getFileReadStatus call which
incorrectly marks them as failed. Change the condition to explicitly check if
projection.content is not null and not undefined (rather than relying on
truthiness), so that empty files with a valid "ready" status are properly
handled in the first return block instead of being misclassified as failed in
the second return statement.
---
Nitpick comments:
In `@src/features/workspaces/ai/web-tools.ts`:
- Around line 31-58: The web_markdown and web_links tools are missing
outputSchema definitions that describe their return values, making their output
contract not discoverable by tool metadata and inspector systems. Add an
outputSchema property to both the web_markdown tool and web_links tool
definitions that describes their respective return structures: web_markdown
returns { content, truncated } and web_links returns { items, truncated }.
Follow the same pattern used in other tools like time_get_current and
time_calculate_relative from time-tools.ts to ensure consistency across the
codebase.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f99b4808-2a43-4e54-8f6e-fe22ecab0fa6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
package.jsonsrc/features/workspaces/ai/ai-inspector-serialization.tssrc/features/workspaces/ai/ai-inspector-view-parsing.tssrc/features/workspaces/ai/ai-inspector-view-types.tssrc/features/workspaces/ai/ai-thread-runtime.tssrc/features/workspaces/ai/ai-thread.tssrc/features/workspaces/ai/time-tools.tssrc/features/workspaces/ai/web-tools.tssrc/features/workspaces/ai/workspace-kernel-ai-common.tssrc/features/workspaces/ai/workspace-kernel-ai-create.tssrc/features/workspaces/ai/workspace-kernel-ai-delete.tssrc/features/workspaces/ai/workspace-kernel-ai-edit.tssrc/features/workspaces/ai/workspace-kernel-ai-move.tssrc/features/workspaces/ai/workspace-kernel-ai-read.tssrc/features/workspaces/ai/workspace-kernel-ai-rename.tssrc/features/workspaces/ai/workspace-tools.tssrc/features/workspaces/components/ai-chat/AiChatInspectorViews.tsxsrc/features/workspaces/kernel/workspace-kernel-list.tssrc/server.tsworker-configuration.d.tswrangler.jsonc
💤 Files with no reviewable changes (1)
- src/features/workspaces/ai/workspace-kernel-ai-common.ts
Greptile SummaryThis PR normalizes workspace AI tool contracts to a consistent
Confidence Score: 3/5The PR is largely safe but has one defect in the read path that turns a normal user input (a non-root folder path) into an unhandled server error. The read tool now throws a generic, uncaught Error when the caller passes any non-root folder path (e.g., /My Documents). The root-path guard covers only /; all other folder items resolve to status: item and reach readWorkspaceKernelAiItem, which throws before the catch block can handle it. This is a latent crash introduced by the simplification of the read path. src/features/workspaces/ai/workspace-kernel-ai-read.ts — the non-root folder guard is missing between the path-resolution switch and the readWorkspaceKernelAiItem call. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (projection.status === "ready" || projection.status === "needs_review") { | ||
| return "failed"; | ||
| } |
There was a problem hiding this comment.
This branch is reached when projection.status is "ready" or "needs_review" but projection.content is falsy (the projection completed but yielded no text). Returning "failed" in that case may mislead the AI model — extraction didn't fail, the file just had no extractable text (e.g., an image-only PDF whose extraction produced an empty string). A status of "unsupported" or a dedicated "empty" value would better convey the actual situation without implying an error in the pipeline.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 22 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/kernel/workspace-kernel-rows.ts">
<violation number="1" location="src/features/workspaces/kernel/workspace-kernel-rows.ts:73">
P2: Deleted-event payload backward-compat handling was removed, so legacy kernel_events payloads can silently skip deletions. Keep a deleted-event normalization/fallback for `itemId` and missing arrays.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return Array.isArray(value) | ||
| ? value.filter((item): item is string => typeof item === "string") | ||
| : []; | ||
| return JSON.parse(row.payload_json) as WorkspaceRealtimeEvent["payload"]; |
There was a problem hiding this comment.
P2: Deleted-event payload backward-compat handling was removed, so legacy kernel_events payloads can silently skip deletions. Keep a deleted-event normalization/fallback for itemId and missing arrays.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/kernel/workspace-kernel-rows.ts, line 73:
<comment>Deleted-event payload backward-compat handling was removed, so legacy kernel_events payloads can silently skip deletions. Keep a deleted-event normalization/fallback for `itemId` and missing arrays.</comment>
<file context>
@@ -70,37 +70,5 @@ export function mapKernelEventRow(
- return Array.isArray(value)
- ? value.filter((item): item is string => typeof item === "string")
- : [];
+ return JSON.parse(row.payload_json) as WorkspaceRealtimeEvent["payload"];
}
</file context>
| return JSON.parse(row.payload_json) as WorkspaceRealtimeEvent["payload"]; | |
| const payload = JSON.parse(row.payload_json) as unknown; | |
| if (row.type === "workspace.item.deleted") { | |
| if (!payload || typeof payload !== "object" || Array.isArray(payload)) { | |
| return { itemIds: [], deletedItemIds: [] }; | |
| } | |
| const record = payload as Record<string, unknown>; | |
| const itemIds = Array.isArray(record.itemIds) | |
| ? record.itemIds.filter((item): item is string => typeof item === "string") | |
| : []; | |
| const legacyItemId = typeof record.itemId === "string" ? record.itemId : null; | |
| return { | |
| itemIds: itemIds.length > 0 ? itemIds : legacyItemId ? [legacyItemId] : [], | |
| deletedItemIds: Array.isArray(record.deletedItemIds) | |
| ? record.deletedItemIds.filter((item): item is string => typeof item === "string") | |
| : [], | |
| }; | |
| } | |
| return payload as WorkspaceRealtimeEvent["payload"]; |
1 participant
Summary
items/failedstyle outputs and surface output schemas in the inspectorTesting
Summary by cubic
Refactored the workspace AI tool surface with strict input/output schemas and consistent { items, failed } results, and added a
codemoderuntime to run sandboxed JavaScript withtoolsand optionalstateFS. Time tools now support IANA time zones and return formatted local time; the chat UI gets a floating toolbar and transcript rail.New Features
codemode_executevia@cloudflare/codemode; boundLOADERand exportedCodemodeRuntime.stateis provided only when the workspace implements an FS; active tools are selected per turn.time_get_currentandtime_calculate_relativeaccept optionaltime_zone, return UTC timestamps pluslocalDateTime, and use strict input/output schemas. Inspector now shows tool output schemas.Refactors
path_is_folder. Files reportstatus: ready | pending | failed | unsupportedwith slimmer paging; removed legacy read shims.{ items, failed }. Consolidated registration; replaced multiple sandbox file ops withsandbox_bashwhen available;codemode_executealways available with narrowed state wiring.{ content, truncated }, links{ items, truncated }. Kernel list now returns{ path, more, items, failed }with items{ path, type }.Written for commit 80da38d. Summary will update on new commits.
Summary by CodeRabbit
time_get_current,time_calculate_relative) with optional IANA time zone support.items+failedresults (with less nested metadata).