think: Extract the legacy Shell workspace - #2073
Conversation
Move the existing storage, Bash, and codemode state behavior behind an explicit legacy workspace while adapting Think's file access to a narrow filesystem and runtime contract.
Keep the shared assistant filesystem on the legacy storage layout and expose its filesystem, runtime, and codemode state capabilities to child agents.
Wrap Think's workspace in the operations interface expected by skill scripts.
Pass skill runners the operations adapter for Think's workspace.
🦋 Changeset detectedLatest commit: fd6c167 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Newly scaffolded projects fail their own checks because the generated agent file is only half-migrated
The generated starter agent still hands the raw workspace to the skill script runner (workspaceInstance: this.workspace at packages/think/src/cli/init.ts:433) even though the new adapter is imported at the top of the same generated file, so every freshly scaffolded project ships with an unused import and a value the runner's interface does not accept.
Impact: Users who scaffold a new project get code that fails the project's own lint/typecheck (npm run check) out of the box.
Incomplete mechanical migration of the skills runner call
The PR migrated every other call site to createWorkspaceOperations(this.workspace) (examples/agent-skills/src/server.ts:26, think-starters/coding-agent/agents/coder/agent.ts:50, think-starters/customer-support/agents/support/agent.ts:51), but the CLI template at packages/think/src/cli/init.ts:411 only added the import while line 433 still passes this.workspace.
Think.workspace is now typed ThinkWorkspace (packages/think/src/think.ts:2880), which exposes only fs/runtime and does not structurally satisfy SkillWorkspace (readFile/writeFile/readDir/glob/stat, see packages/agents/src/skills/runner.ts:18-24). The generated project's typecheck script therefore fails, and oxlint flags the unused createWorkspaceOperations import (no-unused-vars is an error per AGENTS.md), so the generated check script fails.
(Refers to line 433)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const inferred = agent ? optionsFromAgent(agent) : undefined; | ||
| const options: CreateExecuteToolOptions = inferred | ||
| ? { | ||
| ...inferred, | ||
| ...overrides, | ||
| connectors: [ | ||
| ...(inferred.connectors ?? []), | ||
| ...(overrides?.connectors ?? []) | ||
| ] | ||
| } | ||
| : { ...(source as CreateExecuteToolOptions), ...overrides }; |
There was a problem hiding this comment.
🟡 Supplying a custom sandbox filesystem to an agent's code-execution tool now breaks every code run
A caller-supplied sandbox filesystem is now added on top of the one the agent already derives from its workspace (connectors: [...inferred.connectors, ...] at packages/think/src/tools/execute.ts:194-204 combined with options.state at packages/think/src/tools/execute.ts:232-234), so two identically named sandbox namespaces are registered and the sandbox refuses to run any code.
Impact: Agents that pass their own filesystem backend to the code-execution tool see every execution fail with a duplicate-name error instead of running.
Additive merge replaces the previous override semantics
Previously optionsFromAgent(source) set state and { ...optionsFromAgent(source), ...overrides } meant an explicit overrides.state REPLACED the agent-derived one. Now the agent path derives connectors from workspace[workspaceStateProvider](ctx) (packages/think/src/tools/execute.ts:170-175) and overrides.state still pushes an extra StateConnector at packages/think/src/tools/execute.ts:232-234.
StateConnector.name() returns "state" (packages/shell/src/connector.ts:79-81), and the executor rejects duplicates: if (seenNames.has(connector.name)) return { error: 'Duplicate name "state" ...' } (packages/codemode/src/executor.ts:351-357). So createExecuteTool(this, { state: myBackend }) on a Think agent whose workspace provides a state connector fails on every execution.
A fix would be to drop the agent-derived state connectors when the caller supplies state (or their own state-named connector), preserving the old override semantics.
Prompt for agents
In packages/think/src/tools/execute.ts, createExecuteRuntime now concatenates connectors inferred from the agent's workspace state provider with overrides.connectors, while options.state still creates an additional StateConnector later in the function. Both produce a connector named "state", and packages/codemode/src/executor.ts rejects duplicate connector names, so every execution fails. Restore the previous override semantics: when the caller explicitly supplies `state` (or a connector that already occupies the `state` namespace) in the overrides, the agent-derived state connectors should not also be registered.
Was this helpful? React with 👍 or 👎 to provide feedback.
| >; | ||
|
|
||
| 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: |
1 participant
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.