think: Extract the legacy Shell workspace - #2073
Conversation
🦋 Changeset detectedLatest commit: 2e8e6e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| >; | ||
|
|
||
| type FileInfoSource = Awaited<ReturnType<ThinkWorkspace["fs"]["stat"]>>; | ||
|
|
||
| async function fileInfo( | ||
| workspace: ThinkWorkspace, | ||
| path: string, | ||
| hint?: FileInfoHint |
There was a problem hiding this comment.
🟡 File search and text search now issue one extra metadata lookup per matched file
Every file returned by a search is re-queried individually for its metadata (workspace.fs.stat(path) at packages/think/src/tools/workspace.ts:260) after the search already returned the whole list, so a search across a large workspace fans out into hundreds or thousands of extra lookups.
Impact: The find and grep tools get much slower on large workspaces, and in the shared-workspace setup each extra lookup is a separate cross-object call.
Metadata dropped by the find adapter and then re-fetched per entry
LegacyShellFilesystem.find maps glob results down to { path, type } (packages/think/src/workspace-shell-legacy.ts:140-149), discarding size/mimeType/timestamps that the underlying single SQL glob already returned (packages/shell/src/filesystem.ts:1070). workspaceFindOps then calls fileInfo(...) for every entry (packages/think/src/tools/workspace.ts:178-194), and fileInfo performs workspace.fs.stat(path) — one additional query per result. createGrepTool and createFindTool both go through this path, and createWorkspaceOperations().glob inherits it.
The same shape appears in _hostListFiles (packages/think/src/think.ts:7090-7108), which now stats each directory entry; it will also reject the whole listing if any single stat throws.
A fix is to let the filesystem find/readdir surface carry the metadata it already has (or to have the legacy adapter return richer entries) so the tools don't need a per-entry stat.
Was this helpful? React with 👍 or 👎 to provide feedback.
agents
@cloudflare/ai-chat
@cloudflare/codemode
create-think
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
Move existing storage and snapshot Bash behind workspace-legacy, adapt it to Think's Computer-shaped contract, and keep the existing state.* codemode interface across workspace implementations.
Keep workspace types, state compatibility, and provider entrypoints together. Remove the unused generated adapter import and document the deprecated state compatibility bridge.
Extend the structural runtime handle with optional streamed events, cancellation, and callable backend discovery while keeping aggregate result compatibility.
Keep the provider implementation grouped under src/workspace while building the public entrypoint at dist/workspace-legacy.
fd6c167 to
2e8e6e9
Compare
| const selected = entries.slice( | ||
| offset, | ||
| opts?.limit === undefined ? undefined : offset + opts.limit | ||
| ); | ||
| return Promise.all( | ||
| selected.map((entry) => | ||
| fileInfo( | ||
| workspace, | ||
| joinPath(normalizeWorkspacePath(dir), entry.name), | ||
| { | ||
| name: entry.name, | ||
| isFile: entry.isFile, | ||
| isDirectory: entry.isDirectory, | ||
| isSymbolicLink: entry.isSymbolicLink | ||
| } | ||
| ) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🟡 Listing a folder fails completely when it contains a dangling shortcut or a file that just disappeared
Every entry returned by a directory listing is re-checked one by one (fileInfo(...) at packages/think/src/tools/workspace.ts:165-173), and if any single entry can no longer be resolved the whole listing throws instead of skipping it.
Impact: A folder containing one broken shortcut (or a file deleted moments earlier) makes the assistant's list/find/grep of that folder fail outright, so it can no longer see any of the other files.
Per-entry `stat()` turns a per-file miss into a whole-listing rejection
The legacy readDir used to return a fully-populated FileInfo[] in one query. The new adapter calls readdir() and then fileInfo() for every entry, which calls workspaceFilesystem(workspace).stat(path).
LegacyWorkspaceFilesystem.stat (packages/think/src/workspace/types.ts:217-221) throws an ENOENT error whenever the underlying Workspace.stat returns null. Workspace.stat resolves symlinks (packages/shell/src/filesystem.ts:499-521) and returns null for a dangling symlink target, so a directory containing a broken symlink makes Promise.all reject and the whole list tool call errors — even though the readdir hint already knows the entry is a symlink.
The same pattern applies to workspaceFindOps.glob (packages/think/src/tools/workspace.ts:188-198), WorkspaceStateFilesystem.glob/readDir (packages/think/src/workspace/state.ts:114-118, 173-179) and _hostListFiles (packages/think/src/think.ts:7095-7113).
Secondary effect: each listing now issues one extra SQL stat (plus symlink resolution) per entry, where the previous single readDir/glob query already returned type, size and timestamps.
Prompt for agents
In packages/think/src/tools/workspace.ts, workspaceListOps.readDir and workspaceFindOps.glob now call fileInfo() for every directory entry, and fileInfo() always performs a stat(). For the legacy workspace, stat() follows symlinks and throws ENOENT when the target is missing, so a single dangling symlink (or an entry removed between readdir and stat) rejects the whole Promise.all and fails the entire list/find/grep tool call. The same shape exists in packages/think/src/workspace/state.ts (glob/readDir) and think.ts _hostListFiles. Consider making the per-entry metadata lookup fault-tolerant (fall back to the readdir/find hint with size 0 or use lstat when the hint says the entry is a symlink) so one unresolvable entry does not destroy the whole listing. Also consider avoiding the extra per-entry stat entirely when the underlying listing can supply size/mtime.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return { | ||
| path: normalized, | ||
| name: name ?? basename(normalized), | ||
| type, | ||
| mimeType: "application/octet-stream", | ||
| size: stat.size, | ||
| createdAt: stat.mtime, | ||
| updatedAt: stat.mtime, | ||
| ...(type === "symlink" | ||
| ? { target: await this.fs.readlink(normalized) } | ||
| : {}) | ||
| }; |
There was a problem hiding this comment.
🟡 Sandbox file listings now report every file as an unknown binary type and lose the creation date
File details handed to sandboxed code are rebuilt with a hard-coded generic content type and with the creation date replaced by the last-modified date (fileInfoOrThrow at packages/think/src/workspace/state.ts:207-218), so metadata that used to be accurate is now wrong.
Impact: Code and models that inspect file metadata in the sandbox can no longer tell a PNG or JSON file from an opaque blob, and creation timestamps are silently wrong.
Double adaptation drops `mimeType` and `createdAt`
Previously createExecuteRuntime built the codemode state.* backend directly over the workspace (createWorkspaceStateBackend(fs)), so state.stat / state.readDir / state.glob returned the workspace's stored mimeType and real created_at.
Now the chain is StateBackend -> WorkspaceStateFilesystem -> LegacyWorkspaceFilesystem -> Workspace. The intermediate ThinkWorkspaceStat (packages/think/src/workspace/types.ts:7-16) has no mimeType and only a single mtime, which legacyStat fills from updatedAt (packages/think/src/workspace/types.ts:338-354). fileInfoOrThrow then emits mimeType: "application/octet-stream" unconditionally and createdAt: stat.mtime.
The same createdAt = mtime collapse happens in packages/think/src/tools/workspace.ts:283-284 for the list/find tools (there mimeType is at least re-derived from the file extension).
Prompt for agents
packages/think/src/workspace/state.ts rebuilds FileInfo for the codemode state.* backend from the reduced ThinkWorkspaceStat shape, which has no mimeType and only one timestamp. As a result state.stat/readDir/glob now always report mimeType "application/octet-stream" and createdAt equal to the modification time, where the previous direct createWorkspaceStateBackend(workspace) path returned the workspace's stored mime type and real creation time. Consider either extending ThinkWorkspaceStat with optional mimeType/createdAt (populated by LegacyWorkspaceFilesystem from the underlying FileInfo) or deriving the mime type from the file extension as tools/workspace.ts does, so sandboxed programs keep usable metadata.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const entries = await workspaceFilesystem(workspace).find( | ||
| directory, | ||
| relativePattern | ||
| ); | ||
| return Promise.all( | ||
| entries.map((entry) => | ||
| fileInfo(workspace, entry.path, { | ||
| name: basename(entry.path), | ||
| isFile: entry.type === "file", | ||
| isDirectory: entry.type === "dir", | ||
| isSymbolicLink: false |
There was a problem hiding this comment.
🟡 Shortcuts are reported as ordinary files by the file-search tool
Search results are labelled as plain files regardless of what they actually are (isSymbolicLink: false at packages/think/src/tools/workspace.ts:194), so shortcuts that were previously identified as such are now indistinguishable from real files.
Impact: The assistant's find results mislabel symlinks as regular files, which can make it act on a link as though it were the underlying content.
Hint overrides the real stat flags
LegacyWorkspaceFilesystem.find collapses the workspace's three entry types into "dir" | "file" (packages/think/src/workspace/types.ts:251-260), mapping symlink to "file". workspaceFindOps.glob then passes isSymbolicLink: false as a hint into fileInfo, and fileInfo spreads the hint over the real stat result (packages/think/src/tools/workspace.ts:271-277), so type can never be "symlink".
Before this change workspaceFindOps.glob returned ws.glob(pattern) directly, whose FileInfo.type preserved "symlink".
Prompt for agents
packages/think/src/tools/workspace.ts workspaceFindOps.glob passes isSymbolicLink: false as a hint for every entry, and fileInfo() spreads the hint over the real stat result, so symlinks found via glob are always reported with type "file". The root cause is that ThinkWorkspaceFoundEntry only models "file" | "dir" (packages/think/src/workspace/types.ts). Either omit isSymbolicLink from the hint so the stat/lstat result decides, or extend the found-entry type to carry symlink information.
Was this helpful? React with 👍 or 👎 to provide feedback.
Currently Think uses a @cloudflare/shell + just-bash powered interface. We'd like to swap this out for a @cloudflare/computer powered interface.
This touches a lot of moving pieces:
There is also a bunch of stuff (tool arguments, media support & network config) that is on the @cloudflare/computer backlog that needs to be implemented so we have feature parity here.
See PR stack starting at cloudflare/computer#79 and cloudflare/computer#88
To begin this transition, this PR introduces a Think-owned workspace contract with separate filesystem and runtime surfaces. It moves the existing @cloudflare/shell storage, R2 support, snapshot-based Bash, and rich
state.*connector into@cloudflare/think/workspace-shell-legacy. This legacy implementation remains Think's default in this pull request, so existing data and execution behavior do not change.In a follow up PR we'll swap this out with a @cloudflare/computer backed workspace at which point it will be possible to swap out the workspace and have the legacy one tree-shaken out of the final build. Ultimately we should be able to then remove the @shell/just-bash backend.
The Assistant example selects the legacy workspace explicitly for its shared proxy filesystem. Agent Skills and the Think starters use
createWorkspaceOperations()to run scripts against the new structural interface. These updates prepare consumers for alternative workspace implementations without introducing@cloudflare/computer.A following pull request will add
@cloudflare/computerand make its backend-free workspace the default. Existing legacy data will continue to require the explicit legacy workspace and will not be migrated automatically.