diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index a1366c2cbc..e45682f3e7 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -395,6 +395,77 @@ 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. + */ + /** Who the next call will act as. + * + * A create-then-rebind pair is two requests, and `req()` resolves credentials + * independently for each. If the account changes in between — a re-login, an + * edited `altimate.json` — the workspace is created in one tenant and the + * rebind is sent to another with an id that is local to the first. Callers + * capture this before the create and re-check it before the rebind. + * + * The API key is deliberately not part of it: rotating a key for the same + * user on the same tenant is not an identity change, and comparing it would + * abort a legitimate flow. */ + export async function accountFingerprint(): Promise<{ apiUrl: string; tenant: string }> { + const c = await creds() + return { apiUrl: c.url, tenant: c.instance } + } + + /** True when `before` still describes the account in effect. */ + export async function sameAccount(before: { apiUrl: string; tenant: string }): Promise { + const now = await accountFingerprint().catch(() => null) + return now !== null && now.apiUrl === before.apiUrl && now.tenant === before.tenant + } + + 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", + }, + }) + // `typeof` FIRST, before any arithmetic. `Number()` coerces, so the + // previous `Number.isSafeInteger(Number(data?.id))` accepted `true` as 1, + // `"7"` as 7 and `[5]` as 5 — a malformed body would have rebound the + // project to whatever those coerced to (workspace 1, in the boolean case) + // instead of failing. The server's `CreateDatamateResponse` is `{id: int}` + // and FastAPI enforces it, so anything else here is a contract break worth + // refusing loudly rather than guessing at. + const id: unknown = (data as { id?: unknown } | null | undefined)?.id + if (typeof id !== "number" || !Number.isSafeInteger(id) || id <= 0) { + throw new WorkspaceApiError( + `Workspace was created but the server returned no usable id (${JSON.stringify(id) ?? "undefined"}).`, + ) + } + 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 9da25ac1cf..562a59aa1a 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, @@ -475,8 +476,12 @@ function handoffFailureMessage(result: Extract): s * real (billable) SaaS resource the CLI knows nothing about and the project * is still bound to the old workspace (M2 in the consensus review). When * rebind fails, the error message tells the user the workspace was created - * and how to recover; we do NOT silently swallow the orphan. */ -async function createThenBindOrRebind( + * and how to recover; we do NOT silently swallow the orphan. + * + * Exported for tests. The branch it picks — atomic create-and-bind when the + * project is free, unbound-create-then-rebind when it is already linked — is + * the whole of AI-9171, and nothing else in this file can assert it. */ +export async function createThenBindOrRebind( identifier: ProjectIdentifier, name: string, directory: string, @@ -484,20 +489,59 @@ async function createThenBindOrRebind( ): Promise { const spin = prompts.spinner() spin.start(`Creating workspace "${name}"...`) - let created: Awaited> + // Discriminated on how the workspace was made, because the two creates return + // genuinely different things: only `bound` carries a server binding row and a + // manage_url. An optional-field shape let the rest of this function reach for + // `binding` on the path that never has one and silently fall through to a + // default. (review, PR #1314) + type Created = + | { via: "bound"; datamate: DatamateRef; binding: Binding; manage_url: string } + | { via: "unbound"; datamate: DatamateRef } + let created: Created + // Captured BEFORE the create and re-checked before the rebind. Each request + // resolves credentials on its own, so an account switch in between would + // create the workspace on one tenant and rebind on another using an id that + // is local to the first. (review, PR #1314) + const account = await WorkspaceApi.accountFingerprint().catch(() => null) 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 = { via: "unbound", datamate: ws } + } else { + const res = await WorkspaceApi.createAndBind({ name, identifier }) + created = { via: "bound", datamate: res.datamate, binding: res.binding, manage_url: res.manage_url } + } } 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. + // Split by which call actually ran, because they cannot 409 for the same + // reason. `createAndBind` sends the project identifiers, so its conflict is + // a binding race. The unbound create sends none — it cannot produce an + // identity conflict at all, so attributing one there would have sent the + // user looking for a race that did not happen. (review, PR #1314) 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.`, - ) + if (existing) { + prompts.log.error( + `The workspace could not be created: ${err.message}. Nothing was created, and this ` + + `project is still linked to "${stripControlChars(existing.datamate.name)}".`, + ) + } else { + const existingName = conflictExistingName(err.detail) + prompts.log.error( + `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,20 +554,34 @@ 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. + let reboundBinding: Binding | null = null if (existing) { const rebindSpin = prompts.spinner() rebindSpin.start(`Repointing project at "${safeCreatedName}"...`) + // The workspace exists on the account that was in effect a moment ago, and + // its id means nothing anywhere else. Rebinding under a different account + // would point this project at whatever id collides there. + if (account && !(await WorkspaceApi.sameAccount(account))) { + rebindSpin.stop("Could not repoint the project.", 1) + prompts.log.error( + `The signed-in account changed while "${safeCreatedName}" was being created, so it was ` + + `not linked to this project. The workspace exists on the previous account. Re-run ` + + `\`altimate-code link\` to link this project on the account you are on now.`, + ) + process.exitCode = 1 + return + } try { - await rebindByMatchedIdentifier({ + const res = await rebindByMatchedIdentifier({ identifier, targetDatamateId: created.datamate.id, expectedCurrentDatamateId: existing.datamate.id, matchedBy: existing.matchedBy, }) + reboundBinding = res.binding rebindSpin.stop(`Project is now linked to "${safeCreatedName}".`) } catch (err) { rebindSpin.stop("Could not repoint the project.", 1) @@ -537,23 +595,35 @@ 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). + // Whichever call last wrote the row is what gets cached. `createAndBind` + // returns it directly; the unbound path gets it from the rebind. Caching the + // local identifiers instead would record fields the server never stored — a + // path-keyed row rebound through `/by-path` would be cached carrying a + // `repo_remote` that is not on the server's row. (review, PR #1314) + const serverBinding = created.via === "bound" ? created.binding : reboundBinding await recordApprovedBinding(identifier.projectPath ?? directory, { datamateId: created.datamate.id, datamateName: created.datamate.name, - repoRemote: created.binding.repo_remote, - projectPath: created.binding.project_path, + repoRemote: serverBinding?.repo_remote ?? identifier.repoRemote ?? null, + projectPath: serverBinding?.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.via === "bound" ? 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/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 8c81c11b78..cb9d6ee532 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -37,6 +37,7 @@ import { NotFoundError, PreconditionFailedError, WorkspaceApi, + type Binding, type DatamateRef, type MatchedIdentifier, type ProjectBindingLookup, @@ -493,22 +494,48 @@ function toastHandoffFailure(api: TuiPluginApi, result: Extract { api.ui.dialog.clear() - let res: Awaited> + // The same split the CLI makes in `cli/cmd/link.ts` (AI-9171), for the same + // reason. `createAndBind` pre-checks the project identifiers server-side and + // 409s BEFORE creating anything, so on an already-linked project the catch + // below fired and the rebind further down was unreachable — this row never + // worked. The `rebindFrom` comment above described the opposite ("creates and + // binds, leaving the binding pointing at the OLD workspace"); that was the + // assumption the bug rested on. Create unbound first, then repoint. + type Created = + | { via: "bound"; datamate: DatamateRef; binding: Binding } + | { via: "unbound"; datamate: DatamateRef } + let res: Created + // Captured before the create and re-checked before the rebind: the two + // requests resolve credentials independently, and a workspace id means + // nothing on a different account. + const account = await WorkspaceApi.accountFingerprint().catch(() => null) try { - res = await WorkspaceApi.createAndBind({ name, identifier }) + if (rebindFrom) { + const ws = await WorkspaceApi.createWorkspaceUnbound({ name }) + res = { via: "unbound", datamate: ws } + } else { + const created = await WorkspaceApi.createAndBind({ name, identifier }) + res = { via: "bound", datamate: created.datamate, binding: created.binding } + } } catch (err) { - if (err instanceof ConflictError) { + // Only the bound path sends identifiers, so only it can lose an identity + // race. The unbound create sends none. + if (err instanceof ConflictError && !rebindFrom) { api.ui.toast({ variant: "warning", message: `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Use the palette's "Link this project to a workspace" to change.`, @@ -522,19 +549,27 @@ async function createAndBindInline( return } + let reboundBinding: Binding | null = null if (rebindFrom) { - // The atomic create-and-bind wrote a NEW binding for the new workspace, - // but the existing binding for THIS project's remote/path still points - // at the old workspace. Repoint via the matched-identifier rebind - // endpoint. If rebind fails, tell the user the workspace exists but - // the link didn't switch — do not silently orphan. + // The workspace above was created UNBOUND, so this project's binding still + // points at the old one. Repoint it. If this fails the workspace exists but + // the link did not switch — say so rather than silently orphan it. + if (account && !(await WorkspaceApi.sameAccount(account))) { + api.ui.toast({ + variant: "error", + message: `The signed-in account changed while "${res.datamate.name}" was being created, so it was not linked to this project. The workspace exists on the previous account.`, + duration: 15_000, + }) + return + } try { - await rebindByMatchedIdentifier({ + const rebound = await rebindByMatchedIdentifier({ identifier, targetDatamateId: res.datamate.id, expectedCurrentDatamateId: rebindFrom.expectedCurrentDatamateId, matchedBy: rebindFrom.matchedBy, }) + reboundBinding = rebound.binding } catch (err) { api.ui.toast({ variant: "error", @@ -545,6 +580,10 @@ async function createAndBindInline( } } + // Whichever call actually wrote the server row: the atomic create returns it, + // the unbound path gets it from the rebind. + const serverBinding = res.via === "bound" ? res.binding : reboundBinding + // Post-success tail — this function is invoked fire-and-forget // (``void createAndBindInline(...)``), so a bare rejection here would // surface as an unhandled promise and terminate the TUI. Contain the @@ -557,8 +596,8 @@ async function createAndBindInline( await recordApprovedBinding(api.state.path.directory, { datamateId: res.datamate.id, datamateName: res.datamate.name, - repoRemote: res.binding.repo_remote, - projectPath: res.binding.project_path, + repoRemote: serverBinding?.repo_remote ?? identifier.repoRemote ?? null, + projectPath: serverBinding?.project_path ?? identifier.projectPath ?? null, linkedAt: Date.now(), }) await showLinkedConfirmation(api, "Created", res.datamate.id, res.datamate.name) diff --git a/packages/opencode/test/altimate/workspace/create-then-rebind.test.ts b/packages/opencode/test/altimate/workspace/create-then-rebind.test.ts new file mode 100644 index 0000000000..b9f7b2dc7b --- /dev/null +++ b/packages/opencode/test/altimate/workspace/create-then-rebind.test.ts @@ -0,0 +1,218 @@ +// altimate_change - new file +// Control-flow coverage for the "create a quick workspace" row (AI-9171), on +// BOTH surfaces that offer it. +// +// The bug was never in a request shape — it was in which request got sent. +// `createAndBind` 409s before creating anything when the project is already +// linked, so the rebind that was meant to follow was unreachable and the row +// always failed. A test that only checks payloads cannot see that, which is why +// these assert the *sequence of endpoints* instead. +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 ORIGINAL_XDG_STATE_HOME = process.env.XDG_STATE_HOME +const SANDBOX = path.join(os.tmpdir(), `altimate-createflow-${process.pid}-${Date.now()}`) +mkdirSync(path.join(SANDBOX, "home", ".altimate"), { recursive: true }) +mkdirSync(path.join(SANDBOX, "state"), { recursive: true }) +// Set before the modules under test are imported: they resolve `Global.Path` +// at import time, so this cannot move into `beforeEach`. Restored in +// `afterAll`, and the sandbox is per-pid so parallel files cannot collide. +process.env.OPENCODE_TEST_HOME = path.join(SANDBOX, "home") +process.env.XDG_STATE_HOME = path.join(SANDBOX, "state") + +const API_URL = "https://api.example.test" +writeFileSync( + path.join(SANDBOX, "home", ".altimate", "altimate.json"), + JSON.stringify({ altimateUrl: API_URL, altimateInstanceName: "acme", altimateApiKey: "test-key" }), +) + +const { createThenBindOrRebind } = await import("@/cli/cmd/link") +const { createAndBindInline } = await import("@/plugin/tui/altimate/workspace") + +const ORIGINAL_FETCH = globalThis.fetch + +interface Call { + method: string + path: string + body: Record +} +let calls: Call[] = [] +let routes: Array<{ match: RegExp; method?: string; status: number; body: unknown }> = [] + +function stubFetch() { + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = new URL(String(input)) + const method = String(init?.method ?? "GET") + calls.push({ + method, + path: url.pathname, + body: init?.body ? JSON.parse(String(init.body)) : {}, + }) + const route = routes.find( + (r) => r.match.test(url.pathname) && (r.method === undefined || r.method === method), + ) + const { status, body } = route ?? { status: 200, body: {} } + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }) + }) as typeof globalThis.fetch +} + +/** Endpoint sequence, ignoring the best-effort memory/skill traffic that + * `recordApprovedBinding` kicks off — this is about which create ran. */ +const sequence = () => + calls + .filter((c) => c.path.includes("/datamates") || c.path.includes("/datamate-project-bindings")) + .map((c) => `${c.method} ${c.path}`) + +const BINDING = { + id: 1, + datamate_id: 7, + datamate_name: "proj", + repo_remote: "https://github.com/acme/proj", + project_path: null, +} + +const IDENTIFIER = { repoRemote: "https://github.com/acme/proj", projectPath: "/tmp/proj" } +const EXISTING = { + datamate: { id: 3, name: "old-workspace" }, + matchedBy: "remote" as const, + binding: { ...BINDING, id: 9, datamate_id: 3, datamate_name: "old-workspace" }, +} + +beforeEach(() => { + calls = [] + routes = [] + stubFetch() +}) +afterEach(() => { + globalThis.fetch = ORIGINAL_FETCH + process.exitCode = undefined +}) +afterAll(() => { + if (ORIGINAL_TEST_HOME === undefined) delete process.env.OPENCODE_TEST_HOME + else process.env.OPENCODE_TEST_HOME = ORIGINAL_TEST_HOME + if (ORIGINAL_XDG_STATE_HOME === undefined) delete process.env.XDG_STATE_HOME + else process.env.XDG_STATE_HOME = ORIGINAL_XDG_STATE_HOME +}) + +describe("CLI: createThenBindOrRebind", () => { + test("unlinked project uses the atomic create-and-bind, and never rebinds", async () => { + routes = [ + { + match: /datamate-project-bindings\/$/, + method: "POST", + status: 200, + body: { datamate: { id: 7, name: "proj" }, binding: BINDING, manage_url: "https://x.test/w/7" }, + }, + ] + await createThenBindOrRebind(IDENTIFIER, "proj", "/tmp/proj", null) + + expect(sequence()).toEqual(["POST /datamate-project-bindings/"]) + expect(process.exitCode ?? 0).toBe(0) + }) + + test("already-linked project creates UNBOUND, then rebinds", async () => { + routes = [ + { match: /\/datamates\/$/, method: "POST", status: 200, body: { id: 7 } }, + { match: /by-remote/, method: "PUT", status: 200, body: { binding: BINDING } }, + ] + await createThenBindOrRebind(IDENTIFIER, "proj", "/tmp/proj", EXISTING) + + // The whole fix: /datamates/ (no identifiers) first, then the rebind. + // Before AI-9171 this was a single POST to the bindings router that 409'd. + expect(sequence()).toEqual(["POST /datamates/", "PUT /datamate-project-bindings/by-remote"]) + const create = calls.find((c) => c.path.endsWith("/datamates/"))! + expect(create.body).not.toHaveProperty("repo_remote") + expect(create.body).not.toHaveProperty("project_path") + expect(create.body.memory_enabled).toBe(true) + expect(create.body.knowledge_engine_enabled).toBe(true) + expect(process.exitCode ?? 0).toBe(0) + }) + + test("a failed rebind reports the orphan rather than claiming success", async () => { + routes = [ + { match: /\/datamates\/$/, method: "POST", status: 200, body: { id: 7 } }, + { match: /by-remote/, method: "PUT", status: 500, body: { detail: "boom" } }, + ] + await createThenBindOrRebind(IDENTIFIER, "proj", "/tmp/proj", EXISTING) + + // Created, not linked: the caller must exit non-zero so a script does not + // treat this as a successful link. + expect(sequence()).toEqual(["POST /datamates/", "PUT /datamate-project-bindings/by-remote"]) + expect(process.exitCode).toBe(1) + }) + + test("a 409 on the unbound create does not run the rebind", async () => { + routes = [{ match: /\/datamates\/$/, method: "POST", status: 409, body: { detail: "nope" } }] + await createThenBindOrRebind(IDENTIFIER, "proj", "/tmp/proj", EXISTING) + + expect(sequence()).toEqual(["POST /datamates/"]) + expect(process.exitCode).toBe(1) + }) +}) + +describe("TUI: createAndBindInline", () => { + const stubApi = () => { + const toasts: Array<{ variant?: string; message: string }> = [] + return { + toasts, + api: { + state: { path: { directory: "/tmp/proj" } }, + ui: { + toast: (t: { variant?: string; message: string }) => toasts.push(t), + dialog: { clear: () => {}, replace: () => {} }, + }, + } as never, + } + } + + test("already-linked project creates UNBOUND, then rebinds", async () => { + routes = [ + { match: /\/datamates\/$/, method: "POST", status: 200, body: { id: 7 } }, + { match: /by-remote/, method: "PUT", status: 200, body: { binding: BINDING } }, + ] + const { api } = stubApi() + await createAndBindInline(api, IDENTIFIER, "proj", { + expectedCurrentDatamateId: 3, + matchedBy: "remote", + }) + + // The Major issue on PR #1314: this surface kept calling the atomic + // create-and-bind, which 409s first, so the rebind below it never ran. + expect(sequence()).toEqual(["POST /datamates/", "PUT /datamate-project-bindings/by-remote"]) + }) + + test("unlinked project still uses the atomic create-and-bind", async () => { + routes = [ + { + match: /datamate-project-bindings\/$/, + method: "POST", + status: 200, + body: { datamate: { id: 7, name: "proj" }, binding: BINDING, manage_url: "https://x.test/w/7" }, + }, + ] + const { api } = stubApi() + await createAndBindInline(api, IDENTIFIER, "proj") + + expect(sequence()).toEqual(["POST /datamate-project-bindings/"]) + }) + + test("a failed rebind toasts the orphan instead of reporting success", async () => { + routes = [ + { match: /\/datamates\/$/, method: "POST", status: 200, body: { id: 7 } }, + { match: /by-remote/, method: "PUT", status: 500, body: { detail: "boom" } }, + ] + const { api, toasts } = stubApi() + await createAndBindInline(api, IDENTIFIER, "proj", { + expectedCurrentDatamateId: 3, + matchedBy: "remote", + }) + + expect(toasts.some((t) => t.variant === "error" && /CREATED but could not be linked/.test(t.message))).toBe(true) + }) +}) 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 0000000000..5bb5a8b366 --- /dev/null +++ b/packages/opencode/test/altimate/workspace/create-workspace-unbound.test.ts @@ -0,0 +1,192 @@ +// 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" + +// Set at module scope because the module under test resolves `Global.Path` at +// import time — moving this into `beforeEach` would be too late. The sandbox is +// keyed by pid and clock so parallel files cannot share it, the original value +// is restored in `afterAll`, and `globalThis.fetch` is restored after every +// test rather than left installed for whatever loads next. +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, WorkspaceApiError } = 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/) + }) + + // `Number()` coerces, so a guard written as `Number.isSafeInteger(Number(id))` + // accepts all three of these: `true` becomes 1, `"7"` becomes 7, `[5]` + // becomes 5. The first is the dangerous one — a malformed body would have + // rebound the project to workspace 1 rather than failing. The type check has + // to come before the arithmetic. + test.each([ + ["a boolean", true], + ["a numeric string", "7"], + ["a single-element array", [5]], + ["a float", 1.5], + ["zero", 0], + ["a negative", -1], + ])("rejects %s rather than coercing it", async (_label, value) => { + respondWith(200, { id: value }) + await expect(WorkspaceApi.createWorkspaceUnbound({ name: "x" })).rejects.toThrow(/no usable id/) + }) + + test("throws a typed WorkspaceApiError, not a bare Error", async () => { + // Every other failure in this module is typed; callers should be able to + // tell this apart programmatically. (review, PR #1314) + respondWith(200, { id: null }) + const err = await WorkspaceApi.createWorkspaceUnbound({ name: "x" }).catch((e: unknown) => e) + expect(err).toBeInstanceOf(WorkspaceApiError) + }) +}) + +describe("account fingerprint", () => { + test("reports the account the next call will act as", async () => { + const fp = await WorkspaceApi.accountFingerprint() + expect(fp).toEqual({ apiUrl: API_URL, tenant: TENANT }) + }) + + test("sameAccount is true for the account in effect", async () => { + expect(await WorkspaceApi.sameAccount({ apiUrl: API_URL, tenant: TENANT })).toBe(true) + }) + + test("sameAccount is false once the tenant or url differs", async () => { + // What the create-then-rebind pair guards against: a workspace id is local + // to the account that made it, so rebinding under another would point the + // project at whatever id collides there. + expect(await WorkspaceApi.sameAccount({ apiUrl: API_URL, tenant: "other" })).toBe(false) + expect(await WorkspaceApi.sameAccount({ apiUrl: "https://other.test", tenant: TENANT })).toBe(false) + }) +})