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/) + }) +})