diff --git a/.changeset/remove-think-compatibility.md b/.changeset/remove-think-compatibility.md new file mode 100644 index 0000000..a79c0dd --- /dev/null +++ b/.changeset/remove-think-compatibility.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": minor +--- + +Remove the deprecated `useThink` compatibility layer. Think integrations must use `workspace.fs` and `@cloudflare/computer/tools` directly. diff --git a/packages/computer/src/client.test.ts b/packages/computer/src/client.test.ts index 035b0e9..e7b7215 100644 --- a/packages/computer/src/client.test.ts +++ b/packages/computer/src/client.test.ts @@ -10,9 +10,9 @@ import { SQLiteTestStorage } from "@cloudflare/dofs/testing"; import { describe, expect, it } from "vitest"; -import { getWorkspace, type WorkspaceClient } from "./client.js"; +import { getWorkspace } from "./client.js"; import { WORKSPACE, type WorkspaceStubHost } from "./with-workspace.js"; -import { type ThinkWorkspaceCompatibility, Workspace } from "./workspace.js"; +import { Workspace } from "./workspace.js"; interface ExecCall { command: string; @@ -87,7 +87,6 @@ function fakeRemote(): { let disposed = false; const stub = { fs: { marker: "fs" }, - useThink: false, git: { marker: "git" }, assets: undefined, artifacts: { marker: "artifacts" }, @@ -104,14 +103,20 @@ function fakeRemote(): { }; } -function fakeBrokenRemote(): { +function fakeLegacyFlagRemote(): { host: WorkspaceStubHost; disposed: () => boolean; } { + const { runtime } = fakeRuntime(true); let disposed = false; const stub = { + fs: { marker: "fs" }, + runtime, + git: { marker: "git" }, + assets: undefined, + artifacts: { marker: "artifacts" }, get useThink(): boolean { - throw new Error("compatibility lookup failed"); + throw new Error("legacy compatibility flag was read"); }, [Symbol.dispose]() { disposed = true; @@ -177,29 +182,15 @@ describe("getWorkspace — remote dispatch", () => { expect(disposed()).toBe(true); }); - it("disposes the remote stub when client initialization fails", async () => { - const { host, disposed } = fakeBrokenRemote(); + it("does not inspect the removed remote useThink flag", async () => { + const { host, disposed } = fakeLegacyFlagRemote(); - await expect(getWorkspace(host)).rejects.toThrow("compatibility lookup failed"); + const ws = await getWorkspace(host); + expect(ws.fs).toEqual({ marker: "fs" }); + expect(disposed()).toBe(false); + ws[Symbol.dispose](); expect(disposed()).toBe(true); }); - - it("adds Think compatibility when the remote Workspace enables it", async () => { - const workspace = new Workspace({ - storage: new SQLiteTestStorage(), - useThink: true, - }); - await workspace.fs.writeFile("/notes.txt", "hello"); - const host: WorkspaceStubHost = { - __getWorkspaceStub: () => Promise.resolve(workspace.stub()), - }; - - const client = (await getWorkspace(host)) as WorkspaceClient & ThinkWorkspaceCompatibility; - - expect(client).toHaveProperty("readFile"); - await expect(client.readFile("/notes.txt")).resolves.toBe("hello"); - await expect(client.readFile("/missing.txt")).resolves.toBeNull(); - }); }); describe("getWorkspace — local dispatch", () => { @@ -216,20 +207,6 @@ describe("getWorkspace — local dispatch", () => { const ws = await getWorkspace(host); expect(() => ws[Symbol.dispose]()).not.toThrow(); }); - - it("adds Think compatibility when the local Workspace enables it", async () => { - const workspace = new Workspace({ - storage: new SQLiteTestStorage(), - useThink: true, - }); - const host = { [WORKSPACE]: workspace }; - const { runtime } = fakeRuntime(); - Object.defineProperty(workspace, "runtime", { get: () => runtime }); - - const client = (await getWorkspace(host)) as WorkspaceClient & ThinkWorkspaceCompatibility; - - expect(client).toHaveProperty("readFile"); - }); }); describe("client runtime.exec — tagged template form", () => { diff --git a/packages/computer/src/client.ts b/packages/computer/src/client.ts index 6e3bf4b..393b957 100644 --- a/packages/computer/src/client.ts +++ b/packages/computer/src/client.ts @@ -42,11 +42,7 @@ import { decodeRuntimeEvents } from "./runtime/wire.js"; import { type ShellValue, sh } from "./sh.js"; import type { ExecEncoding } from "./shell.js"; import { WORKSPACE, type WorkspaceStubHost } from "./with-workspace.js"; -import { - createThinkCompatibility, - type ThinkWorkspaceCompatibility, - Workspace, -} from "./workspace.js"; +import { Workspace } from "./workspace.js"; // The remote runtime handle stub: a result / stream / kill surface // carried across Workers RPC. @@ -315,7 +311,7 @@ function withExecutionId( // `fs`, `git`, `artifacts`, and `assets` are the underlying surface's // members, passed through. The filesystem stub mirrors the local // filesystem, so it also serves as the common client type. -export interface WorkspaceClient extends Partial { +export interface WorkspaceClient { readonly fs: WorkspaceFilesystem; readonly runtime: WorkspaceRuntimeClient; // biome-ignore lint/suspicious/noExplicitAny: git type differs local vs remote @@ -332,7 +328,6 @@ function makeClient( surface: any, rehydrate: (handle: unknown, metadata?: RuntimeHandleMetadata) => unknown, dispose: () => void, - useThink: boolean, ): WorkspaceClient { const runtime = makeRuntimeClient( surface.runtime as UnderlyingRuntime, @@ -354,7 +349,6 @@ function makeClient( }, [Symbol.dispose]: dispose, }; - if (useThink) Object.assign(client, createThinkCompatibility(client.fs)); return client; } @@ -383,7 +377,6 @@ export async function getWorkspace(handle: WorkspaceHandle): Promise h, () => {}, - local.useThink, ); } // Remote path: fetch the stub over RPC and delegate to it. Handle @@ -397,7 +390,6 @@ export async function getWorkspace(handle: WorkspaceHandle): Promise { (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.(); }, - await stub.useThink, ); } catch (error) { (stub as { [Symbol.dispose]?: () => void })[Symbol.dispose]?.(); diff --git a/packages/computer/src/index.ts b/packages/computer/src/index.ts index 2d52105..c7d23dd 100644 --- a/packages/computer/src/index.ts +++ b/packages/computer/src/index.ts @@ -98,7 +98,6 @@ export { type SyncRetryIntent, type SyncRetryOptions, type SyncRetryScheduler, - type ThinkWorkspaceCompatibility, Workspace, type WorkspaceGitFactory, type WorkspaceOptions, diff --git a/packages/computer/src/stub.ts b/packages/computer/src/stub.ts index 0099424..4505dfe 100644 --- a/packages/computer/src/stub.ts +++ b/packages/computer/src/stub.ts @@ -599,7 +599,6 @@ export class WorkspaceStub extends RpcTarget { readonly #git: WorkspaceGitStub; readonly #assets: WorkspaceAssetsStub | undefined; readonly #artifacts: WorkspaceArtifactsStub; - readonly #useThink: boolean; constructor(ws: Workspace) { super(); @@ -608,7 +607,6 @@ export class WorkspaceStub extends RpcTarget { this.#git = new WorkspaceGitStub(ws); this.#assets = ws.assets === undefined ? undefined : new WorkspaceAssetsStub(ws); this.#artifacts = new WorkspaceArtifactsStub(ws.artifacts); - this.#useThink = ws.useThink; trackStub(this); } @@ -631,10 +629,6 @@ export class WorkspaceStub extends RpcTarget { return this.#fs; } - get useThink(): boolean { - return this.#useThink; - } - get runtime(): WorkspaceRuntimeStub { return this.#runtime; } diff --git a/packages/computer/src/workspace.test.ts b/packages/computer/src/workspace.test.ts index 95d3dc0..8570794 100644 --- a/packages/computer/src/workspace.test.ts +++ b/packages/computer/src/workspace.test.ts @@ -5,25 +5,12 @@ import type { BackendHandle, WorkspaceBackend } from "./backend.js"; import { createGitClient } from "./git/index.js"; import type { WorkspaceModuleBackend } from "./runtime/types.js"; import { WorkspaceTransportError } from "./transport-failure.js"; -import { type ThinkWorkspaceCompatibility, Workspace } from "./workspace.js"; +import { Workspace } from "./workspace.js"; function makeStorage(): SQLiteTestStorage { return new SQLiteTestStorage(); } -function expectThinkWorkspace( - ws: Workspace, -): asserts ws is Workspace & ThinkWorkspaceCompatibility { - expect(ws).toHaveProperty("readFile"); - expect(ws).toHaveProperty("readFileBytes"); - expect(ws).toHaveProperty("writeFile"); - expect(ws).toHaveProperty("readDir"); - expect(ws).toHaveProperty("glob"); - expect(ws).toHaveProperty("mkdir"); - expect(ws).toHaveProperty("rm"); - expect(ws).toHaveProperty("stat"); -} - // In-process fakes. We never spawn anything from the package // code; the backend's only contract is "produce a SyncRPC // stub that computerd would speak". A plain object is enough. @@ -1463,49 +1450,15 @@ describe("Workspace transport-failure invalidation", () => { ]); }); }); -describe("Workspace Think compatibility", () => { - it("adds Think-compatible filesystem methods when useThink is true", async () => { +describe("Workspace compatibility surface", () => { + it("does not accept useThink or add root-level filesystem methods", () => { const ws = new Workspace({ storage: makeStorage(), + // @ts-expect-error useThink was removed with the compatibility layer. useThink: true, - now: () => 1_700_000_000_000, - }); - - expectThinkWorkspace(ws); - - await ws.mkdir("/workspace/notes", { recursive: true }); - await ws.writeFile("/workspace/notes/a.txt", "hello"); - await ws.writeFile("/workspace/notes/b.md", "# title"); - - expect(await ws.readFile("/workspace/notes/a.txt")).toBe("hello"); - expect(await ws.readFile("/workspace/missing.txt")).toBeNull(); - expect(new TextDecoder().decode(await ws.readFileBytes("/workspace/notes/a.txt"))).toBe( - "hello", - ); - expect(await ws.readFileBytes("/workspace/missing.txt")).toBeNull(); - - await expect(ws.stat("/workspace/notes/a.txt")).resolves.toMatchObject({ - path: "/workspace/notes/a.txt", - name: "a.txt", - type: "file", - size: 5, }); - await expect(ws.stat("/workspace/missing.txt")).resolves.toBeNull(); - - await expect(ws.readDir("/workspace/notes", { limit: 1, offset: 1 })).resolves.toEqual([ - expect.objectContaining({ path: "/workspace/notes/b.md", name: "b.md", type: "file" }), - ]); - await expect(ws.glob("/workspace/notes/**/*.txt")).resolves.toEqual([ - expect.objectContaining({ path: "/workspace/notes/a.txt", name: "a.txt", type: "file" }), - ]); - - await ws.rm("/workspace/notes/a.txt", { force: true }); - expect(await ws.readFile("/workspace/notes/a.txt")).toBeNull(); - }); - - it("does not add Think compatibility methods by default", () => { - const ws = new Workspace({ storage: makeStorage() }); + expect(ws).not.toHaveProperty("useThink"); expect(ws).not.toHaveProperty("readFile"); expect(ws).not.toHaveProperty("readFileBytes"); expect(ws).not.toHaveProperty("writeFile"); @@ -1514,5 +1467,6 @@ describe("Workspace Think compatibility", () => { expect(ws).not.toHaveProperty("mkdir"); expect(ws).not.toHaveProperty("rm"); expect(ws).not.toHaveProperty("stat"); + expect(ws.stub()).not.toHaveProperty("useThink"); }); }); diff --git a/packages/computer/src/workspace.ts b/packages/computer/src/workspace.ts index ac4b9e9..56b1e20 100644 --- a/packages/computer/src/workspace.ts +++ b/packages/computer/src/workspace.ts @@ -149,40 +149,8 @@ export interface WorkspaceOptions { binding: Artifacts; sessionId?: string; }; - - // Add Think's string-oriented WorkspaceLike filesystem methods - // directly to the Workspace instance. This is off by default so - // the primary Workspace API stays on the `workspace.fs` facade; - // enable it when assigning a Workspace to `Think.workspace`. - useThink?: boolean; -} - -export interface ThinkFileInfo { - path: string; - name: string; - type: "file" | "directory"; - mimeType: string; - size: number; - createdAt: number; - updatedAt: number; } -export interface ThinkWorkspaceCompatibility { - readFile(path: string): Promise; - readFileBytes(path: string): Promise; - writeFile(path: string, content: string): Promise; - readDir(dir: string, opts?: { limit?: number; offset?: number }): Promise; - rm(path: string, opts?: { recursive?: boolean; force?: boolean }): Promise; - glob(pattern: string): Promise; - mkdir(path: string, opts?: { recursive?: boolean }): Promise; - stat(path: string): Promise; -} - -export type ThinkWorkspaceFilesystem = Pick< - WorkspaceFilesystem, - "find" | "mkdir" | "readFile" | "readdir" | "rm" | "stat" | "writeFile" ->; - export type WorkspaceGitFactory = GitClientFactory; const GIT_NOT_CONFIGURED_MESSAGE = @@ -222,7 +190,6 @@ export class Workspace { readonly #sessionId: string; readonly #gitFactory: WorkspaceGitFactory | undefined; readonly #defaultGitIdentity: GitIdentity | undefined; - readonly #useThink: boolean; readonly #assets: AssetsClient | undefined; readonly #artifacts: ArtifactClient; // Lazily-constructed git client, cached so the dynamic @@ -258,15 +225,6 @@ export class Workspace { // and updates it. See docs/02 "Concurrent mutators". readonly #mutationTails = new Map>(); - declare readonly readFile?: ThinkWorkspaceCompatibility["readFile"]; - declare readonly readFileBytes?: ThinkWorkspaceCompatibility["readFileBytes"]; - declare readonly writeFile?: ThinkWorkspaceCompatibility["writeFile"]; - declare readonly readDir?: ThinkWorkspaceCompatibility["readDir"]; - declare readonly rm?: ThinkWorkspaceCompatibility["rm"]; - declare readonly glob?: ThinkWorkspaceCompatibility["glob"]; - declare readonly mkdir?: ThinkWorkspaceCompatibility["mkdir"]; - declare readonly stat?: ThinkWorkspaceCompatibility["stat"]; - constructor(options: WorkspaceOptions) { this.#now = options.now ?? Date.now; this.#retryScheduler = options.retryScheduler; @@ -288,7 +246,6 @@ export class Workspace { this.#sessionId = options.sessionId ?? ""; this.#gitFactory = options.git; this.#defaultGitIdentity = options.defaultGitIdentity; - this.#useThink = options.useThink ?? false; this.#artifacts = options.artifacts ? createArtifact( options.artifacts.binding, @@ -332,10 +289,6 @@ export class Workspace { mounts: this.#mounts, }); this.#assets = typeof options.assets === "function" ? options.assets(this) : options.assets; - if (this.#useThink) { - const think = createThinkCompatibility(this.fs); - Object.assign(this, think); - } } // Force every registered mount to materialize. Idempotent; safe to @@ -380,10 +333,6 @@ export class Workspace { return this.#fs; } - get useThink(): boolean { - return this.#useThink; - } - // Identifier for this workspace / session, as passed to the // constructor. Empty string when the caller did not supply one. // Forwarded to mount factories and used by the assets module to @@ -1061,161 +1010,3 @@ function createDisabledArtifactsClient(): ArtifactClient { }, } as ArtifactClient; } - -export function createThinkCompatibility( - fs: ThinkWorkspaceFilesystem, -): ThinkWorkspaceCompatibility { - return { - async readFile(path) { - try { - return await fs.readFile(path, "utf8"); - } catch (err) { - if (isEnoent(err)) return null; - throw err; - } - }, - async readFileBytes(path) { - try { - return await drainBytes(await fs.readFile(path)); - } catch (err) { - if (isEnoent(err)) return null; - throw err; - } - }, - async writeFile(path, content) { - await fs.writeFile(path, content); - }, - async readDir(dir, opts) { - const entries = await fs.readdir(dir); - const offset = opts?.offset ?? 0; - const limit = opts?.limit ?? entries.length; - return entries.slice(offset, offset + limit).map((entry) => - toThinkFileInfo({ - path: joinPath(dir, entry.name), - name: entry.name, - size: 0, - mtime: 0, - isDirectory: entry.isDirectory, - isFile: entry.isFile, - }), - ); - }, - async rm(path, opts) { - await fs.rm(path, opts); - }, - async glob(pattern) { - const { directory, relativePattern } = splitGlobPattern(pattern); - const matches = await fs.find(directory, relativePattern); - return matches.map((match) => - toThinkFileInfo({ - path: match.path, - name: basename(match.path), - size: 0, - mtime: 0, - isDirectory: match.type === "dir", - isFile: match.type === "file", - }), - ); - }, - async mkdir(path, opts) { - await fs.mkdir(path, opts); - }, - async stat(path) { - try { - const stat = await fs.stat(path); - return toThinkFileInfo({ ...stat, path, name: basename(path) }); - } catch (err) { - if (isEnoent(err)) return null; - throw err; - } - }, - }; -} - -async function drainBytes(stream: ReadableStream): Promise { - const reader = stream.getReader(); - const parts: Uint8Array[] = []; - let total = 0; - try { - while (true) { - const { value, done } = await reader.read(); - if (done) break; - if (!value) continue; - parts.push(value); - total += value.byteLength; - } - } finally { - reader.releaseLock(); - } - if (parts.length === 1) return parts[0]; - const out = new Uint8Array(total); - let offset = 0; - for (const part of parts) { - out.set(part, offset); - offset += part.byteLength; - } - return out; -} - -function toThinkFileInfo(input: { - path: string; - name: string; - size: number; - mtime: number; - isDirectory: boolean; - isFile: boolean; -}): ThinkFileInfo { - const type = input.isDirectory ? "directory" : "file"; - return { - path: input.path, - name: input.name, - type, - mimeType: type === "directory" ? "inode/directory" : "application/octet-stream", - size: input.size, - createdAt: input.mtime, - updatedAt: input.mtime, - }; -} - -function splitGlobPattern(pattern: string): { directory: string; relativePattern?: string } { - const normalized = pattern.startsWith("/") ? pattern : `/workspace/${pattern}`; - const wildcard = firstWildcardIndex(normalized); - if (wildcard === -1) { - return { directory: dirname(normalized), relativePattern: basename(normalized) }; - } - const slash = normalized.lastIndexOf("/", wildcard); - const directory = slash <= 0 ? "/" : normalized.slice(0, slash); - const relativePattern = normalized.slice(slash + 1); - return { directory, relativePattern }; -} - -function firstWildcardIndex(pattern: string): number { - const star = pattern.indexOf("*"); - const question = pattern.indexOf("?"); - if (star === -1) return question; - if (question === -1) return star; - return Math.min(star, question); -} - -function joinPath(dir: string, name: string): string { - return dir === "/" ? `/${name}` : `${dir}/${name}`; -} - -function dirname(path: string): string { - const index = path.lastIndexOf("/"); - if (index <= 0) return "/"; - return path.slice(0, index); -} - -function basename(path: string): string { - const trimmed = path.endsWith("/") && path !== "/" ? path.slice(0, -1) : path; - const index = trimmed.lastIndexOf("/"); - return index === -1 ? trimmed : trimmed.slice(index + 1); -} - -function isEnoent(err: unknown): boolean { - if (!err || typeof err !== "object") return false; - const e = err as { code?: string; message?: string }; - if (e.code === "ENOENT") return true; - return typeof e.message === "string" && /ENOENT|no such/i.test(e.message); -}