diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 75522a6b5..92a925c13 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -159,6 +159,14 @@ export class WorkspaceApiError extends Error { } } +/** A credential captured once and passed to every request of one flow, so the flow cannot + * drift onto another account part-way: workspace and user ids are per tenant. */ +export interface ActAs { + url: string + instance: string + apiKey: string +} + async function creds(): Promise<{ url: string; instance: string; apiKey: string }> { if (!(await AltimateApi.isConfigured())) throw new NotConfiguredError() const c = await AltimateApi.getCredentials() @@ -521,9 +529,15 @@ export namespace WorkspaceApi { return { id, name: input.name } } + /** The ambient credential as an `ActAs`, or null when none is configured. */ + export async function captureCredentials(): Promise { + return creds().catch(() => null) + } + export async function bindExisting( datamateId: number, identifier: ProjectIdentifier, + actAs?: ActAs, ): Promise { return req("POST", "/bind", { body: { @@ -531,6 +545,7 @@ export namespace WorkspaceApi { repo_remote: identifier.repoRemote ?? null, project_path: identifier.projectPath ?? null, }, + ...(actAs ? { actAs } : {}), }) } @@ -538,6 +553,7 @@ export namespace WorkspaceApi { remote: string targetDatamateId: number expectedCurrentDatamateId?: number + actAs?: ActAs }): Promise { return req("PUT", "/by-remote", { body: { @@ -547,6 +563,7 @@ export namespace WorkspaceApi { ? { expected_current_datamate_id: input.expectedCurrentDatamateId } : {}), }, + ...(input.actAs ? { actAs: input.actAs } : {}), }) } @@ -556,6 +573,7 @@ export namespace WorkspaceApi { projectPath: string targetDatamateId: number expectedCurrentDatamateId?: number + actAs?: ActAs }): Promise { return req("PUT", "/by-path", { body: { @@ -565,6 +583,7 @@ export namespace WorkspaceApi { ? { expected_current_datamate_id: input.expectedCurrentDatamateId } : {}), }, + ...(input.actAs ? { actAs: input.actAs } : {}), }) } @@ -627,8 +646,8 @@ export namespace WorkspaceApi { /** The caller's own user id, from ``GET /users/me``. Needed wherever the * client must compare ownership — skill attachment requires the caller to * OWN the workspace, and the credentials carry no user id of their own. */ - export async function whoami(): Promise { - const me = await req<{ id?: unknown }>("GET", "/me", { base: "/users" }) + export async function whoami(actAs?: ActAs): Promise { + const me = await req<{ id?: unknown }>("GET", "/me", { base: "/users", ...(actAs ? { actAs } : {}) }) const id = Number(me?.id) if (!Number.isInteger(id) || id <= 0) throw new WorkspaceApiError("The server did not say who this account is.") return id diff --git a/packages/opencode/src/altimate/workspace/workspace-name.ts b/packages/opencode/src/altimate/workspace/workspace-name.ts index f201e6fa1..60c56ebac 100644 --- a/packages/opencode/src/altimate/workspace/workspace-name.ts +++ b/packages/opencode/src/altimate/workspace/workspace-name.ts @@ -2,7 +2,7 @@ // // The one piece of workspace text handling that BOTH realms need: the session // code renders the name into the system prompt, and the TUI plugin renders it -// into a dialog header. Kept free of imports and module state on purpose — +// into a dialog header. Kept free of imports and mutable module state on purpose — // `precedence.ts`, where this lived, is server-side only (see its header), and // a plugin importing it would load a second copy of that module's state into // the plugin realm. @@ -15,10 +15,7 @@ * what the model reads. */ export const MAX_WORKSPACE_NAME_CHARS = 80 export function inertWorkspaceName(name: string): string { - const cleaned = name - .replace(/[\u0000-\u001F\u007F-\u009F\u2028\u2029]+/g, " ") - .replace(/\s+/g, " ") - .trim() + const cleaned = oneLine(name) const points = Array.from(cleaned) return points.length > MAX_WORKSPACE_NAME_CHARS ? points.slice(0, MAX_WORKSPACE_NAME_CHARS - 1).join("") + "…" : cleaned } @@ -47,3 +44,91 @@ export function workspaceLabel(name: string, id: string | undefined, budget = MA // so it is what survives. return label.length > budget ? suffix.trim() : label } + +/** A name on one line: control characters (C0, DEL and C1) and the Unicode line and + * paragraph separators become spaces, runs of whitespace collapse, ends are trimmed. + * The one normalisation both the rendered and the compared name start from. */ +function oneLine(name: string): string { + return name + .replace(/[\u0000-\u001F\u007F-\u009F\u2028\u2029]+/g, " ") + .replace(/\s+/g, " ") + .trim() +} + +/** A name as the link pickers compare it: on one line, and the sharp S spelled out, + * which collation otherwise keeps apart from "ss". */ +function comparableName(name: string): string { + return oneLine(name).replace(/[ßẞ]/g, "ss") +} + +/** The bidi controls (ALM, LRM/RLM, LRE..RLO, LRI..PDI): harmless in a link, but they + * can visually reverse or reorder a displayed name. */ +const BIDI_CONTROLS = /[\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]/g +export function stripBidiControls(text: string): string { + return text.replace(BIDI_CONTROLS, "") +} + +/** A workspace name as a dialog shows it: inert, and with no bidi controls. */ +export function displayWorkspaceName(name: string): string { + return stripBidiControls(inertWorkspaceName(name)) +} + +/** One collator for every comparison, pinned to `en`: an unpinned one follows the host + * locale, and under Turkish `I` no longer pairs with `i`, under Danish `aa` equals `å`. */ +export const NAME_COLLATOR = new Intl.Collator("en", { sensitivity: "accent", usage: "search" }) + +export interface Namesakes { + /** Every listed workspace with the name a quick create would use, in list order. */ + all: T[] + /** The first of them the caller owns, or undefined. The only one a picker opens on: a plain + * Enter on a namesake links to it and sends this machine's memory there, which must not + * happen to a colleague's workspace by accident. */ + own: T | undefined +} + +/** The listed workspaces already named what a quick create would call this project. + * + * Names compare by Unicode collation, ignoring case but not accents: `Straße` matches + * `STRASSE`, `ΟΔΟΣ` matches `οδος` (final sigma) and `finance` matches `FINANCE` (ligature), + * all through collation, while `ı` and `i` or `café` and `cafe` stay different. + * Characters collation ignores, such as zero-width and bidi controls, do not make a name + * different. Ownership counts only when both the owner and the caller are known. */ +export function findNamesakes( + list: readonly T[], + proposedName: string, + userId: number | undefined, +): Namesakes { + const target = comparableName(proposedName) + if (!target) return { all: [], own: undefined } + const all = list.filter((workspace) => NAME_COLLATOR.compare(comparableName(workspace.name), target) === 0) + const own = userId === undefined ? undefined : all.find((workspace) => workspace.ownerId === userId) + return { all, own } +} + +/** The hint a picker shows on a namesake's row, or undefined for any other row. */ +export function namesakeHint( + workspace: T, + namesakes: Namesakes, + userId: number | undefined, +): string | undefined { + if (!namesakes.all.includes(workspace)) return undefined + const someoneElses = userId !== undefined && workspace.ownerId !== undefined && workspace.ownerId !== userId + return someoneElses ? "same name, owned by someone else" : "same name as this project" +} + +/** Where a link picker opens: the current link, else the caller's own namesake, else create. */ +export function linkPickerOpensOn( + currentId: number | undefined, + namesakes: Namesakes, +): number | "create" { + return currentId ?? namesakes.own?.id ?? "create" +} + +/** Whether a choice in a link picker or the setup dialog must confirm first: both create + * paths start from the project's name, so either one would make a second namesake. */ +export function confirmsNamesake( + choice: "create" | "browser" | "workspace", + namesakes: Namesakes, +): boolean { + return choice !== "workspace" && namesakes.all.length > 0 +} diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 6f10edf08..587990a37 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -25,6 +25,7 @@ import { NotConfiguredError, NotFoundError, PreconditionFailedError, + type ActAs, type Binding, type DatamateRef, type MatchedIdentifier, @@ -42,8 +43,16 @@ import { resolveWorkspaceWebUrl, type HandoffResult, } from "@/altimate/workspace/browser-handoff" -import { accountDigest, recordApprovedBinding } from "@/altimate/workspace/state" +import { accountDigest, credentialDigest, recordApprovedBinding } from "@/altimate/workspace/state" import type { SeedOutcome } from "@/altimate/workspace/memory-backfill" +import { + confirmsNamesake, + displayWorkspaceName, + findNamesakes, + linkPickerOpensOn, + namesakeHint, + stripBidiControls, +} from "@/altimate/workspace/workspace-name" const CREATE_NEW_SENTINEL = "__create_new__" const SET_UP_IN_BROWSER_SENTINEL = "__browser_handoff__" @@ -60,11 +69,18 @@ export function stripControlChars(text: string): string { // covered C0/DEL, leaving C1 controls unstripped. ESC (the OSC 8 breakout // vector) was always covered, but the doc comment claimed C1 coverage it // didn't have. (Kilo, PR #1274.) - // Plus the Unicode bidi controls (LRM/RLM, LRE..RLO, LRI..PDI): they cannot break out of the + // Plus the Unicode bidi controls (ALM, LRM/RLM, LRE..RLO, LRI..PDI): they cannot break out of the // hyperlink (the href is always `buildManageUrl`, never the name) but can visually reverse or // reorder the displayed name in the picker. (v0.11.2 release review.) // eslint-disable-next-line no-control-regex - return text.replace(/[\x00-\x1f\x7f-\x9f\u200e\u200f\u202a-\u202e\u2066-\u2069]/g, "") + return stripBidiControls(text.replace(/[\x00-\x1f\x7f-\x9f]/g, "")) +} + +/** What a pick in the link picker does: one of the two create paths, or an existing workspace. */ +export function linkPickKind(pick: string): "create" | "browser" | "workspace" { + if (pick === CREATE_NEW_SENTINEL) return "create" + if (pick === SET_UP_IN_BROWSER_SENTINEL) return "browser" + return "workspace" } /** Sanitized display name for a ``ConflictError``'s existing-binding name, @@ -229,11 +245,19 @@ export const LinkCommand = cmd({ ) } + // One credential for the list, the owner lookup and the bind: workspace and user ids are + // per tenant, so a switch while the picker is open must not mix two accounts. + const actAs = await WorkspaceApi.captureCredentials() + if (!actAs) { + prompts.log.error("Could not read your Altimate credentials. Check /connect and try again.") + process.exitCode = 1 + return + } const spin = prompts.spinner() spin.start("Loading workspaces...") let list: DatamateRef[] try { - list = await WorkspaceApi.listDatamates() + list = await WorkspaceApi.listDatamates(actAs) } catch (err) { spin.stop("Could not load workspaces.", 1) prompts.log.error(err instanceof Error ? err.message : String(err)) @@ -246,6 +270,11 @@ export const LinkCommand = cmd({ ? projectNameFromRemote(identifier.repoRemote) : projectNameFromPath(identifier.projectPath) const currentId = existing?.datamate.id + // Listed workspaces already named what a quick create would use. The picker opens on + // the caller's own (never a colleague's), and creating another namesake is confirmed. + // Without a user id nothing is preselected; the confirmation still applies. + const userId = await WorkspaceApi.whoami(actAs).catch(() => undefined) + const namesakes = findNamesakes(list, autoName, userId) // Sanitized once here so every downstream display (the picker message, // the "Kept" outro, hyperlink()'s own text) is covered — hyperlink() // only sanitized its own `text` param, not the raw name reaching @@ -313,7 +342,7 @@ export const LinkCommand = cmd({ return { value: String(dm.id), label: dm.id === currentId ? `● ${hyperlink(safeDmName, currentManageUrl)}` : ` ${safeDmName}`, - hint: dm.id === currentId ? "currently linked here" : undefined, + hint: dm.id === currentId ? "currently linked here" : namesakeHint(dm, namesakes, userId), } }), ] @@ -323,7 +352,9 @@ export const LinkCommand = cmd({ ? `Currently linked to "${hyperlink(currentName!, currentManageUrl)}". Pick a workspace (or create a new one):` : "Pick a workspace to link (or create a new one):", options, - initialValue: currentId !== undefined ? String(currentId) : CREATE_NEW_SENTINEL, + initialValue: ((at) => (at === "create" ? CREATE_NEW_SENTINEL : String(at)))( + linkPickerOpensOn(currentId, namesakes), + ), }) if (prompts.isCancel(pick)) { @@ -331,6 +362,19 @@ export const LinkCommand = cmd({ return } + // Both create paths start from the project's name, so both confirm a namesake. + const twin = namesakes.own ?? namesakes.all[0] + if (twin && confirmsNamesake(linkPickKind(pick), namesakes)) { + const again = await prompts.confirm({ + message: `A workspace named "${stripControlChars(displayWorkspaceName(twin.name))}" already exists. Create another one with the same name?`, + initialValue: false, + }) + if (prompts.isCancel(again) || !again) { + prompts.outro("No changes.") + return + } + } + if (pick === SET_UP_IN_BROWSER_SENTINEL) { await runBrowserHandoff(identifier, autoName, args.directory) return @@ -347,7 +391,7 @@ export const LinkCommand = cmd({ return } - await bindOrRebind(identifier, targetId, existing, preCheckOk, args.directory) + await bindOrRebind(identifier, targetId, existing, preCheckOk, args.directory, actAs) }, }) @@ -678,12 +722,13 @@ async function bindOrRebind( existing: ProjectBindingLookup | null, preCheckOk: boolean, directory: string, + /** The credential the picker's list was read as. The bind runs as it, and the seed refuses + * (account-changed) if the configured account switches mid-way. */ + actAs: ActAs, ): Promise { - // The account this bind acts as; the seed refuses (account-changed) if it switches mid-way. - // Unreadable credentials cannot link anyway, and must not leave the bind unguarded. - const linkAccount = await accountDigest() - if (linkAccount === null) { - prompts.log.error("Could not read your Altimate credentials, so nothing was linked. Check /connect and try again.") + const linkAccount = credentialDigest(actAs.url, actAs.instance, actAs.apiKey) + if ((await accountDigest()) !== linkAccount) { + prompts.log.error("Your Altimate account changed since the list was loaded, so nothing was linked. Re-run `altimate-code link`.") process.exitCode = 1 return } @@ -698,13 +743,14 @@ async function bindOrRebind( targetDatamateId, expectedCurrentDatamateId: existing.datamate.id, matchedBy: existing.matchedBy, + actAs, }) } else { // No known binding OR pre-check failed. Try bindExisting first — if the // pre-check missed a real binding, the server will 409, and we retry as // rebind when we're allowed to. (m10) try { - res = await WorkspaceApi.bindExisting(targetDatamateId, identifier) + res = await WorkspaceApi.bindExisting(targetDatamateId, identifier, actAs) } catch (err) { // A teammate's private workspace is not a pre-check race: rebinding it only fails // again (forbidden), and would hide the explanation the outer handler gives. @@ -733,11 +779,13 @@ async function bindOrRebind( res = await WorkspaceApi.rebindByPath({ projectPath: conflictPath, targetDatamateId, + actAs, }) } else if (conflictRemote) { res = await WorkspaceApi.rebindByRemote({ remote: conflictRemote, targetDatamateId, + actAs, }) } else { // Server didn't tell us which identifier owned the conflict — @@ -747,10 +795,12 @@ async function bindOrRebind( ? await WorkspaceApi.rebindByRemote({ remote: identifier.repoRemote, targetDatamateId, + actAs, }) : await WorkspaceApi.rebindByPath({ projectPath: identifier.projectPath!, targetDatamateId, + actAs, }) } rebindSpin.stop(`Re-linked to "${stripControlChars(res.binding.datamate_name)}".`) @@ -834,12 +884,14 @@ async function rebindByMatchedIdentifier(input: { targetDatamateId: number expectedCurrentDatamateId: number matchedBy: MatchedIdentifier + actAs?: ActAs }) { if (input.matchedBy === "remote" && input.identifier.repoRemote) { return WorkspaceApi.rebindByRemote({ remote: input.identifier.repoRemote, targetDatamateId: input.targetDatamateId, expectedCurrentDatamateId: input.expectedCurrentDatamateId, + actAs: input.actAs, }) } if (input.matchedBy === "path" && input.identifier.projectPath) { @@ -847,6 +899,7 @@ async function rebindByMatchedIdentifier(input: { projectPath: input.identifier.projectPath, targetDatamateId: input.targetDatamateId, expectedCurrentDatamateId: input.expectedCurrentDatamateId, + actAs: input.actAs, }) } throw new Error( diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 2bcc026cb..abdd13089 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -29,9 +29,17 @@ import open from "open" // altimate_change start - the /workspace action menu import * as Manage from "@/altimate/workspace/manage" import { describeSyncProblems } from "@/altimate/workspace/skill-sync" -import { inertWorkspaceName } from "@/altimate/workspace/workspace-name" +import { + confirmsNamesake, + displayWorkspaceName, + findNamesakes, + inertWorkspaceName, + linkPickerOpensOn, + namesakeHint, + type Namesakes, +} from "@/altimate/workspace/workspace-name" // altimate_change end -import { createSignal, onCleanup, onMount } from "solid-js" +import { createMemo, createSignal, onCleanup, onMount } from "solid-js" import { ConflictError, HIDDEN_BINDING_MESSAGE, @@ -42,6 +50,7 @@ import { PreconditionFailedError, WorkspaceApi, type Binding, + type ActAs, type DatamateRef, type MatchedIdentifier, type ProjectBindingLookup, @@ -60,6 +69,7 @@ import { } from "@/altimate/workspace/detect" import { accountDigest, + credentialDigest, readLocalBinding, recordApprovedBinding, resolvePinnedBindingForRouting, @@ -194,11 +204,75 @@ interface OfferProps { * mid-render await. Null when creds are unavailable — latch falls back * to unscoped. (cubic round 3.) */ latchScope: LatchScope | null + /** Listed workspaces already named `defaultName`, found by the caller. Either create + * action confirms first when there is one, and the caller's own is offered as the + * default. Empty when the list could not be read. */ + namesakes: Namesakes + /** The credential the namesakes were listed under. Workspace ids are per tenant, so the + * namesake is linked as this credential, and only while it is still the configured one. */ + listedAs: ActAs | null +} + +/** Asked before a create that would make a second workspace with this name. A select + * rather than DialogConfirm, which opens on Confirm: a stray Enter must not create the + * duplicate, so the dialog opens on No. */ +function NamesakeConfirmDialog(props: { + api: TuiPluginApi + existingName: string + proposedName: string + onCreate: () => void +}) { + return ( + + title={`A workspace named "${displayWorkspaceName(props.existingName)}" already exists`} + options={[ + { title: "No, don't create it", value: "no", description: "Nothing changes." }, + { + title: `Yes, create another "${displayWorkspaceName(props.proposedName)}"`, + value: "yes", + description: "Two workspaces will share this name.", + }, + ]} + current="no" + onSelect={(choice) => { + if (choice.value === "yes") props.onCreate() + else props.api.ui.dialog.clear() + }} + /> + ) +} + +/** Runs `create` at once, or after the namesake confirmation when the name is taken. */ +function createUnlessNamesake( + api: TuiPluginApi, + choice: "create" | "browser", + namesakes: Namesakes, + proposedName: string, + create: () => void, +) { + const twin = namesakes.own ?? namesakes.all[0] + if (!twin || !confirmsNamesake(choice, namesakes)) { + create() + return + } + api.ui.dialog.replace(() => ( + + )) } function OfferDialog(props: OfferProps) { const identLabel = () => props.identifier.repoRemote ?? props.identifier.projectPath ?? "this project" + const own = props.namesakes.own const options = [ + ...(own + ? [ + { + title: `Link to "${displayWorkspaceName(own.name)}"`, + value: "namesake", + description: "Your workspace with this project's name.", + }, + ] + : []), ...(props.browserAvailable ? [ { @@ -224,7 +298,7 @@ function OfferDialog(props: OfferProps) { description: "Won't ask again for 7 days.", }, ] - const defaultValue = props.browserAvailable ? "browser" : "create" + const defaultValue = own ? "namesake" : props.browserAvailable ? "browser" : "create" return ( { + void runBrowserHandoff(props.api, props.identifier, props.defaultName) + }) return } if (option.value === "create") { // Local direct-create — the CLI-only fallback. The SaaS UI is the // place to rename / configure; this branch establishes the binding // without a browser round-trip. - void createAndBindInline(props.api, props.identifier, props.defaultName) + createUnlessNamesake(props.api, "create", props.namesakes, props.defaultName, () => { + void createAndBindInline(props.api, props.identifier, props.defaultName) + }) return } // link → picker (fresh-project attach path) @@ -689,12 +771,14 @@ async function rebindByMatchedIdentifier(input: { targetDatamateId: number expectedCurrentDatamateId: number matchedBy: MatchedIdentifier + actAs?: ActAs }) { if (input.matchedBy === "remote" && input.identifier.repoRemote) { return WorkspaceApi.rebindByRemote({ remote: input.identifier.repoRemote, targetDatamateId: input.targetDatamateId, expectedCurrentDatamateId: input.expectedCurrentDatamateId, + actAs: input.actAs, }) } if (input.matchedBy === "path" && input.identifier.projectPath) { @@ -702,6 +786,7 @@ async function rebindByMatchedIdentifier(input: { projectPath: input.identifier.projectPath, targetDatamateId: input.targetDatamateId, expectedCurrentDatamateId: input.expectedCurrentDatamateId, + actAs: input.actAs, }) } throw new Error( @@ -1000,10 +1085,20 @@ interface OnDemandPickerProps { * be here). Auto-names any new workspace from the git repo. */ function OnDemandPickerDialog(props: OnDemandPickerProps) { const [datamates, setDatamates] = createSignal(null) + const [userId, setUserId] = createSignal() + // The credential the list and the owner lookup are read as; a pick binds as it, and only + // while it is still the configured one. + let listedAs: ActAs | null = null onMount(async () => { try { - const list = await WorkspaceApi.listDatamates() + listedAs = await WorkspaceApi.captureCredentials() + // Without a user id nothing is preselected; the create confirmation still applies. + const [list, me] = await Promise.all([ + WorkspaceApi.listDatamates(listedAs ?? undefined), + WorkspaceApi.whoami(listedAs ?? undefined).catch(() => undefined), + ]) + setUserId(me) setDatamates(list) } catch (err) { props.api.ui.toast({ @@ -1014,6 +1109,15 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { } }) + // Listed workspaces already named what a quick create would use: the picker opens on the + // caller's own, and creating another namesake is confirmed first. Computed once per list; + // each row reads the cached value. + const namesakes = createMemo(() => findNamesakes(datamates() ?? [], props.defaultName, userId())) + const opensOn = () => { + const at = linkPickerOpensOn(props.currentlyLinkedDatamateId, namesakes()) + return at === "create" ? CREATE_NEW_SENTINEL : at + } + const options = () => { const list = datamates() // No ``disabled: true`` — DialogSelect filters those out (Kilo cycle 6). @@ -1030,7 +1134,15 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { ...list.map((dm) => ({ title: dm.id === props.currentlyLinkedDatamateId ? `● ${dm.name}` : ` ${dm.name}`, value: dm.id, - description: dm.id === props.currentlyLinkedDatamateId ? "currently linked to this project" : undefined, + description: + dm.id === props.currentlyLinkedDatamateId + ? "currently linked to this project" + : namesakeHint(dm, namesakes(), userId()), + // The cursor opens on the caller's namesake, but DialogSelect marks the `current` row + // with ●, which this picker uses for "linked here". A blank gutter keeps it unmarked. + ...(props.currentlyLinkedDatamateId === undefined && dm === namesakes().own + ? { gutter: () => } + : {}), })), ] } @@ -1039,7 +1151,7 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { title="Link this project to a workspace" options={options()} - current={props.currentlyLinkedDatamateId ?? CREATE_NEW_SENTINEL} + current={opensOn()} onSelect={(option) => { if (option.value === -1) { props.api.ui.dialog.clear() @@ -1057,7 +1169,9 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { matchedBy: props.matchedBy, } : undefined - void createAndBindInline(props.api, props.identifier, props.defaultName, rebindFrom) + createUnlessNamesake(props.api, "create", namesakes(), props.defaultName, () => { + void createAndBindInline(props.api, props.identifier, props.defaultName, rebindFrom) + }) return } // Picked an existing workspace. @@ -1074,7 +1188,7 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { props.currentlyLinkedDatamateId !== undefined && props.matchedBy ? { datamateId: props.currentlyLinkedDatamateId, matchedBy: props.matchedBy } : undefined - void bindOrRebindInline(props.api, props.identifier, option.value, existing) + void bindOrRebindInline(props.api, props.identifier, option.value, existing, listedAs) }} /> ) @@ -1088,9 +1202,22 @@ export async function bindOrRebindInline( * we call bindExisting; present means "linked" and we rebind via the * matched-identifier endpoint (M3). */ existing: { datamateId: number; matchedBy: MatchedIdentifier } | undefined, + /** The credential the target id was listed under, when the caller has one. Workspace ids + * are per tenant: the bind runs as this credential, a switch since the list loaded links + * nothing, and the record and memory seed are pinned to it. */ + listedAs?: ActAs | null, ): Promise { api.ui.dialog.clear() const isRebind = existing !== undefined + const listedAccount = listedAs ? credentialDigest(listedAs.url, listedAs.instance, listedAs.apiKey) : undefined + if (listedAs !== undefined && (listedAccount === undefined || (await accountDigest()) !== listedAccount)) { + api.ui.toast({ + variant: "warning", + message: "Your Altimate account changed since the list was loaded, so nothing was linked. Open the picker again.", + duration: 8_000, + }) + return + } try { const res = await (async () => { if (existing) { @@ -1099,17 +1226,31 @@ export async function bindOrRebindInline( targetDatamateId, expectedCurrentDatamateId: existing.datamateId, matchedBy: existing.matchedBy, + actAs: listedAs ?? undefined, }) } - return WorkspaceApi.bindExisting(targetDatamateId, identifier) + return WorkspaceApi.bindExisting(targetDatamateId, identifier, listedAs ?? undefined) })() - await recordApprovedBinding(api.state.path.directory, { - datamateId: res.binding.datamate_id, - datamateName: res.binding.datamate_name, - repoRemote: res.binding.repo_remote, - projectPath: res.binding.project_path, - linkedAt: Date.now(), - }) + const recorded = await recordApprovedBinding( + api.state.path.directory, + { + datamateId: res.binding.datamate_id, + datamateName: res.binding.datamate_name, + repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, + linkedAt: Date.now(), + }, + listedAccount ? { account: listedAccount } : undefined, + ) + if (recorded?.status === "account-changed") { + // The bind ran as the listed account; only the local record and seed were refused. + api.ui.toast({ + variant: "warning", + message: "Linked, but your Altimate account changed during the link, so saved memory was not sent.", + duration: 8_000, + }) + return + } await showLinkedConfirmation( api, isRebind ? "Re-linked" : "Linked", @@ -1286,6 +1427,7 @@ async function runFlow(api: TuiPluginApi, directory: string): Promise { if (serverBinding === null) { // Server confirmed unbound → offer create-or-link. + const { namesakes, listedAs } = await namesakesFor(defaultName, flowAccount) api.ui.dialog.replace(() => ( { defaultName={defaultName} browserAvailable={browserAvailable} latchScope={latchScope} + namesakes={namesakes} + listedAs={listedAs} /> )) return @@ -1366,6 +1510,7 @@ async function runFlow(api: TuiPluginApi, directory: string): Promise { variant: "warning", message: "Could not reach the Altimate workspace service to check for an existing link.", }) + const { namesakes, listedAs } = await namesakesFor(defaultName, flowAccount) api.ui.dialog.replace(() => ( { defaultName={defaultName} browserAvailable={browserAvailable} latchScope={latchScope} + namesakes={namesakes} + listedAs={listedAs} /> )) } +/** The workspaces already named `defaultName`, for the setup dialog, read as one captured + * credential, the one the flow started with. Best effort: when the list cannot be read, or + * the account changed since the flow began, there is nothing to compare against, and the + * dialog offers create without the confirmation, as before. */ +async function namesakesFor( + defaultName: string, + flowAccount: string | null, +): Promise<{ namesakes: Namesakes; listedAs: ActAs | null }> { + const none = { namesakes: findNamesakes([] as DatamateRef[], defaultName, undefined), listedAs: null } + const actAs = await WorkspaceApi.captureCredentials() + if (!actAs || credentialDigest(actAs.url, actAs.instance, actAs.apiKey) !== flowAccount) return none + const [list, userId] = await Promise.all([ + WorkspaceApi.listDatamates(actAs).catch(() => [] as DatamateRef[]), + WorkspaceApi.whoami(actAs).catch(() => undefined), + ]) + return { namesakes: findNamesakes(list, defaultName, userId), listedAs: actAs } +} + // ───────────────────────────────────────────────────────────────────────────── // Engine install offer. The workspace engine overlay decides there is no usable engine and hands // the offer here; this file owns the interaction. Offer, never silently @@ -2139,6 +2304,9 @@ export function canInstallWith(nodeMajor: number | null, hasNpm: boolean): boole return nodeMajor !== null && nodeMajor >= MIN_NODE_MAJOR && hasNpm } +/** Test seam: the link dialogs, rendered against a stand-in `api.ui.DialogSelect`. */ +export const linkDialogInternals = { OfferDialog, OnDemandPickerDialog } + /** Test seam for the raise path's process-wide state. */ export const engineOfferInternals = { get visible() { diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index 121e8f09e..93c432e39 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -37,6 +37,7 @@ const { canInstallWith, engineOfferInternals, showEngineInstallOffer, + linkDialogInternals, } = await import( "../../../src/plugin/tui/altimate/workspace" ) @@ -52,6 +53,9 @@ const { syncInternals } = await import("../../../src/altimate/workspace/engine-s // and recordApprovedBinding for tenant/apiUrl scoping. Re-import allows // per-test override of the module state. import { AltimateApi } from "../../../src/altimate/api/client" +import { WorkspaceApi } from "../../../src/altimate/workspace/api-client" +import { findNamesakes } from "../../../src/altimate/workspace/workspace-name" +import { createRoot } from "solid-js" const originalIsConfigured = AltimateApi.isConfigured const originalGetCreds = AltimateApi.getCredentials type Creds = Awaited> @@ -833,3 +837,218 @@ describe("engine install offer — kv hydration", () => { expect(Date.now() - t0).toBeGreaterThanOrEqual(50) }) }) + +// The dialogs run against a stand-in `api.ui.DialogSelect` that records its props, so each +// test can read what the dialog offers and drive `onSelect` the way an Enter would. The +// workspace calls are stubbed: a create or bind records itself and fails, which ends the flow. +describe("link dialogs: a workspace that already has this project's name", () => { + const ME = 10 + const COLLEAGUE = 20 + const identifier = { repoRemote: "git@github.com:acme/analytics.git", projectPath: "/tmp/analytics" } + const calls = { create: 0, bind: [] as number[], bindAs: [] as (string | undefined)[], listAs: [] as (string | undefined)[] } + const stubbed = ["listDatamates", "whoami", "accountFingerprint", "createAndBind", "createWorkspaceUnbound", "bindExisting"] + const original: Record = {} + const api = WorkspaceApi as unknown as Record + + function stubWorkspaces(list: { id: number; name: string; ownerId?: number }[]) { + for (const key of stubbed) original[key] = api[key] + Object.assign(api, { + listDatamates: async (actAs?: { instance: string }) => { + calls.listAs.push(actAs?.instance) + return list + }, + whoami: async () => ME, + accountFingerprint: async () => null, + createAndBind: async () => { + calls.create += 1 + throw new Error("stub: create") + }, + createWorkspaceUnbound: async () => { + calls.create += 1 + throw new Error("stub: create") + }, + bindExisting: async (id: number, _identifier: unknown, actAs?: { instance: string }) => { + calls.bind.push(id) + calls.bindAs.push(actAs?.instance) + throw new Error("stub: bind") + }, + }) + } + afterEach(() => { + for (const key of Object.keys(original)) api[key] = original[key] + calls.create = 0 + calls.bind = [] + calls.bindAs = [] + calls.listAs = [] + }) + + type Select = { options: { title: string; value: unknown; description?: string }[]; current: unknown; onSelect: (o: { value: unknown }) => void } + function harness() { + const h = { selects: [] as Select[], replaced: [] as (() => unknown)[], cleared: 0, toasts: [] as string[] } + const tui = { + state: { path: { directory: os.tmpdir() } }, + kv: { ...makeKv(), ready: true }, + ui: { + DialogSelect: (props: Select) => { + h.selects.push(props) + return null + }, + toast: (t: { message: string }) => { + h.toasts.push(t.message) + }, + dialog: { + replace: (factory: () => unknown) => { + h.replaced.push(factory) + }, + clear: () => { + h.cleared += 1 + }, + }, + }, + } as any + return { tui, h } + } + const render = (factory: () => unknown) => createRoot(() => factory()) + const settle = () => new Promise((resolve) => setTimeout(resolve, 20)) + + async function offer(list: { id: number; name: string; ownerId?: number }[], browserAvailable = true) { + stubWorkspaces(list) + stubCreds("acme", "https://api.acme.example.com") + const listedAs = await WorkspaceApi.captureCredentials() + const { tui, h } = harness() + render(() => + linkDialogInternals.OfferDialog({ + api: tui, + identifier, + defaultName: "analytics", + browserAvailable, + latchScope: null, + namesakes: findNamesakes(list, "analytics", ME), + listedAs, + }), + ) + return { tui, h, dialog: h.selects[0]! } + } + + test.each([ + ["Create quick workspace", "create"], + ["Set up in browser", "browser"], + ] as const)("setup dialog: %s on a taken name asks first, and No creates nothing", async (_label, value) => { + const { h, dialog } = await offer([{ id: 1, name: "Analytics", ownerId: COLLEAGUE }]) + dialog.onSelect({ value }) + expect(h.replaced).toHaveLength(1) + render(h.replaced[0]!) + const confirm = h.selects[1]! + expect(confirm.current).toBe("no") + confirm.onSelect({ value: "no" }) + await settle() + expect(h.cleared).toBe(1) + expect(calls.create).toBe(0) + }) + + test("setup dialog: Yes on the confirmation creates exactly once", async () => { + const { h, dialog } = await offer([{ id: 1, name: "analytics", ownerId: COLLEAGUE }]) + dialog.onSelect({ value: "create" }) + render(h.replaced[0]!) + h.selects[1]!.onSelect({ value: "yes" }) + await settle() + expect(calls.create).toBe(1) + }) + + test("setup dialog: a free name creates without asking", async () => { + const { h, dialog } = await offer([{ id: 1, name: "marketing", ownerId: ME }]) + dialog.onSelect({ value: "create" }) + await settle() + expect(h.replaced).toHaveLength(0) + expect(calls.create).toBe(1) + }) + + test("setup dialog: my own namesake is offered first, opens selected, and links on Enter", async () => { + const { dialog } = await offer([ + { id: 1, name: "analytics", ownerId: COLLEAGUE }, + { id: 2, name: "analytics", ownerId: ME }, + ]) + expect(dialog.options[0]).toMatchObject({ value: "namesake", title: 'Link to "analytics"' }) + expect(dialog.current).toBe("namesake") + dialog.onSelect({ value: dialog.current }) + await settle() + expect(calls.bind).toEqual([2]) + expect(calls.create).toBe(0) + }) + + test.each([ + ["with the browser handoff", true, "browser"], + ["without it", false, "create"], + ] as const)("setup dialog: only a colleague's namesake is not offered or preselected, %s", async (_label, browser, opensOn) => { + const { dialog } = await offer([{ id: 1, name: "analytics", ownerId: COLLEAGUE }], browser) + expect(dialog.options.map((o) => o.value)).not.toContain("namesake") + expect(dialog.current).toBe(opensOn) + }) + + async function picker(list: { id: number; name: string; ownerId?: number }[]) { + stubWorkspaces(list) + stubCreds("acme", "https://api.acme.example.com") + const { tui, h } = harness() + render(() => linkDialogInternals.OnDemandPickerDialog({ api: tui, identifier, defaultName: "analytics" })) + await settle() + return { h, dialog: h.selects[0]! } + } + + test("picker: opens on my namesake and labels a colleague's", async () => { + const { dialog } = await picker([ + { id: 1, name: "analytics", ownerId: COLLEAGUE }, + { id: 2, name: "analytics", ownerId: ME }, + ]) + expect(dialog.current).toBe(2) + expect(dialog.options.find((o) => o.value === 1)?.description).toBe("same name, owned by someone else") + expect(dialog.options.find((o) => o.value === 2)?.description).toBe("same name as this project") + }) + + test("picker: with only a colleague's namesake it opens on create, which asks first", async () => { + const { h, dialog } = await picker([{ id: 1, name: "analytics", ownerId: COLLEAGUE }]) + const create = dialog.options.find((o) => o.value !== 1 && typeof o.value === "number" && o.value < -1) + expect(dialog.current).toBe(create?.value) + dialog.onSelect({ value: dialog.current }) + expect(h.replaced).toHaveLength(1) + render(h.replaced[0]!) + h.selects[1]!.onSelect({ value: "no" }) + await settle() + expect(calls.create).toBe(0) + }) + + // Workspace ids are per tenant: an id listed under one account is another workspace under + // the next, so a switch while a dialog is open must link nothing. + test("setup dialog: an account switch after the offer loads links nothing", async () => { + const { h, dialog } = await offer([{ id: 2, name: "analytics", ownerId: ME }]) + stubCreds("other-tenant", "https://api.other.example.com") + dialog.onSelect({ value: "namesake" }) + await settle() + expect(calls.bind).toEqual([]) + expect(h.toasts.some((m) => m.includes("account changed"))).toBe(true) + }) + + test("picker: an account switch after the list loads links nothing", async () => { + const { h, dialog } = await picker([{ id: 2, name: "analytics", ownerId: ME }]) + stubCreds("other-tenant", "https://api.other.example.com") + dialog.onSelect({ value: 2 }) + await settle() + expect(calls.bind).toEqual([]) + expect(h.toasts.some((m) => m.includes("account changed"))).toBe(true) + }) + + test("picker: the list and the bind run as the credential captured when it opened", async () => { + const { dialog } = await picker([{ id: 2, name: "analytics", ownerId: ME }]) + dialog.onSelect({ value: 2 }) + await settle() + expect(calls.listAs).toEqual(["acme"]) + expect(calls.bind).toEqual([2]) + expect(calls.bindAs).toEqual(["acme"]) + }) + + test("setup dialog: the namesake is bound as the credential it was listed under", async () => { + const { dialog } = await offer([{ id: 2, name: "analytics", ownerId: ME }]) + dialog.onSelect({ value: "namesake" }) + await settle() + expect(calls.bindAs).toEqual(["acme"]) + }) +}) diff --git a/packages/opencode/test/altimate/workspace/same-named-workspace.test.ts b/packages/opencode/test/altimate/workspace/same-named-workspace.test.ts new file mode 100644 index 000000000..04efbc192 --- /dev/null +++ b/packages/opencode/test/altimate/workspace/same-named-workspace.test.ts @@ -0,0 +1,122 @@ +// altimate_change - new file +// +// The link pickers open on the caller's own workspace that already has the name a quick +// create would use, never on a colleague's, and creating another workspace with that +// name takes a deliberate choice. +import { describe, expect, test } from "bun:test" +import { + NAME_COLLATOR, + confirmsNamesake, + displayWorkspaceName, + findNamesakes, + linkPickerOpensOn, + namesakeHint, +} from "../../../src/altimate/workspace/workspace-name" + +const ME = 10 +const COLLEAGUE = 20 +const ws = (id: number, name: string, ownerId?: number) => ({ id, name, ownerId }) + +describe("findNamesakes: which names match", () => { + test.each([ + ["exact name", [ws(1, "analytics")], "analytics", [1]], + ["case differs", [ws(1, "Analytics")], "analytics", [1]], + ["case differs beyond ASCII: ß and SS", [ws(1, "Straße")], "STRASSE", [1]], + ["case differs beyond ASCII: capital sharp S and SS", [ws(1, "ẞ")], "SS", [1]], + ["Greek final sigma", [ws(1, "ΟΔΟΣ")], "οδος", [1]], + ["a ligature against its letters", [ws(1, "finance")], "FINANCE", [1]], + ["dotless i stays distinct from i", [ws(1, "ı")], "i", []], + ["an accent is a different name", [ws(1, "café")], "cafe", []], + ["Danish aa is not å", [ws(1, "aa")], "å", []], + ["NFD and NFC spellings are one name", [ws(1, "café")], "café", [1]], + ["surrounding and inner whitespace differ", [ws(1, " data platform ")], "data platform", [1]], + ["a control character in the listed name", [ws(1, "analy\u0007tics")], "analy tics", [1]], + ["a control character in the proposed name", [ws(1, "analy tics")], "analy\u0007tics", [1]], + ["a zero-width space in the listed name is ignored", [ws(1, "analy​tics")], "analytics", [1]], + ["a bidi control in the listed name is ignored", [ws(1, "analy؜tics")], "analytics", [1]], + ["no workspace has the name", [ws(1, "marketing"), ws(2, "finance")], "analytics", []], + ["a longer name that only starts the same", [ws(1, "analytics-prod")], "analytics", []], + ["every namesake, in list order", [ws(3, "analytics"), ws(5, "x"), ws(4, "ANALYTICS")], "analytics", [3, 4]], + ["an empty proposed name matches nothing", [ws(1, "")], " ", []], + ["an empty list", [], "analytics", []], + ] as const)("%s", (_label, list, proposed, expected) => { + expect(findNamesakes(list, proposed, ME).all.map((w) => w.id)).toEqual([...expected]) + }) + + test("returns the listed objects themselves, so callers keep each id and display name", () => { + const listed = ws(7, "Analytics", ME) + const found = findNamesakes([ws(1, "other"), listed], "analytics", ME) + expect(found.all[0]).toBe(listed) + expect(found.own).toBe(listed) + }) + + test("the collator is pinned to en, whatever the host locale", () => { + // Under a Turkish default an unpinned collator stops pairing I with i. + expect(NAME_COLLATOR.resolvedOptions().locale).toBe("en") + }) +}) + +describe("findNamesakes: which namesake a picker may open on", () => { + test.each([ + ["mine", [ws(1, "analytics", ME)], ME, 1], + ["a colleague's", [ws(1, "analytics", COLLEAGUE)], ME, undefined], + ["a colleague's first, then mine: mine", [ws(1, "analytics", COLLEAGUE), ws(2, "analytics", ME)], ME, 2], + ["two of mine: the first", [ws(1, "analytics", ME), ws(2, "Analytics", ME)], ME, 1], + ["owner not reported by the server", [ws(1, "analytics")], ME, undefined], + ["caller unknown", [ws(1, "analytics", ME)], undefined, undefined], + ] as const)("%s", (_label, list, userId, expected) => { + expect(findNamesakes(list, "analytics", userId).own?.id).toBe(expected) + }) +}) + +describe("namesakeHint", () => { + const mine = ws(1, "analytics", ME) + const theirs = ws(2, "analytics", COLLEAGUE) + const unknownOwner = ws(3, "analytics") + const other = ws(4, "marketing", ME) + const list = [mine, theirs, unknownOwner, other] + test.each([ + ["my namesake", mine, ME, "same name as this project"], + ["a colleague's namesake", theirs, ME, "same name, owned by someone else"], + ["a namesake whose owner is not reported", unknownOwner, ME, "same name as this project"], + ["a namesake when the caller is unknown", theirs, undefined, "same name as this project"], + ["a workspace with another name", other, ME, undefined], + ] as const)("%s", (_label, row, userId, expected) => { + expect(namesakeHint(row, findNamesakes(list, "analytics", userId), userId)).toBe(expected) + }) +}) + +describe("linkPickerOpensOn", () => { + test.each([ + ["linked, with my namesake: the link", 9, [ws(1, "analytics", ME)], 9], + ["unlinked, with my namesake: the namesake", undefined, [ws(1, "analytics", ME)], 1], + ["unlinked, with only a colleague's namesake: create", undefined, [ws(1, "analytics", COLLEAGUE)], "create"], + ["unlinked, no namesake: create", undefined, [ws(1, "marketing", ME)], "create"], + ] as const)("%s", (_label, currentId, list, expected) => { + expect(linkPickerOpensOn(currentId, findNamesakes(list, "analytics", ME))).toBe(expected) + }) +}) + +describe("confirmsNamesake", () => { + const withNamesake = findNamesakes([ws(1, "analytics", COLLEAGUE)], "analytics", ME) + const without = findNamesakes([ws(1, "marketing", ME)], "analytics", ME) + test.each([ + ["create, name taken", "create", withNamesake, true], + ["browser, name taken", "browser", withNamesake, true], + ["an existing workspace, name taken", "workspace", withNamesake, false], + ["create, name free", "create", without, false], + ["browser, name free", "browser", without, false], + ] as const)("%s", (_label, choice, namesakes, expected) => { + expect(confirmsNamesake(choice, namesakes)).toBe(expected) + }) +}) + +describe("displayWorkspaceName", () => { + test.each([ + ["bidi controls are stripped", "‮analytics⁦x⁩؜", "analyticsx"], + ["a newline cannot break the line", "ana\nlytics", "ana lytics"], + ["plain text is unchanged", "analytics", "analytics"], + ] as const)("%s", (_label, name, expected) => { + expect(displayWorkspaceName(name)).toBe(expected) + }) +}) diff --git a/packages/opencode/test/cli/cmd/link.test.ts b/packages/opencode/test/cli/cmd/link.test.ts index 6d325d756..d9c64dd9c 100644 --- a/packages/opencode/test/cli/cmd/link.test.ts +++ b/packages/opencode/test/cli/cmd/link.test.ts @@ -9,7 +9,8 @@ // (LinkCommand.handler) needs a TTY and is covered by manual verification // (PR #1274), not here. import { afterEach, beforeEach, describe, expect, test } from "bun:test" -import { hyperlink, stripControlChars, terminalSupportsHyperlinks } from "../../../src/cli/cmd/link" +import { hyperlink, linkPickKind, stripControlChars, terminalSupportsHyperlinks } from "../../../src/cli/cmd/link" +import { confirmsNamesake, findNamesakes } from "../../../src/altimate/workspace/workspace-name" // Shared by both describe blocks below that exercise terminalSupportsHyperlinks // (directly, or indirectly via hyperlink()). Object.defineProperty defaults @@ -252,3 +253,16 @@ describe("hyperlink", () => { expect(out).not.toContain("\x1b") }) }) + +// Both create rows in the picker must reach the namesake confirmation. +describe("linkPickKind", () => { + const taken = findNamesakes([{ id: 1, name: "analytics", ownerId: 20 }], "analytics", 10) + test.each([ + ["the quick-create row", "__create_new__", "create", true], + ["the browser set-up row", "__browser_handoff__", "browser", true], + ["an existing workspace's row", "42", "workspace", false], + ] as const)("%s", (_label, pick, kind, confirms) => { + expect(linkPickKind(pick)).toBe(kind) + expect(confirmsNamesake(linkPickKind(pick), taken)).toBe(confirms) + }) +}) diff --git a/packages/opencode/test/skill/release-v0.11.2-adversarial.test.ts b/packages/opencode/test/skill/release-v0.11.2-adversarial.test.ts index 1c0dc6c99..43d6df53d 100644 --- a/packages/opencode/test/skill/release-v0.11.2-adversarial.test.ts +++ b/packages/opencode/test/skill/release-v0.11.2-adversarial.test.ts @@ -153,6 +153,7 @@ describe("v0.11.2 adversarial: stripControlChars", () => { const ranges: [number, number][] = [ [0x00, 0x1f], [0x7f, 0x9f], + [0x061c, 0x061c], [0x200e, 0x200f], [0x202a, 0x202e], [0x2066, 0x2069],