From 2efd7f30e38eade53e456de8e9048fb02559babd Mon Sep 17 00:00:00 2001 From: Sarav Date: Wed, 16 Sep 2026 10:49:27 +0530 Subject: [PATCH] fix: [AI-9171] create a quick workspace from an already-linked project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker's "＋ Create a quick workspace here" row always failed when the project was already linked, and told the user to re-run the command they were already inside. `createThenBindOrRebind` already had a correct rebind branch — it was simply unreachable. Step one called `createAndBind`, and the server's `create_and_bind` pre-checks both identifiers and 409s *before* creating anything, deliberately, so a binding conflict cannot strand a half-created workspace. On an already linked project that refuses the whole call, so nothing was created and the rebind below it was dead code. The row's own hint promised the opposite: "Creates a new workspace and repoints this project to it". Split the two cases: - unlinked: unchanged — `createAndBind` still creates and binds in one server-side transaction, which is what makes a stranded workspace impossible there. - already linked: create the workspace unbound via `POST /datamates/`, then repoint through the existing rebind path. `createWorkspaceUnbound` sends `memory_enabled` and `knowledge_engine_enabled` explicitly. `POST /datamates/` is the SaaS/extension creation path and defaults both to false, while the create-and-bind path sets both true — so without this the same menu row would hand back a differently configured workspace depending only on whether the project happened to be linked, with memory and the knowledge engine silently off. The comment names the coupling so the two move together. Also corrected the 409 message. After this change a conflict can only mean another workspace claimed the project mid-selection, so it says that instead of directing the user back into the command they are already running. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/altimate/workspace/api-client.ts | 40 +++++ packages/opencode/src/cli/cmd/link.ts | 69 ++++++--- .../create-workspace-unbound.test.ts | 143 ++++++++++++++++++ 3 files changed, 230 insertions(+), 22 deletions(-) create mode 100644 packages/opencode/test/altimate/workspace/create-workspace-unbound.test.ts diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index a1366c2cb..4053d738a 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -395,6 +395,46 @@ export namespace WorkspaceApi { }) } + /** Create a workspace WITHOUT binding anything to it. + * + * ``createAndBind`` is the right call for an unlinked project: it creates and + * binds in one server-side transaction, so a binding conflict cannot strand a + * workspace. But it pre-checks the identifiers and 409s *before* creating, + * which makes it unusable when the project is already linked — there is + * nothing to create, and the caller's rebind never gets a target (AI-9171). + * This is the two-step path for that case: create here, then rebind. + * + * The flags below deliberately mirror ``_create_datamate_flush_only`` in + * altimate-backend, which is what ``createAndBind`` reaches. ``POST + * /datamates/`` is the SaaS/extension creation path and defaults BOTH to + * false, so omitting them would hand a differently-configured workspace to + * whichever caller happened to be already linked — same menu row, memory and + * knowledge engine silently off. If the backend's workspace defaults move, + * this has to move with them; there is no endpoint that applies them without + * also binding. + */ + export async function createWorkspaceUnbound(input: { + name: string + description?: string + }): Promise<{ id: number; name: string }> { + const data = await req<{ id: number }>("POST", "/", { + base: "/datamates", + body: { + name: input.name, + description: input.description ?? null, + integrations: [], + memory_enabled: true, + knowledge_engine_enabled: true, + privacy: "private", + }, + }) + const id = Number(data?.id) + if (!Number.isSafeInteger(id) || id <= 0) { + throw new Error(`Workspace was created but the server returned no usable id (${String(data?.id)}).`) + } + return { id, name: input.name } + } + export async function bindExisting( datamateId: number, identifier: ProjectIdentifier, diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 9da25ac1c..3ad8b5af1 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -22,6 +22,7 @@ import { NotConfiguredError, NotFoundError, PreconditionFailedError, + type Binding, type DatamateRef, type MatchedIdentifier, type ProjectBindingLookup, @@ -484,19 +485,36 @@ async function createThenBindOrRebind( ): Promise { const spin = prompts.spinner() spin.start(`Creating workspace "${name}"...`) - let created: Awaited> + let created: { datamate: DatamateRef; binding?: Binding; manage_url?: string } try { - created = await WorkspaceApi.createAndBind({ name, identifier }) + // Two different creates, because the server offers two different things. + // + // Unlinked: ``createAndBind`` creates and binds in ONE transaction, so a + // conflicting binding can never strand a half-created workspace. + // + // Already linked: that same atomicity makes it unusable. ``create_and_bind`` + // pre-checks the identifiers and 409s *before* creating anything, so the + // rebind below never got a target and this row simply always failed — with + // an error telling the user to re-run the command they were already inside + // (AI-9171). Create unbound first, then repoint, which is what the row's + // own hint promises. + if (existing) { + const ws = await WorkspaceApi.createWorkspaceUnbound({ name }) + created = { datamate: ws } + } else { + created = await WorkspaceApi.createAndBind({ name, identifier }) + } } catch (err) { spin.stop("Failed to create workspace.", 1) - // A 409 from create means someone else's binding on the same - // remote/path beat us. If the pre-check already knew about it, the user - // can pick from the list; if the pre-check missed it, this is the - // authoritative signal — surface it and hint the picker. + // A 409 here can only be a binding this caller did not know about: the + // already-linked path does not bind at all, and the unlinked path only + // reaches the server after a pre-check that said the project was free. So + // someone else's binding on the same remote/path landed in between. if (err instanceof ConflictError) { const existingName = conflictExistingName(err.detail) prompts.log.error( - `This project is already linked to "${existingName}". Re-run \`altimate-code link\` to switch to a different workspace.`, + `Another workspace, "${existingName}", claimed this project while you were choosing. ` + + `Nothing was created. Run \`altimate-code link\` again to see the current list.`, ) } else { prompts.log.error(err instanceof Error ? err.message : String(err)) @@ -510,10 +528,9 @@ async function createThenBindOrRebind( const safeCreatedName = stripControlChars(created.datamate.name) spin.stop(`Workspace "${safeCreatedName}" created.`) - // If the project was already linked, the new workspace exists but the - // binding still points at the OLD workspace — rebind so the project is - // now bound to the freshly-created one. Otherwise createAndBind already - // wrote the binding as part of the atomic create; we're done. + // The already-linked path created an unbound workspace above, so the binding + // still points at the OLD one — repoint it now. The unlinked path already got + // its binding from the atomic create, so there is nothing left to do. if (existing) { const rebindSpin = prompts.spinner() rebindSpin.start(`Repointing project at "${safeCreatedName}"...`) @@ -537,23 +554,31 @@ async function createThenBindOrRebind( // Prefer the canonicalized ``identifier.projectPath`` over the raw // ``--directory`` argument so ``altimate-code link -d ./myproj`` and its // symlink-resolved twin both write under the same cache key (Kilo cycle 6). + // The unbound path has no server binding row to echo, so fall back to the + // identifier we just rebound on — the same values the rebind wrote. await recordApprovedBinding(identifier.projectPath ?? directory, { datamateId: created.datamate.id, datamateName: created.datamate.name, - repoRemote: created.binding.repo_remote, - projectPath: created.binding.project_path, + repoRemote: created.binding?.repo_remote ?? identifier.repoRemote ?? null, + projectPath: created.binding?.project_path ?? identifier.projectPath ?? null, linkedAt: Date.now(), }, { awaitBackfill: true }) prompts.log.info("Saved memory blocks will sync to this workspace if memory is enabled for it.") - prompts.log.info(`Manage it at: ${created.manage_url}`) - // Guard against a server that hands back a non-http(s) manage_url — ``open`` - // delegates to the OS handler, so a rogue value could launch an unrelated - // application. Log a warning and skip the auto-open rather than trusting - // whatever protocol the URL parses to. - if (isSafeHttpUrl(created.manage_url)) { - await open(created.manage_url).catch(() => undefined) - } else { - prompts.log.warn(`Skipped auto-open: manage_url is not an http/https URL.`) + // ``createAndBind`` hands back a manage_url; the unbound create does not, so + // derive it from credentials exactly as the rest of this file does. Null on + // BYOK / unresolvable deployments — then there is simply nothing to show. + const manageUrl = created.manage_url ?? (await manageUrlFor(created.datamate.id)) + if (manageUrl) { + prompts.log.info(`Manage it at: ${manageUrl}`) + // Guard against a server that hands back a non-http(s) manage_url — ``open`` + // delegates to the OS handler, so a rogue value could launch an unrelated + // application. Log a warning and skip the auto-open rather than trusting + // whatever protocol the URL parses to. + if (isSafeHttpUrl(manageUrl)) { + await open(manageUrl).catch(() => undefined) + } else { + prompts.log.warn(`Skipped auto-open: manage_url is not an http/https URL.`) + } } prompts.outro("Done.") } diff --git a/packages/opencode/test/altimate/workspace/create-workspace-unbound.test.ts b/packages/opencode/test/altimate/workspace/create-workspace-unbound.test.ts new file mode 100644 index 000000000..16e23e53d --- /dev/null +++ b/packages/opencode/test/altimate/workspace/create-workspace-unbound.test.ts @@ -0,0 +1,143 @@ +// altimate_change - new file +// Coverage for WorkspaceApi.createWorkspaceUnbound (AI-9171). +// +// This exists because `altimate link`'s "create a quick workspace" row always +// failed on an already-linked project: it went through `createAndBind`, whose +// server handler pre-checks the identifiers and 409s BEFORE creating, so the +// rebind that was supposed to follow never got a target. The fix creates the +// workspace unbound first, then repoints. +// +// The assertions worth having are about the REQUEST, not the response. Two +// creation paths now exist, and the silent failure mode is that they disagree: +// `POST /datamates/` defaults memory and the knowledge engine to false, while +// the create-and-bind path sets both true. If this drifts, the same menu row +// produces a differently-configured workspace depending only on whether the +// project happened to be linked — which nothing else would catch. +import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test" +import { mkdirSync, writeFileSync } from "node:fs" +import path from "node:path" +import os from "node:os" + +const ORIGINAL_TEST_HOME = process.env.OPENCODE_TEST_HOME +const SANDBOX = path.join(os.tmpdir(), `altimate-createunbound-${process.pid}-${Date.now()}`) +mkdirSync(path.join(SANDBOX, "home", ".altimate"), { recursive: true }) +process.env.OPENCODE_TEST_HOME = path.join(SANDBOX, "home") + +const API_URL = "https://api.example.test" +const TENANT = "acme" + +// A real credentials file, so the module resolves them the same way it does in +// production rather than through a stubbed export. +writeFileSync( + path.join(SANDBOX, "home", ".altimate", "altimate.json"), + JSON.stringify({ + altimateUrl: API_URL, + altimateInstanceName: TENANT, + altimateApiKey: "test-key", + }), +) + +const { WorkspaceApi } = await import("@/altimate/workspace/api-client") + +const ORIGINAL_FETCH = globalThis.fetch + +interface Captured { + url: string + method: string + body: Record +} + +let captured: Captured[] = [] + +/** Stub fetch, recording each request and replying with `reply`. */ +function respondWith(status: number, reply: unknown) { + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + captured.push({ + url: String(input), + method: String(init?.method ?? "GET"), + body: init?.body ? JSON.parse(String(init.body)) : {}, + }) + return new Response(JSON.stringify(reply), { + status, + headers: { "content-type": "application/json" }, + }) + }) as typeof globalThis.fetch +} + +beforeEach(() => { + captured = [] +}) + +afterEach(() => { + globalThis.fetch = ORIGINAL_FETCH +}) + +afterAll(() => { + if (ORIGINAL_TEST_HOME === undefined) delete process.env.OPENCODE_TEST_HOME + else process.env.OPENCODE_TEST_HOME = ORIGINAL_TEST_HOME +}) + +describe("createWorkspaceUnbound", () => { + test("posts to /datamates/, NOT to the binding router", async () => { + respondWith(200, { id: 77 }) + await WorkspaceApi.createWorkspaceUnbound({ name: "jaffle_shop" }) + + expect(captured).toHaveLength(1) + expect(captured[0].method).toBe("POST") + // The whole point: this must not reach create_and_bind, which would 409. + expect(captured[0].url).not.toContain("datamate-project-bindings") + expect(captured[0].url).toContain("/datamates/") + }) + + test("sends no project identifier — binding is the caller's next step", async () => { + respondWith(200, { id: 77 }) + await WorkspaceApi.createWorkspaceUnbound({ name: "jaffle_shop" }) + + expect(captured[0].body).not.toHaveProperty("repo_remote") + expect(captured[0].body).not.toHaveProperty("project_path") + }) + + test("applies the workspace defaults, not the SaaS ones", async () => { + respondWith(200, { id: 77 }) + await WorkspaceApi.createWorkspaceUnbound({ name: "jaffle_shop" }) + + // `POST /datamates/` defaults both to false. Sending them explicitly is what + // keeps an already-linked project's new workspace configured like every + // other CLI-created one. Dropping either line is the regression. + expect(captured[0].body.memory_enabled).toBe(true) + expect(captured[0].body.knowledge_engine_enabled).toBe(true) + expect(captured[0].body.privacy).toBe("private") + // Required by CreateDatamateRequest — omitting it is a 422. + expect(captured[0].body.integrations).toEqual([]) + }) + + test("returns the created id and the caller's name", async () => { + respondWith(200, { id: 77 }) + const created = await WorkspaceApi.createWorkspaceUnbound({ name: "jaffle_shop" }) + expect(created).toEqual({ id: 77, name: "jaffle_shop" }) + }) + + test("passes a description through when given, null when not", async () => { + respondWith(200, { id: 77 }) + await WorkspaceApi.createWorkspaceUnbound({ name: "a", description: "from the CLI" }) + expect(captured[0].body.description).toBe("from the CLI") + + captured = [] + respondWith(200, { id: 78 }) + await WorkspaceApi.createWorkspaceUnbound({ name: "b" }) + expect(captured[0].body.description).toBeNull() + }) + + test("rejects a response with no usable id rather than returning NaN", async () => { + // A workspace the caller cannot then rebind to is worse than a clear error: + // `Number(undefined)` is NaN, which would reach the rebind as a garbage + // target id. + respondWith(200, { id: null }) + await expect(WorkspaceApi.createWorkspaceUnbound({ name: "x" })).rejects.toThrow(/no usable id/) + }) + + test("rejects a non-integer id", async () => { + respondWith(200, { id: "not-a-number" }) + await expect(WorkspaceApi.createWorkspaceUnbound({ name: "x" })).rejects.toThrow(/no usable id/) + }) +})