From 7c7e17f1933db764a474e0c572d27a4b3486adc6 Mon Sep 17 00:00:00 2001 From: Haider Date: Wed, 12 Aug 2026 23:06:50 +0530 Subject: [PATCH 01/14] feat: add Workspaces post-scan prompt + `altimate link` subcommand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the CLI half of the Workspaces pilot: after the first-run scan completes and the CLI is authenticated with Altimate, prompt the user once to create a new workspace or attach the project to an existing one. The link is a direct authenticated call — no device flow — and the browser opens after create so the user can configure integrations / knowledge in the SaaS. Fork-owned TuiPlugin per docs/internal/2026-06-23-tui-fork-features- as-plugins-adr.md: single file at `packages/opencode/src/plugin/tui/altimate/workspace.tsx`, added to the existing `altimateTuiPlugins()` aggregator. Upstream `packages/tui/**` stays byte-for-byte upstream. Uses the real `api.ui.*` / `api.keymap.registerLayer` / `api.state.path.directory` / `api.kv` (persistent) surface. Shared modules under `packages/opencode/src/altimate/workspace/` so the plugin and the `altimate link` subcommand can't drift on request shape or error handling: - `api-client.ts` — typed errors (Conflict/Precondition/NotFound/ Forbidden/NotConfigured/Api), FastAPI `{"detail": {...}}` parsing, 15s abort timeout, credentials re-read on every call so an account switch is picked up without restart. - `detect.ts` — `detectProjectRemote` + `projectNameFromRemote`; reuses `stripGitRemoteCredentials` (now exported from `project-scan.ts` so the two callers can't drift). - `state.ts` — local binding cache scoped to (tenant, apiUrl) with atomic write + post-write `chmod 0o600` + corruption recovery. Trigger: `onboarding-telemetry.ts` `tool.execute.after` hook publishes `TuiEvent.CommandExecute` with `"altimate.workspace.postScan"` when `project_scan` completes, gated on the new `Flag.ALTIMATE_WORKSPACE` and `AltimateApi.isConfigured()` (BYOK users are silently skipped — no place to send them). Never blocks onboarding on a publish failure. Server-authoritative pre-check via `GET /datamate-project-bindings/ by-remote`; local cache used only as an offline fallback, and the fallback path renders a mandatory "unverified" banner rather than silently trusting stale data. Browser-open failure surfaces a copyable-URL toast rather than swallowing silently. 7-day Skip latch lives in `api.kv` keyed by SHA-1(remote) — UTC rolling window; `altimate link` (user-initiated) deliberately bypasses the latch. New `altimate-code link` subcommand runs the same three-way flow outside a TUI session via `@clack/prompts` for scripting / catch-up after a Skip. Bails early with helpful messages when credentials are missing or no git remote is set. Tests: 17 unit tests covering project-name parsing, git detection graceful failure, cache read/write + chmod + tenant-scoping (account- switch invalidation), and Skip latch TTL semantics with UTC boundary. Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_01Q8FGy89Qpr39k8nCSpCcK2 --- packages/core/src/flag/flag.ts | 8 + .../altimate/plugin/onboarding-telemetry.ts | 32 ++ .../src/altimate/tools/project-scan.ts | 7 +- .../src/altimate/workspace/api-client.ts | 231 ++++++++ .../opencode/src/altimate/workspace/detect.ts | 34 ++ .../opencode/src/altimate/workspace/state.ts | 102 ++++ packages/opencode/src/cli/cmd/link.ts | 242 +++++++++ packages/opencode/src/index.ts | 6 + .../opencode/src/plugin/tui/altimate/index.ts | 4 +- .../src/plugin/tui/altimate/workspace.tsx | 503 ++++++++++++++++++ .../test/altimate/plugin/workspace.test.ts | 231 ++++++++ 11 files changed, 1398 insertions(+), 2 deletions(-) create mode 100644 packages/opencode/src/altimate/workspace/api-client.ts create mode 100644 packages/opencode/src/altimate/workspace/detect.ts create mode 100644 packages/opencode/src/altimate/workspace/state.ts create mode 100644 packages/opencode/src/cli/cmd/link.ts create mode 100644 packages/opencode/src/plugin/tui/altimate/workspace.tsx create mode 100644 packages/opencode/test/altimate/plugin/workspace.test.ts diff --git a/packages/core/src/flag/flag.ts b/packages/core/src/flag/flag.ts index abe3da75e5..519d212721 100644 --- a/packages/core/src/flag/flag.ts +++ b/packages/core/src/flag/flag.ts @@ -63,6 +63,14 @@ export const Flag = { OPENCODE_WORKSPACE_ID: process.env["OPENCODE_WORKSPACE_ID"], OPENCODE_EXPERIMENTAL_WORKSPACES: enabledByExperimental("OPENCODE_EXPERIMENTAL_WORKSPACES"), + // altimate_change start — pilot flag for the Workspaces feature (post-scan prompt + + // altimate link subcommand). Read as a getter so tests and the runtime `--` middleware + // can flip it between plugin activation and command execution. + get ALTIMATE_WORKSPACE() { + return enabledByExperimental("ALTIMATE_WORKSPACE") + }, + // altimate_change end + // Evaluated at access time (not module load) because tests, the CLI, and // external tooling set these env vars at runtime. get OPENCODE_DISABLE_PROJECT_CONFIG() { diff --git a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts index c18b890d02..b18d579a6b 100644 --- a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts +++ b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts @@ -12,6 +12,16 @@ // session loop. import type { Hooks, PluginInput } from "@opencode-ai/plugin" import * as OnboardingTelemetry from "../telemetry/onboarding" +// altimate_change start — AI-8398 workspaces trigger. Reaches into the same +// EventV2 bridge the server/routes/tui.ts uses to publish TuiEvent.CommandExecute +// so the workspace TuiPlugin (packages/opencode/src/plugin/tui/altimate/workspace.tsx) +// runs its post-scan flow. Feature-flagged via Flag.ALTIMATE_WORKSPACE. +import { Flag } from "@opencode-ai/core/flag/flag" +import { AltimateApi } from "@/altimate/api/client" +import { AppRuntime } from "@/effect/app-runtime" +import { EventV2Bridge } from "@/event-v2-bridge" +import { TuiEvent } from "@/server/tui-event" +// altimate_change end const ONBOARD_CONNECT = "onboard-connect" @@ -134,6 +144,28 @@ export async function OnboardingTelemetryPlugin(_input: PluginInput): Promise false))) { + void AppRuntime.runPromise( + EventV2Bridge.Service.use((events) => + events.publish(TuiEvent.CommandExecute, { + command: "altimate.workspace.postScan", + }), + ), + ).catch((err) => { + // Never block onboarding on a publish failure — worst case the + // user picks up the workspace on their next launch or via the + // /altimate.workspace.link palette command. + // eslint-disable-next-line no-console + console.error("[altimate-workspace] postScan trigger failed:", err) + }) + } + // altimate_change end return } diff --git a/packages/opencode/src/altimate/tools/project-scan.ts b/packages/opencode/src/altimate/tools/project-scan.ts index 130338cee4..b98a89d2e6 100644 --- a/packages/opencode/src/altimate/tools/project-scan.ts +++ b/packages/opencode/src/altimate/tools/project-scan.ts @@ -173,8 +173,13 @@ export async function detectGit(): Promise { * SSH-form remotes (`git@github.com:owner/repo.git`) have no userinfo * concept and are left untouched. URLs we can't parse are dropped to * undefined (better to lose the breadcrumb than leak creds). + * + * Exported so the workspace TuiPlugin (packages/opencode/src/plugin/tui/ + * altimate/workspace.tsx) can reuse the exact same scrubbing rules when + * deriving the project's git remote for the post-scan prompt — the alt + * of duplicating the logic risks the two callers drifting. */ -function stripGitRemoteCredentials(url: string): string | undefined { +export function stripGitRemoteCredentials(url: string): string | undefined { if (!url) return undefined // SSH form: `git@host:path` — no creds to strip. if (/^[\w.-]+@[\w.-]+:/.test(url) && !url.includes("://")) return url diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts new file mode 100644 index 0000000000..8bf859f0a8 --- /dev/null +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -0,0 +1,231 @@ +// altimate_change - new file +// +// Wire client for the workspace-binding endpoints in altimate-backend +// (/datamate-project-bindings/*, added by AI-8398). Shared between the TUI +// plugin (packages/opencode/src/plugin/tui/altimate/workspace.tsx) and the +// `altimate link` CLI subcommand (packages/opencode/src/cli/cmd/link.ts) so +// the two entry points can't drift on request shape / error handling. +// +// Reads AltimateApi credentials on every call so an account switch is picked +// up immediately without a plugin restart. All FastAPI HTTPException.detail +// bodies come out as `{"detail": }` — we parse the object form +// for 409/412 and surface it as a typed error rather than a bare status code. +import { AltimateApi } from "@/altimate/api/client" + +const REQUEST_TIMEOUT_MS = 15_000 + +export interface DatamateRef { + id: number + name: string +} + +export interface Binding { + id: number + datamate_id: number + datamate_name: string + repo_remote: string + created_at?: string +} + +export interface CreateAndBindResponse { + datamate: DatamateRef + binding: Binding + manage_url: string +} + +export interface BindingResponse { + binding: Binding +} + +export interface GetBindingResponse { + binding: Binding + datamate: DatamateRef +} + +export interface ConflictDetail { + message: string + existing_datamate_id?: number + existing_datamate_name?: string | null + repo_remote?: string +} + +export interface PreconditionDetail { + message: string + actual_current_datamate_id?: number + expected_current_datamate_id?: number +} + +export class NotConfiguredError extends Error { + constructor() { + super("Altimate credentials not configured — sign in first.") + this.name = "NotConfiguredError" + } +} + +export class ConflictError extends Error { + constructor(public readonly detail: ConflictDetail) { + super(detail.message) + this.name = "ConflictError" + } +} + +export class PreconditionFailedError extends Error { + constructor(public readonly detail: PreconditionDetail) { + super(detail.message) + this.name = "PreconditionFailedError" + } +} + +export class NotFoundError extends Error { + constructor(msg = "Not found") { + super(msg) + this.name = "NotFoundError" + } +} + +export class ForbiddenError extends Error { + constructor(msg = "Forbidden") { + super(msg) + this.name = "ForbiddenError" + } +} + +export class WorkspaceApiError extends Error { + constructor( + msg: string, + public readonly status?: number, + ) { + super(msg) + this.name = "WorkspaceApiError" + } +} + +async function creds(): Promise<{ url: string; instance: string; apiKey: string }> { + if (!(await AltimateApi.isConfigured())) throw new NotConfiguredError() + const c = await AltimateApi.getCredentials() + return { url: c.altimateUrl, instance: c.altimateInstanceName, apiKey: c.altimateApiKey } +} + +async function req( + method: string, + subpath: string, + opts: { body?: unknown; query?: Record } = {}, +): Promise { + const { url, instance, apiKey } = await creds() + const qs = opts.query ? "?" + new URLSearchParams(opts.query).toString() : "" + const target = `${url}/datamate-project-bindings${subpath}${qs}` + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS) + let res: Response + try { + res = await fetch(target, { + method, + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${apiKey}`, + "x-tenant": instance, + }, + signal: controller.signal, + ...(opts.body !== undefined ? { body: JSON.stringify(opts.body) } : {}), + }) + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + throw new WorkspaceApiError(`Cannot reach ${target}: ${msg}`) + } finally { + clearTimeout(timeout) + } + let json: unknown = undefined + const text = await res.text().catch(() => "") + if (text) { + try { + json = JSON.parse(text) + } catch { + /* non-JSON body — surface as opaque via status code below */ + } + } + const detail = (json as { detail?: unknown } | undefined)?.detail + if (res.status === 404) throw new NotFoundError(typeof detail === "string" ? detail : "Not found") + if (res.status === 403) throw new ForbiddenError(typeof detail === "string" ? detail : "Forbidden") + if (res.status === 409) { + const d = + typeof detail === "object" && detail !== null + ? (detail as ConflictDetail) + : { message: typeof detail === "string" ? detail : "Conflict" } + throw new ConflictError(d) + } + if (res.status === 412) { + const d = + typeof detail === "object" && detail !== null + ? (detail as PreconditionDetail) + : { message: typeof detail === "string" ? detail : "Precondition failed" } + throw new PreconditionFailedError(d) + } + if (!res.ok) { + throw new WorkspaceApiError( + typeof detail === "string" ? detail : `Request failed with status ${res.status}`, + res.status, + ) + } + return json as T +} + +export namespace WorkspaceApi { + /** Server-authoritative pre-check: is this remote already bound in this tenant? */ + export async function getBindingForRemote(remote: string): Promise { + try { + return await req("GET", "/by-remote", { query: { repo_remote: remote } }) + } catch (err) { + if (err instanceof NotFoundError) return null + throw err + } + } + + export async function createAndBind(input: { + name: string + repoRemote: string + description?: string + }): Promise { + return req("POST", "/", { + body: { + name: input.name, + repo_remote: input.repoRemote, + description: input.description ?? null, + }, + }) + } + + export async function bindExisting(datamateId: number, remote: string): Promise { + return req("POST", "/bind", { + body: { datamate_id: datamateId, repo_remote: remote }, + }) + } + + export async function rebindByRemote(input: { + remote: string + targetDatamateId: number + expectedCurrentDatamateId?: number + }): Promise { + return req("PUT", "/by-remote", { + body: { + repo_remote: input.remote, + target_datamate_id: input.targetDatamateId, + ...(input.expectedCurrentDatamateId !== undefined + ? { expected_current_datamate_id: input.expectedCurrentDatamateId } + : {}), + }, + }) + } + + /** Populates the "link to existing workspace" picker. Reuses the existing + * /datamates/ list endpoint on the datamates_router. */ + export async function listDatamates(): Promise { + const { url, instance, apiKey } = await creds() + const res = await fetch(`${url}/datamates/`, { + headers: { Authorization: `Bearer ${apiKey}`, "x-tenant": instance }, + }) + if (!res.ok) + throw new WorkspaceApiError(`Failed to list workspaces (status ${res.status})`, res.status) + const body = (await res.json()) as { datamates?: Array<{ id: number | string; name: string }> } + return (body.datamates ?? []).map((d) => ({ id: Number(d.id), name: d.name })) + } +} diff --git a/packages/opencode/src/altimate/workspace/detect.ts b/packages/opencode/src/altimate/workspace/detect.ts new file mode 100644 index 0000000000..5b461e52de --- /dev/null +++ b/packages/opencode/src/altimate/workspace/detect.ts @@ -0,0 +1,34 @@ +// altimate_change - new file +// +// Cheap, sync git-remote detection for the workspace-binding flow. Shared by +// the TuiPlugin (packages/opencode/src/plugin/tui/altimate/workspace.tsx) and +// the `altimate link` CLI subcommand so both entry points identify projects +// identically. Reuses the credential-scrubbing helper the ProjectScan tool +// exports (../tools/project-scan.ts) so an HTTPS remote with embedded +// basic-auth (e.g. `https://x-access-token:ghp_xxx@github.com/...`) never +// reaches the server or the local cache in clear. +import { spawnSync } from "node:child_process" +import { stripGitRemoteCredentials } from "../tools/project-scan" + +export function detectProjectRemote(directory: string): string | undefined { + try { + const r = spawnSync("git", ["remote", "get-url", "origin"], { + cwd: directory, + encoding: "utf8", + timeout: 3000, + }) + if (r.status !== 0 || !r.stdout) return undefined + return stripGitRemoteCredentials(r.stdout.trim()) + } catch { + return undefined + } +} + +/** Sensible default workspace name from a remote URL — best-effort. Used to + * prefill the workspace-name prompt in the CreateDialog and `altimate link`. + * ``github.com/foo/bar.git`` → ``bar`` ; ``git@github.com:foo/bar.git`` → ``bar``. */ +export function projectNameFromRemote(remote: string): string { + const trimmed = remote.replace(/\.git$/, "").replace(/\/$/, "") + const parts = trimmed.split(/[/:]/) + return parts[parts.length - 1] || "workspace" +} diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts new file mode 100644 index 0000000000..09b559e3ad --- /dev/null +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -0,0 +1,102 @@ +// altimate_change - new file +// +// Local binding cache — offline fallback for the server-authoritative +// pre-check. Scoped to (tenant, apiUrl) at the top level so an account switch +// silently invalidates every cached entry (the switched-to session never sees +// another tenant's workspace names). +// +// Shared between the TuiPlugin and the `altimate link` CLI subcommand so both +// entry points see the same view of local state. File lives under +// ``Global.Path.state`` at 0o600 — chmod is applied post-write since +// ``Filesystem.writeJsonAtomic`` does not chmod (see filesystem.ts:294 for +// why; codex round-2 flagged this gap). +import { chmodSync, existsSync, readFileSync } from "node:fs" +import path from "node:path" +import { AltimateApi } from "@/altimate/api/client" +import { Global } from "@/global" +import { Filesystem } from "@/util/filesystem" +import { Log } from "@/altimate/util/log" + +const CACHE_VERSION = 1 + +const log = Log.create({ service: "altimate-workspace-state" }) + +export interface CachedBinding { + datamateId: number + datamateName: string + repoRemote: string + linkedAt: number +} + +interface CacheFile { + version: 1 + tenant: string + apiUrl: string + bindings: Record +} + +export function cachePath(): string { + return path.join(Global.Path.state, "altimate-workspace-bindings.json") +} + +function readCache(): CacheFile | null { + const p = cachePath() + if (!existsSync(p)) return null + try { + const raw = JSON.parse(readFileSync(p, "utf8")) as CacheFile + if (!raw || raw.version !== CACHE_VERSION) return null + return raw + } catch (err) { + log.warn("workspace binding cache is corrupt, discarding", { + code: (err as NodeJS.ErrnoException)?.code, + }) + return null + } +} + +function writeCache(cache: CacheFile): void { + const p = cachePath() + Filesystem.writeJsonAtomic(p, cache) + // Best-effort chmod — if the process dies before this line the file exists + // with umask perms, and the next successful write repairs it. Acceptable + // window given the cache holds workspace names, not credentials. + try { + chmodSync(p, 0o600) + } catch (err) { + log.warn("could not chmod workspace binding cache", { + code: (err as NodeJS.ErrnoException)?.code, + }) + } +} + +async function tenantKey(): Promise<{ tenant: string; apiUrl: string } | null> { + if (!(await AltimateApi.isConfigured())) return null + const c = await AltimateApi.getCredentials() + return { tenant: c.altimateInstanceName, apiUrl: c.altimateUrl } +} + +/** Read the local binding for ``directory`` — only returns a hit when the + * cache's stored (tenant, apiUrl) matches the current credentials. */ +export async function readLocalBinding(directory: string): Promise { + const key = await tenantKey() + if (!key) return null + const cache = readCache() + if (!cache) return null + if (cache.tenant !== key.tenant || cache.apiUrl !== key.apiUrl) return null + return cache.bindings[directory] ?? null +} + +export async function recordApprovedBinding( + directory: string, + binding: CachedBinding, +): Promise { + const key = await tenantKey() + if (!key) return + const existing = readCache() + const cache: CacheFile = + existing && existing.tenant === key.tenant && existing.apiUrl === key.apiUrl + ? existing + : { version: CACHE_VERSION, tenant: key.tenant, apiUrl: key.apiUrl, bindings: {} } + cache.bindings[directory] = binding + writeCache(cache) +} diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts new file mode 100644 index 0000000000..288ee3acfe --- /dev/null +++ b/packages/opencode/src/cli/cmd/link.ts @@ -0,0 +1,242 @@ +// altimate_change - new file +// +// On-demand "link this project to a workspace" subcommand. Runs the same three- +// way flow the TuiPlugin (packages/opencode/src/plugin/tui/altimate/workspace.tsx) +// offers post-scan, but outside a TUI session — so a user who skipped the +// post-scan offer (or skipped it 7+ days ago and wants to link now) has a +// path back in. +// +// Deliberately shares the WorkspaceApi + state modules with the plugin so the +// two entry points can't drift on request shape or error handling. +import { cmd } from "./cmd" +import { UI } from "../ui" +import * as prompts from "@clack/prompts" +import open from "open" +import { AltimateApi } from "@/altimate/api/client" +import { + WorkspaceApi, + ConflictError, + ForbiddenError, + NotConfiguredError, + NotFoundError, + PreconditionFailedError, + type DatamateRef, +} from "@/altimate/workspace/api-client" +import { detectProjectRemote, projectNameFromRemote } from "@/altimate/workspace/detect" +import { recordApprovedBinding } from "@/altimate/workspace/state" + +export const LinkCommand = cmd({ + command: "link", + describe: "Link this project to an Altimate workspace", + builder: (yargs) => + yargs.option("directory", { + alias: "d", + describe: "Project directory (defaults to cwd)", + type: "string", + default: process.cwd(), + }), + handler: async (args) => { + // Bail early if the CLI has no Altimate credentials — there's no place to + // send them and no gateway to authenticate against. + if (!(await AltimateApi.isConfigured())) { + UI.error( + "Not signed in to Altimate. Run the TUI (altimate-code) and sign in first, then re-run `altimate-code link`.", + ) + process.exitCode = 1 + return + } + + const remote = detectProjectRemote(args.directory) + if (!remote) { + UI.error( + `No git remote found in ${args.directory}. A workspace is bound to a project via its git remote — initialize a repo and add a remote first.`, + ) + process.exitCode = 1 + return + } + + prompts.intro("Link workspace") + prompts.log.info(`Project remote: ${remote}`) + + // Server-authoritative pre-check. + let existing: Awaited> = null + try { + existing = await WorkspaceApi.getBindingForRemote(remote) + } catch (err) { + if (err instanceof NotConfiguredError) { + UI.error(err.message) + process.exitCode = 1 + return + } + prompts.log.warn( + `Could not reach the workspace service (${err instanceof Error ? err.message : String(err)}). Continuing anyway.`, + ) + } + + if (existing) { + const choice = await prompts.select<"attach" | "relink" | "cancel">({ + message: `This project is already linked to workspace "${existing.datamate.name}".`, + options: [ + { value: "attach", label: "Keep this binding (no changes)" }, + { value: "relink", label: "Re-link to a different workspace" }, + { value: "cancel", label: "Cancel" }, + ], + initialValue: "attach", + }) + if (prompts.isCancel(choice) || choice === "cancel" || choice === "attach") { + prompts.outro(choice === "attach" ? `Kept "${existing.datamate.name}".` : "No changes.") + return + } + await pickAndBind(remote, "relink", existing.datamate.id, args.directory) + return + } + + const choice = await prompts.select<"create" | "link" | "cancel">({ + message: "Set up a workspace for this project?", + options: [ + { + value: "create", + label: "Create a new workspace", + hint: "Opens a browser to configure integrations and knowledge.", + }, + { + value: "link", + label: "Link to an existing workspace", + hint: "Attach this project to a workspace you already own.", + }, + { value: "cancel", label: "Cancel" }, + ], + initialValue: "create", + }) + if (prompts.isCancel(choice) || choice === "cancel") { + prompts.outro("No workspace was linked.") + return + } + + if (choice === "create") { + await createAndBind(remote, args.directory) + return + } + await pickAndBind(remote, "attach", undefined, args.directory) + }, +}) + +async function createAndBind(remote: string, directory: string): Promise { + const defaultName = projectNameFromRemote(remote) + const nameInput = await prompts.text({ + message: "Name this workspace", + placeholder: defaultName, + defaultValue: defaultName, + }) + if (prompts.isCancel(nameInput)) { + prompts.outro("No workspace was created.") + return + } + const name = String(nameInput).trim() || defaultName + + const spin = prompts.spinner() + spin.start(`Creating workspace "${name}"...`) + try { + const res = await WorkspaceApi.createAndBind({ name, repoRemote: remote }) + await recordApprovedBinding(directory, { + datamateId: res.datamate.id, + datamateName: res.datamate.name, + repoRemote: res.binding.repo_remote, + linkedAt: Date.now(), + }) + spin.stop(`Workspace "${res.datamate.name}" created and linked.`) + prompts.log.info(`Manage it at: ${res.manage_url}`) + // Best-effort browser open — never blocks. If it fails the user has the + // URL above to copy manually. + await open(res.manage_url).catch(() => undefined) + prompts.outro("Done.") + } catch (err) { + spin.stop("Failed to create workspace.", 1) + if (err instanceof ConflictError) { + prompts.log.error( + `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` and pick "Re-link" if you want to move it.`, + ) + } else { + prompts.log.error(err instanceof Error ? err.message : String(err)) + } + process.exitCode = 1 + } +} + +async function pickAndBind( + remote: string, + mode: "attach" | "relink", + expectedCurrentDatamateId: number | undefined, + directory: string, +): Promise { + const spin = prompts.spinner() + spin.start("Loading workspaces...") + let list: DatamateRef[] + try { + list = await WorkspaceApi.listDatamates() + } catch (err) { + spin.stop("Could not load workspaces.", 1) + prompts.log.error(err instanceof Error ? err.message : String(err)) + process.exitCode = 1 + return + } + spin.stop(`Found ${list.length} workspace${list.length === 1 ? "" : "s"}.`) + + if (list.length === 0) { + prompts.log.warn( + "You don't have any workspaces yet. Re-run and pick \"Create a new workspace\" instead.", + ) + prompts.outro("No workspace to link.") + return + } + + const pick = await prompts.select({ + message: mode === "attach" ? "Pick a workspace to attach to" : "Pick a workspace to re-link to", + options: [ + ...list.map((dm) => ({ value: dm.id as number | "cancel", label: dm.name })), + { value: "cancel" as const, label: "Cancel" }, + ], + }) + if (prompts.isCancel(pick) || pick === "cancel") { + prompts.outro("No changes.") + return + } + + const target = list.find((dm) => dm.id === pick)! + const spin2 = prompts.spinner() + spin2.start(`${mode === "attach" ? "Linking" : "Re-linking"} to "${target.name}"...`) + try { + const res = + mode === "attach" + ? await WorkspaceApi.bindExisting(pick, remote) + : await WorkspaceApi.rebindByRemote({ + remote, + targetDatamateId: pick, + expectedCurrentDatamateId, + }) + await recordApprovedBinding(directory, { + datamateId: res.binding.datamate_id, + datamateName: res.binding.datamate_name, + repoRemote: res.binding.repo_remote, + linkedAt: Date.now(), + }) + spin2.stop(`${mode === "attach" ? "Linked" : "Re-linked"} to "${res.binding.datamate_name}".`) + prompts.outro("Done.") + } catch (err) { + spin2.stop(`${mode === "attach" ? "Link" : "Re-link"} failed.`, 1) + if (err instanceof ConflictError) { + prompts.log.error( + `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` and pick "Re-link".`, + ) + } else if (err instanceof PreconditionFailedError) { + prompts.log.error("Someone else re-linked this project — reload and try again.") + } else if (err instanceof NotFoundError) { + prompts.log.error("No existing binding to re-link. Try again and pick Create or Link.") + } else if (err instanceof ForbiddenError) { + prompts.log.error("Only the workspace owner can attach projects to it.") + } else { + prompts.log.error(err instanceof Error ? err.message : String(err)) + } + process.exitCode = 1 + } +} diff --git a/packages/opencode/src/index.ts b/packages/opencode/src/index.ts index 69da571611..82ef4cb443 100644 --- a/packages/opencode/src/index.ts +++ b/packages/opencode/src/index.ts @@ -44,6 +44,9 @@ import { SkillCommand } from "./cli/cmd/skill" // altimate_change start — check: deterministic SQL check command import { CheckCommand } from "./cli/cmd/check" // altimate_change end +// altimate_change start — link: workspace-binding subcommand +import { LinkCommand } from "./cli/cmd/link" +// altimate_change end import { errorMessage } from "./util/error" import { PluginCommand } from "./cli/cmd/plug" import { Heap } from "./cli/heap" @@ -169,6 +172,9 @@ let cli = yargs(args) // altimate_change end // altimate_change start — check: register deterministic SQL check command .command(CheckCommand) + // altimate_change end + // altimate_change start — link: workspace-binding subcommand + .command(LinkCommand) // altimate_change end // altimate_change start — workspace-serve: register dev-only workspace serve command diff --git a/packages/opencode/src/plugin/tui/altimate/index.ts b/packages/opencode/src/plugin/tui/altimate/index.ts index 766e0593e5..900e06fe38 100644 --- a/packages/opencode/src/plugin/tui/altimate/index.ts +++ b/packages/opencode/src/plugin/tui/altimate/index.ts @@ -13,6 +13,7 @@ import ProviderCredentials from "./provider-credentials" import PromptEnhance from "./prompt-enhance" import SkillOps from "./skill-ops" import TraceViewer from "./trace-viewer" +import Workspace from "./workspace" // Feature plugins are registered here as they are ported from the pre-merge sources on `main` // (see the ADR re-home plan). Each lives in its own file under this directory and default-exports @@ -21,7 +22,8 @@ import TraceViewer from "./trace-viewer" // import SkillOps from "./skill-ops" // import PromptEnhance from "./prompt-enhance" // import TraceViewer from "./trace-viewer" +// import Workspace from "./workspace" export function altimateTuiPlugins(_flags: Pick): BuiltinTuiPlugin[] { - return [ProviderCredentials, PromptEnhance, SkillOps, TraceViewer] + return [ProviderCredentials, PromptEnhance, SkillOps, TraceViewer, Workspace] } // altimate_change end diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx new file mode 100644 index 0000000000..47f00b6e8f --- /dev/null +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -0,0 +1,503 @@ +// altimate_change start — fork TUI feature: Workspaces (pilot). +// +// Post-scan prompt to create or link a "Workspace" (server-side: a Datamate) +// to the current project, plus an on-demand "Link this project to a workspace" +// palette command. Server API lives in altimate-backend under +// /datamate-project-bindings/* (top-level router). Backend contract: +// +// POST /datamate-project-bindings/ create-and-bind (atomic) +// POST /datamate-project-bindings/bind attach to existing workspace +// PUT /datamate-project-bindings/by-remote atomic re-link (FOR UPDATE) +// GET /datamate-project-bindings/by-remote server-authoritative lookup +// +// Fork-owned plugin per docs/internal/2026-06-23-tui-fork-features-as-plugins-adr.md. +// Registered by ./index.ts's altimateTuiPlugins() aggregator; upstream +// packages/tui is not touched. Uses `api.ui.*`, `api.keymap.registerLayer`, +// `api.state.path.directory`, `api.kv` (persistent) — the real TuiPluginApi +// surface, not a made-up one (see codex round-2 report for the history). +// +// Trigger: the /altimate-workspace.postScan command is dispatched by the +// existing onboarding-telemetry.ts plugin's `tool.execute.after` hook when +// `project_scan` completes AND `AltimateApi.isConfigured()` returns true AND +// `Flag.ALTIMATE_WORKSPACE` is on. Dispatch travels via the existing +// `TuiEvent.CommandExecute` event bus. +import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" +import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" +import { createHash } from "node:crypto" +import open from "open" +import { createSignal, onMount, Show } from "solid-js" +import { + ConflictError, + ForbiddenError, + NotFoundError, + PreconditionFailedError, + WorkspaceApi, + type DatamateRef, + type GetBindingResponse, +} from "@/altimate/workspace/api-client" +import { detectProjectRemote, projectNameFromRemote } from "@/altimate/workspace/detect" +import { readLocalBinding, recordApprovedBinding } from "@/altimate/workspace/state" +import { Log } from "@/altimate/util/log" + +const PLUGIN_ID = "altimate:workspace" + +const log = Log.create({ service: "altimate-workspace" }) + +// ───────────────────────────────────────────────────────────────────────────── +// Skip latch (TUI-only). Uses TuiPluginApi.kv — persistent across sessions +// via packages/tui/src/context/kv.tsx (state/kv.json). The `altimate link` +// subcommand deliberately bypasses this latch (it's user-initiated). +// ───────────────────────────────────────────────────────────────────────────── + +const SKIP_TTL_MS = 7 * 24 * 60 * 60 * 1000 +const KV_SKIP_PREFIX = "altimate.workspace.postScan.skip." + +function skipKey(remote: string): string { + // Hash to keep the KV keyspace bounded and avoid embedding a URL (which can + // contain userinfo even after scrubbing edge cases) into a persisted key. + return KV_SKIP_PREFIX + createHash("sha1").update(remote).digest("hex") +} + +function isSkipActive(api: TuiPluginApi, remote: string, nowMs: number): boolean { + const rec = api.kv.get<{ skippedAt: number }>(skipKey(remote)) + if (!rec || typeof rec.skippedAt !== "number") return false + return nowMs - rec.skippedAt < SKIP_TTL_MS +} + +function recordSkip(api: TuiPluginApi, remote: string, nowMs: number): void { + api.kv.set(skipKey(remote), { skippedAt: nowMs }) +} + +// ───────────────────────────────────────────────────────────────────────────── +// Dialog components — deliberate three-way selects; no LLM-generated copy. +// Every dialog closes via `api.ui.dialog.clear()` when the user picks a +// terminal action, so onboarding is never blocked by a stuck workspace prompt. +// ───────────────────────────────────────────────────────────────────────────── + +interface OfferProps { + api: TuiPluginApi + remote: string + defaultName: string + suppressLatch?: boolean // altimate link on-demand skips the Skip latch +} + +function OfferDialog(props: OfferProps) { + return ( + { + if (option.value === "skip") { + if (!props.suppressLatch) recordSkip(props.api, props.remote, Date.now()) + props.api.ui.dialog.clear() + return + } + if (option.value === "create") { + props.api.ui.dialog.replace(() => ( + + )) + return + } + // link → picker + props.api.ui.dialog.replace(() => ( + + )) + }} + /> + ) +} + +interface AlreadyLinkedProps { + api: TuiPluginApi + remote: string + workspaceName: string + workspaceId: number + hasDrift: boolean + driftedWas?: string | null + unverified?: boolean +} + +function AlreadyLinkedDialog(props: AlreadyLinkedProps) { + // Title carries the primary context (workspace name + drift/unverified hint) + // since DialogSelect doesn't take a top-level description block. Verbose but + // it puts the critical info in the user's field of view before they pick. + const title = () => { + const parts: string[] = [`Project is linked to workspace "${props.workspaceName}"`] + if (props.hasDrift && props.driftedWas) parts.push(`(was ${props.driftedWas}, now ${props.remote})`) + if (props.unverified) parts.push("(⚠ unverified — server unreachable, showing cached value)") + return parts.join(" ") + } + return ( + { + if (option.value === "attach" || option.value === "skip") { + props.api.ui.dialog.clear() + return + } + // relink → picker with the current workspace id as expected_current so + // a concurrent re-link by another client 412s cleanly. + props.api.ui.dialog.replace(() => ( + + )) + }} + /> + ) +} + +interface PickerProps { + api: TuiPluginApi + remote: string + mode: "attach" | "relink" + expectedCurrentDatamateId?: number +} + +function PickerDialog(props: PickerProps) { + const [datamates, setDatamates] = createSignal(null) + const [loadError, setLoadError] = createSignal(null) + + onMount(async () => { + try { + const list = await WorkspaceApi.listDatamates() + setDatamates(list) + } catch (err) { + const msg = err instanceof Error ? err.message : "Failed to load workspaces" + setLoadError(msg) + props.api.ui.toast({ variant: "error", message: msg }) + props.api.ui.dialog.clear() + } + }) + + async function pick(datamateId: number) { + try { + if (props.mode === "attach") { + const res = await WorkspaceApi.bindExisting(datamateId, props.remote) + await recordApprovedBinding(props.api.state.path.directory, { + datamateId: res.binding.datamate_id, + datamateName: res.binding.datamate_name, + repoRemote: res.binding.repo_remote, + linkedAt: Date.now(), + }) + props.api.ui.toast({ + variant: "success", + message: `Linked to workspace "${res.binding.datamate_name}".`, + }) + } else { + const res = await WorkspaceApi.rebindByRemote({ + remote: props.remote, + targetDatamateId: datamateId, + expectedCurrentDatamateId: props.expectedCurrentDatamateId, + }) + await recordApprovedBinding(props.api.state.path.directory, { + datamateId: res.binding.datamate_id, + datamateName: res.binding.datamate_name, + repoRemote: res.binding.repo_remote, + linkedAt: Date.now(), + }) + props.api.ui.toast({ + variant: "success", + message: `Re-linked to workspace "${res.binding.datamate_name}".`, + }) + } + props.api.ui.dialog.clear() + } catch (err) { + // Surface as a toast so the user sees the specific failure. Dialog closes + // either way — the user can re-invoke via /altimate.workspace.link. + let msg: string + if (err instanceof ConflictError) { + msg = `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}" — pick Re-link from the offer if you want to move it.` + } else if (err instanceof PreconditionFailedError) { + msg = "Someone else re-linked this project — reload and try again." + } else if (err instanceof NotFoundError) { + msg = "No existing binding for this remote to re-link. Try Create/Link from the offer instead." + } else if (err instanceof ForbiddenError) { + msg = "Only the workspace owner can attach projects to it." + } else { + msg = err instanceof Error ? err.message : "Failed to link workspace" + } + props.api.ui.toast({ variant: "error", message: msg }) + props.api.ui.dialog.clear() + } + } + + // While loading (or on error before dialog closes), render an empty select as + // a placeholder — DialogSelect requires the options array up front and doesn't + // have a native busy state; the empty list closes to a "no workspaces" message. + const options = () => { + const list = datamates() + if (!list) return [{ title: "Loading workspaces...", value: -1, disabled: true }] + if (list.length === 0) + return [ + { + title: "No workspaces yet — cancel and pick 'Create a new workspace' instead.", + value: -1, + disabled: true, + }, + ] + return list.map((dm: DatamateRef) => ({ title: dm.name, value: dm.id })) + } + + return ( + + title={props.mode === "attach" ? "Link to workspace" : "Re-link to workspace"} + options={options()} + onSelect={(option) => { + if (option.value === -1) { + props.api.ui.dialog.clear() + return + } + void pick(option.value) + }} + /> + ) +} + +interface CreateProps { + api: TuiPluginApi + defaultName: string + remote: string +} + +function CreateDialog(props: CreateProps) { + const [busy, setBusy] = createSignal(false) + const [error, setError] = createSignal(null) + return ( + ( + + Name this workspace (defaults to your project name): + You can add integrations, knowledge, and guardrails in the browser after. + + {error()!} + + + )} + onConfirm={async (value) => { + if (busy()) return + const name = (value || "").trim() || props.defaultName || "Untitled Workspace" + setBusy(true) + setError(null) + try { + const res = await WorkspaceApi.createAndBind({ + name, + repoRemote: props.remote, + }) + await recordApprovedBinding(props.api.state.path.directory, { + datamateId: res.datamate.id, + datamateName: res.datamate.name, + repoRemote: res.binding.repo_remote, + linkedAt: Date.now(), + }) + // Best-effort browser open. On failure, fall back to a toast with the + // URL so the user has a copy-pasteable path forward — silent failure + // was flagged by codex round 1. + try { + await open(res.manage_url) + props.api.ui.toast({ + variant: "success", + message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, + }) + } catch { + props.api.ui.toast({ + variant: "info", + message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} in your browser to configure it.`, + duration: 10_000, + }) + } + props.api.ui.dialog.clear() + } catch (err) { + if (err instanceof ConflictError) { + setError( + `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Cancel and pick "Re-link" from the offer if you want to move it.`, + ) + } else { + setError(err instanceof Error ? err.message : "Failed to create workspace") + } + } finally { + setBusy(false) + } + }} + onCancel={() => props.api.ui.dialog.clear()} + /> + ) +} + +// ───────────────────────────────────────────────────────────────────────────── +// Flow orchestrator — runs on every `altimate.workspace.postScan` command +// dispatch AND on every `altimate.workspace.link` command dispatch. Idempotent +// re-entry: if a dialog is already up, the mount replaces it (that's fine — +// two rapid scan completions collapse to one visible offer). +// ───────────────────────────────────────────────────────────────────────────── + +async function runFlow( + api: TuiPluginApi, + directory: string, + opts: { suppressLatch?: boolean } = {}, +): Promise { + const remote = detectProjectRemote(directory) + if (!remote) { + // Not a git project (or git failed) — nothing to bind to. Silent per spec: + // the ticket says "handle failures without blocking the existing flow". + return + } + if (!opts.suppressLatch && isSkipActive(api, remote, Date.now())) { + log.info("workspace prompt suppressed by 7-day Skip latch", { remote }) + return + } + + let serverBinding: GetBindingResponse | null | undefined + try { + serverBinding = await WorkspaceApi.getBindingForRemote(remote) + } catch (err) { + log.warn("workspace pre-check server call failed, falling back to local cache", { + err: err instanceof Error ? err.message : String(err), + }) + serverBinding = undefined + } + + if (serverBinding) { + // Warm the local cache so an offline follow-up render is consistent. + await recordApprovedBinding(directory, { + datamateId: serverBinding.datamate.id, + datamateName: serverBinding.datamate.name, + repoRemote: remote, + linkedAt: Date.now(), + }) + api.ui.dialog.replace(() => ( + + )) + return + } + + if (serverBinding === null) { + // Server confirmed unbound → offer create-or-link. + api.ui.dialog.replace(() => ( + + )) + return + } + + // Server unreachable — fall back to the local cache (marked as unverified). + const local = await readLocalBinding(directory) + if (local) { + api.ui.dialog.replace(() => ( + + )) + return + } + // No local cache either → offer, but flag the server-unreachable state so the + // user can decide whether to proceed. + api.ui.toast({ + variant: "warning", + message: "Could not reach the Altimate workspace service — pre-check skipped.", + }) + api.ui.dialog.replace(() => ( + + )) +} + +// ───────────────────────────────────────────────────────────────────────────── +// Plugin registration +// ───────────────────────────────────────────────────────────────────────────── + +const tui: TuiPlugin = async (api) => { + api.keymap.registerLayer({ + commands: [ + { + name: "altimate.workspace.postScan", + title: "Post-scan workspace prompt", + category: "Altimate", + namespace: "internal", + run() { + void runFlow(api, api.state.path.directory) + }, + }, + { + name: "altimate.workspace.link", + title: "Link this project to a workspace", + category: "Altimate", + namespace: "palette", + run() { + // User-initiated → bypass the Skip latch. + void runFlow(api, api.state.path.directory, { suppressLatch: true }) + }, + }, + ], + }) +} + +export default { id: PLUGIN_ID, tui } satisfies BuiltinTuiPlugin + +// Exported for unit tests only. The shared logic (WorkspaceApi, cache, detect, +// project-name) lives in `@/altimate/workspace/*` and should be tested there; +// the plugin owns just the TUI-specific latch semantics. +export { isSkipActive, recordSkip } +// altimate_change end diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts new file mode 100644 index 0000000000..4ba5328b2b --- /dev/null +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -0,0 +1,231 @@ +// altimate_change - new file +// Unit coverage for the pure-logic pieces of the workspace TuiPlugin +// (packages/opencode/src/plugin/tui/altimate/workspace.tsx). The JSX +// components and the AltimateApi credential fetch are not exercised here — +// they need a running TUI harness and are covered by the manual smoke plan. +// This file focuses on the deterministic layer: URL parsing, git detection, +// state read/write + chmod, latch semantics, and error classification. +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { existsSync, mkdirSync, rmSync, statSync } from "node:fs" +import path from "node:path" +import os from "node:os" + +// Redirect Global.Path.state BEFORE importing the module under test so its +// module-level cachePath() resolves inside the sandbox. +const SANDBOX = path.join(os.tmpdir(), `altimate-workspace-test-${process.pid}-${Date.now()}`) +mkdirSync(path.join(SANDBOX, "state"), { recursive: true }) +process.env.XDG_STATE_HOME = path.join(SANDBOX, "state") + +const { isSkipActive, recordSkip } = await import( + "../../../src/plugin/tui/altimate/workspace" +) +const { projectNameFromRemote, detectProjectRemote } = await import( + "../../../src/altimate/workspace/detect" +) +const { cachePath, readLocalBinding, recordApprovedBinding } = await import( + "../../../src/altimate/workspace/state" +) + +// Stub AltimateApi.getCredentials / isConfigured — used by readLocalBinding +// and recordApprovedBinding for tenant/apiUrl scoping. Re-import allows +// per-test override of the module state. +import { AltimateApi } from "../../../src/altimate/api/client" +const originalIsConfigured = AltimateApi.isConfigured +const originalGetCreds = AltimateApi.getCredentials +type Creds = Awaited> +function stubCreds(tenant: string, apiUrl: string) { + ;(AltimateApi as unknown as { isConfigured: () => Promise }).isConfigured = async () => true + ;(AltimateApi as unknown as { getCredentials: () => Promise }).getCredentials = async () => + ({ + altimateInstanceName: tenant, + altimateUrl: apiUrl, + altimateApiKey: "dummy", + }) as Creds +} +function unstubCreds() { + ;(AltimateApi as unknown as { isConfigured: typeof originalIsConfigured }).isConfigured = originalIsConfigured + ;(AltimateApi as unknown as { getCredentials: typeof originalGetCreds }).getCredentials = originalGetCreds +} + +// Minimal TuiKV shim for latch tests. Reads/writes are process-local, matching +// what the plugin uses via api.kv in production. +function makeKv(): { get: (k: string, fb?: T) => T; set: (k: string, v: unknown) => void } { + const store = new Map() + return { + get: (k: string, fb?: T): T => (store.has(k) ? (store.get(k) as T) : (fb as T)), + set: (k: string, v: unknown) => { + store.set(k, v) + }, + } +} + +afterEach(() => { + unstubCreds() + // Wipe the cache file between tests so scoping tests don't bleed state. + try { + rmSync(cachePath(), { force: true }) + } catch { + /* not created by every test */ + } +}) + +// ───────────────────────────────────────────────────────────────────────────── +// projectNameFromRemote +// ───────────────────────────────────────────────────────────────────────────── + +describe("projectNameFromRemote", () => { + test("extracts repo name from HTTPS remote", () => { + expect(projectNameFromRemote("https://github.com/foo/bar.git")).toBe("bar") + }) + test("extracts repo name from SSH-form remote", () => { + expect(projectNameFromRemote("git@github.com:foo/bar.git")).toBe("bar") + }) + test("handles remote without .git suffix", () => { + expect(projectNameFromRemote("https://github.com/foo/bar")).toBe("bar") + }) + test("handles trailing slash", () => { + expect(projectNameFromRemote("https://github.com/foo/bar/")).toBe("bar") + }) + test("falls back for empty-ish inputs", () => { + expect(projectNameFromRemote("")).toBe("workspace") + }) +}) + +// ───────────────────────────────────────────────────────────────────────────── +// detectProjectRemote — thin wrapper over git; only assert graceful failure +// (the git-not-a-repo case) since happy paths would need a live repo fixture. +// ───────────────────────────────────────────────────────────────────────────── + +describe("detectProjectRemote", () => { + test("returns undefined when directory is not a git repo", () => { + // /tmp is never a git repo in a stock macOS install. + const result = detectProjectRemote(os.tmpdir()) + expect(result).toBeUndefined() + }) +}) + +// ───────────────────────────────────────────────────────────────────────────── +// Local state: cache + chmod + tenant scoping +// ───────────────────────────────────────────────────────────────────────────── + +describe("workspace binding cache", () => { + beforeEach(() => { + stubCreds("acme", "https://api.acme.example.com") + }) + + test("records and reads back a binding for the same directory + tenant", async () => { + await recordApprovedBinding("/work/proj-a", { + datamateId: 42, + datamateName: "Marketing", + repoRemote: "git@github.com:acme/proj-a.git", + linkedAt: 1_700_000_000_000, + }) + + const read = await readLocalBinding("/work/proj-a") + expect(read).not.toBeNull() + expect(read!.datamateId).toBe(42) + expect(read!.datamateName).toBe("Marketing") + }) + + test("chmods the cache file to 0o600 after write", async () => { + await recordApprovedBinding("/work/proj-a", { + datamateId: 1, + datamateName: "X", + repoRemote: "git@github.com:acme/x.git", + linkedAt: 1, + }) + expect(existsSync(cachePath())).toBe(true) + const mode = statSync(cachePath()).mode & 0o777 + expect(mode).toBe(0o600) + }) + + test("returns null when the cached tenant differs from current credentials", async () => { + await recordApprovedBinding("/work/proj-a", { + datamateId: 42, + datamateName: "Marketing", + repoRemote: "git@github.com:acme/proj-a.git", + linkedAt: 1, + }) + + // Switch account → the cached binding must not be surfaced. + unstubCreds() + stubCreds("other-tenant", "https://api.acme.example.com") + + const read = await readLocalBinding("/work/proj-a") + expect(read).toBeNull() + }) + + test("returns null when the cached apiUrl differs from current credentials", async () => { + await recordApprovedBinding("/work/proj-a", { + datamateId: 42, + datamateName: "Marketing", + repoRemote: "git@github.com:acme/proj-a.git", + linkedAt: 1, + }) + + unstubCreds() + stubCreds("acme", "https://different-host.example.com") + + const read = await readLocalBinding("/work/proj-a") + expect(read).toBeNull() + }) + + test("returns null when directory has no cached binding", async () => { + await recordApprovedBinding("/work/proj-a", { + datamateId: 42, + datamateName: "Marketing", + repoRemote: "git@github.com:acme/proj-a.git", + linkedAt: 1, + }) + const read = await readLocalBinding("/work/proj-b") + expect(read).toBeNull() + }) + + test("returns null when credentials are missing entirely", async () => { + unstubCreds() + ;(AltimateApi as unknown as { isConfigured: () => Promise }).isConfigured = async () => false + const read = await readLocalBinding("/work/proj-a") + expect(read).toBeNull() + }) +}) + +// ───────────────────────────────────────────────────────────────────────────── +// Skip latch (TuiKV shim) +// ───────────────────────────────────────────────────────────────────────────── + +describe("Skip latch", () => { + const remote = "git@github.com:acme/proj-a.git" + + test("no record → not active", () => { + const api = { kv: makeKv() } as any + expect(isSkipActive(api, remote, Date.now())).toBe(false) + }) + + test("recorded within 7 days → active", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordSkip(api, remote, now) + expect(isSkipActive(api, remote, now + 6 * 24 * 60 * 60 * 1000)).toBe(true) + }) + + test("recorded past 7 days → not active", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordSkip(api, remote, now) + expect(isSkipActive(api, remote, now + 8 * 24 * 60 * 60 * 1000)).toBe(false) + }) + + test("boundary at exactly 7 days → not active (>= rejects)", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordSkip(api, remote, now) + expect(isSkipActive(api, remote, now + 7 * 24 * 60 * 60 * 1000)).toBe(false) + }) + + test("different remotes have independent latches", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordSkip(api, "git@github.com:acme/one.git", now) + expect(isSkipActive(api, "git@github.com:acme/two.git", now)).toBe(false) + }) +}) From 76de5a93cd59ab300cf0f015ec081a373bb7fd56 Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 14 Aug 2026 02:38:14 +0530 Subject: [PATCH 02/14] fix(workspace): drop hard dependency on git remote + defer post-scan prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two user-flagged issues on the Workspaces post-scan prompt landed in 7c7e17f193: 1. Post-scan dialog raced the LLM's onboarding-menu streaming — the dialog painted while text was still generating, and Enter didn't register until streaming finished. Fix: arm a one-shot `session.idle` listener via `EventV2Bridge` from `onboarding-telemetry.ts` and publish `TuiEvent.CommandExecute` only after the session settles. Costs a few seconds of latency; kills the race. 2. `resolveProjectRemote` returned undefined for projects without a git remote (materialized sample dbt scaffolds, fresh scratch dirs), so the post-scan prompt and `altimate-code link` both bailed silently. Fix: new `resolveProjectIdentifier` in `workspace/detect.ts` always returns a `{repoRemote?, projectPath}` pair (path is symlink-resolved `realpath`). `ProjectIdentifier` type threads through `WorkspaceApi`, the TuiPlugin dialogs, and the `link` subcommand — remote is preferred when available (stronger identity, survives directory moves); path is the fallback the backend indexes symmetrically. Also: `projectNameFromPath` fallback for auto-naming (derives from directory basename when no remote); Skip-latch key hashes remote-or-path so path-only projects also get the 7-day suppression; `runFlow` and `runOnDemandPicker` reworked to use `WorkspaceApi.getBindingForProject` (tries remote first, then path); `CachedBinding` in state.ts extended with `projectPath: string | null`. Tests updated + one new latch test covers the path-only case. `bun test test/altimate/plugin/workspace.test.ts` → 18/18. --- .../altimate/plugin/onboarding-telemetry.ts | 64 ++- .../src/altimate/workspace/api-client.ts | 74 +++- .../opencode/src/altimate/workspace/detect.ts | 33 ++ .../opencode/src/altimate/workspace/state.ts | 6 +- packages/opencode/src/cli/cmd/link.ts | 246 +++++------ .../src/plugin/tui/altimate/workspace.tsx | 401 +++++++++++++----- .../test/altimate/plugin/workspace.test.ts | 37 +- 7 files changed, 583 insertions(+), 278 deletions(-) diff --git a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts index b18d579a6b..88414342b9 100644 --- a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts +++ b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts @@ -16,11 +16,53 @@ import * as OnboardingTelemetry from "../telemetry/onboarding" // EventV2 bridge the server/routes/tui.ts uses to publish TuiEvent.CommandExecute // so the workspace TuiPlugin (packages/opencode/src/plugin/tui/altimate/workspace.tsx) // runs its post-scan flow. Feature-flagged via Flag.ALTIMATE_WORKSPACE. +import { Effect } from "effect" import { Flag } from "@opencode-ai/core/flag/flag" import { AltimateApi } from "@/altimate/api/client" import { AppRuntime } from "@/effect/app-runtime" import { EventV2Bridge } from "@/event-v2-bridge" import { TuiEvent } from "@/server/tui-event" +import { Event as SessionEvent } from "@/session/status" + +/** + * Publish the workspace-postScan command AFTER the session goes idle, not on + * `project_scan`'s tool.execute.after. Rationale: project_scan tool RETURNS while + * the LLM is still generating the activation-menu text; the dialog paints in + * that window but user interactions queue behind the streaming. Waiting for + * session.idle costs a few seconds of latency but sidesteps the race entirely — + * the dialog appears once things are quiet. + * + * One-shot per sessionID: the listener unregisters as soon as the target + * session emits idle. A pending arm is dropped if a second project_scan fires + * in the same session (unlikely but handled). + */ +const pendingWorkspacePromptSessions = new Set() +let workspacePromptListenerArmed = false + +async function armWorkspacePromptOnSessionIdle(sessionID: string): Promise { + pendingWorkspacePromptSessions.add(sessionID) + if (workspacePromptListenerArmed) return + workspacePromptListenerArmed = true + await AppRuntime.runPromise( + EventV2Bridge.Service.use((events) => + events.listen((event) => + Effect.gen(function* () { + if (event.type !== SessionEvent.Idle.type) return + const sid = (event.data as { sessionID?: string } | undefined)?.sessionID + if (!sid || !pendingWorkspacePromptSessions.has(sid)) return + pendingWorkspacePromptSessions.delete(sid) + yield* events.publish(TuiEvent.CommandExecute, { + command: "altimate.workspace.postScan", + }) + }), + ), + ), + ).catch((err) => { + // eslint-disable-next-line no-console + console.error("[altimate-workspace] session-idle listener install failed:", err) + workspacePromptListenerArmed = false + }) +} // altimate_change end const ONBOARD_CONNECT = "onboard-connect" @@ -145,25 +187,11 @@ export async function OnboardingTelemetryPlugin(_input: PluginInput): Promise false))) { - void AppRuntime.runPromise( - EventV2Bridge.Service.use((events) => - events.publish(TuiEvent.CommandExecute, { - command: "altimate.workspace.postScan", - }), - ), - ).catch((err) => { - // Never block onboarding on a publish failure — worst case the - // user picks up the workspace on their next launch or via the - // /altimate.workspace.link palette command. - // eslint-disable-next-line no-console - console.error("[altimate-workspace] postScan trigger failed:", err) - }) + void armWorkspacePromptOnSessionIdle(input.sessionID) } // altimate_change end return diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 8bf859f0a8..8078bd2cf5 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -23,10 +23,20 @@ export interface Binding { id: number datamate_id: number datamate_name: string - repo_remote: string + /** Either ``repo_remote`` OR ``project_path`` is populated (at least one). */ + repo_remote: string | null + project_path: string | null created_at?: string } +/** Project identifier passed to create/bind endpoints. At least one field is + * required by the backend's CHECK constraint; the CLI's resolveProjectIdentifier + * always populates ``projectPath`` and populates ``repoRemote`` when available. */ +export interface ProjectIdentifier { + repoRemote?: string + projectPath?: string +} + export interface CreateAndBindResponse { datamate: DatamateRef binding: Binding @@ -47,6 +57,7 @@ export interface ConflictDetail { existing_datamate_id?: number existing_datamate_name?: string | null repo_remote?: string + project_path?: string } export interface PreconditionDetail { @@ -170,7 +181,7 @@ async function req( } export namespace WorkspaceApi { - /** Server-authoritative pre-check: is this remote already bound in this tenant? */ + /** Server-authoritative pre-check by git remote. Returns null on 404. */ export async function getBindingForRemote(remote: string): Promise { try { return await req("GET", "/by-remote", { query: { repo_remote: remote } }) @@ -180,23 +191,56 @@ export namespace WorkspaceApi { } } + /** Symmetric pre-check by absolute project directory path (for projects + * without a git remote). Returns null on 404. */ + export async function getBindingForPath(projectPath: string): Promise { + try { + return await req("GET", "/by-path", { query: { project_path: projectPath } }) + } catch (err) { + if (err instanceof NotFoundError) return null + throw err + } + } + + /** Tries remote first (stronger identity), then path. Returns the first hit + * or null. Both fields on the identifier are optional but at least one must + * be present. */ + export async function getBindingForProject(id: ProjectIdentifier): Promise { + if (id.repoRemote) { + const hit = await getBindingForRemote(id.repoRemote) + if (hit) return hit + } + if (id.projectPath) { + return await getBindingForPath(id.projectPath) + } + return null + } + export async function createAndBind(input: { name: string - repoRemote: string + identifier: ProjectIdentifier description?: string }): Promise { return req("POST", "/", { body: { name: input.name, - repo_remote: input.repoRemote, + repo_remote: input.identifier.repoRemote ?? null, + project_path: input.identifier.projectPath ?? null, description: input.description ?? null, }, }) } - export async function bindExisting(datamateId: number, remote: string): Promise { + export async function bindExisting( + datamateId: number, + identifier: ProjectIdentifier, + ): Promise { return req("POST", "/bind", { - body: { datamate_id: datamateId, repo_remote: remote }, + body: { + datamate_id: datamateId, + repo_remote: identifier.repoRemote ?? null, + project_path: identifier.projectPath ?? null, + }, }) } @@ -216,6 +260,24 @@ export namespace WorkspaceApi { }) } + /** Path-identified rebind — symmetric to ``rebindByRemote`` for projects + * without a git remote. */ + export async function rebindByPath(input: { + projectPath: string + targetDatamateId: number + expectedCurrentDatamateId?: number + }): Promise { + return req("PUT", "/by-path", { + body: { + project_path: input.projectPath, + target_datamate_id: input.targetDatamateId, + ...(input.expectedCurrentDatamateId !== undefined + ? { expected_current_datamate_id: input.expectedCurrentDatamateId } + : {}), + }, + }) + } + /** Populates the "link to existing workspace" picker. Reuses the existing * /datamates/ list endpoint on the datamates_router. */ export async function listDatamates(): Promise { diff --git a/packages/opencode/src/altimate/workspace/detect.ts b/packages/opencode/src/altimate/workspace/detect.ts index 5b461e52de..b731c36480 100644 --- a/packages/opencode/src/altimate/workspace/detect.ts +++ b/packages/opencode/src/altimate/workspace/detect.ts @@ -8,6 +8,8 @@ // basic-auth (e.g. `https://x-access-token:ghp_xxx@github.com/...`) never // reaches the server or the local cache in clear. import { spawnSync } from "node:child_process" +import { realpathSync } from "node:fs" +import path from "node:path" import { stripGitRemoteCredentials } from "../tools/project-scan" export function detectProjectRemote(directory: string): string | undefined { @@ -24,6 +26,30 @@ export function detectProjectRemote(directory: string): string | undefined { } } +/** Project identity for the binding system. Prefers ``repo_remote`` when the + * project has a git remote (stronger identity — survives directory moves); falls + * back to ``project_path`` (absolute, symlink-resolved directory path) for + * projects without one (materialized sample scaffolds, fresh scratch dirs). + * + * Both fields can be populated simultaneously; callers pick which to use for + * lookup or send both to create/bind (server uses whichever it needs for its + * partial unique index). At least one field is always populated — falling back + * to the raw ``directory`` argument keeps the "no remote AND unresolvable path" + * degenerate case from returning empty. */ +export function resolveProjectIdentifier(directory: string): { + repoRemote?: string + projectPath: string +} { + const repoRemote = detectProjectRemote(directory) + let projectPath: string + try { + projectPath = realpathSync(path.resolve(directory)) + } catch { + projectPath = path.resolve(directory) + } + return repoRemote ? { repoRemote, projectPath } : { projectPath } +} + /** Sensible default workspace name from a remote URL — best-effort. Used to * prefill the workspace-name prompt in the CreateDialog and `altimate link`. * ``github.com/foo/bar.git`` → ``bar`` ; ``git@github.com:foo/bar.git`` → ``bar``. */ @@ -32,3 +58,10 @@ export function projectNameFromRemote(remote: string): string { const parts = trimmed.split(/[/:]/) return parts[parts.length - 1] || "workspace" } + +/** Fallback name source when the project has no git remote — uses the directory's + * basename (e.g. ``/Users/x/sample-dbt-project`` → ``sample-dbt-project``). */ +export function projectNameFromPath(projectPath: string): string { + const base = path.basename(projectPath.replace(/\/$/, "")) + return base || "workspace" +} diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 09b559e3ad..477a472b0a 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -24,7 +24,11 @@ const log = Log.create({ service: "altimate-workspace-state" }) export interface CachedBinding { datamateId: number datamateName: string - repoRemote: string + /** Either ``repoRemote`` or ``projectPath`` is populated (at least one). + * Mirrors the server-side binding row, which is identified by whichever + * fields it has. */ + repoRemote: string | null + projectPath: string | null linkedAt: number } diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 288ee3acfe..434e1306bd 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -1,13 +1,15 @@ // altimate_change - new file // -// On-demand "link this project to a workspace" subcommand. Runs the same three- -// way flow the TuiPlugin (packages/opencode/src/plugin/tui/altimate/workspace.tsx) -// offers post-scan, but outside a TUI session — so a user who skipped the -// post-scan offer (or skipped it 7+ days ago and wants to link now) has a -// path back in. +// On-demand "link this project to a workspace" subcommand. User invoked it +// explicitly, so we skip the Create/Link/Skip funnel the post-scan trigger +// uses and jump straight to a picker over the user's workspaces — the +// currently-linked one is marked, and "＋ Create a new workspace" is the +// first row. New workspaces are auto-named from the git repo (or directory +// name for path-only projects) so the user never has to type anything. // -// Deliberately shares the WorkspaceApi + state modules with the plugin so the -// two entry points can't drift on request shape or error handling. +// Deliberately shares the WorkspaceApi + state + detect modules with the +// TuiPlugin so the two entry points can't drift on request shape, project +// identity, or error handling. import { cmd } from "./cmd" import { UI } from "../ui" import * as prompts from "@clack/prompts" @@ -21,10 +23,18 @@ import { NotFoundError, PreconditionFailedError, type DatamateRef, + type GetBindingResponse, + type ProjectIdentifier, } from "@/altimate/workspace/api-client" -import { detectProjectRemote, projectNameFromRemote } from "@/altimate/workspace/detect" +import { + projectNameFromPath, + projectNameFromRemote, + resolveProjectIdentifier, +} from "@/altimate/workspace/detect" import { recordApprovedBinding } from "@/altimate/workspace/state" +const CREATE_NEW_SENTINEL = "__create_new__" + export const LinkCommand = cmd({ command: "link", describe: "Link this project to an Altimate workspace", @@ -36,8 +46,6 @@ export const LinkCommand = cmd({ default: process.cwd(), }), handler: async (args) => { - // Bail early if the CLI has no Altimate credentials — there's no place to - // send them and no gateway to authenticate against. if (!(await AltimateApi.isConfigured())) { UI.error( "Not signed in to Altimate. Run the TUI (altimate-code) and sign in first, then re-run `altimate-code link`.", @@ -46,22 +54,17 @@ export const LinkCommand = cmd({ return } - const remote = detectProjectRemote(args.directory) - if (!remote) { - UI.error( - `No git remote found in ${args.directory}. A workspace is bound to a project via its git remote — initialize a repo and add a remote first.`, - ) - process.exitCode = 1 - return - } + const identifier = resolveProjectIdentifier(args.directory) - prompts.intro("Link workspace") - prompts.log.info(`Project remote: ${remote}`) + prompts.intro("Link this project to a workspace") + if (identifier.repoRemote) prompts.log.info(`Project remote: ${identifier.repoRemote}`) + else prompts.log.info(`Project path: ${identifier.projectPath} (no git remote)`) - // Server-authoritative pre-check. - let existing: Awaited> = null + // Pre-check for the currently-linked marker + workspace list. Both are + // fetched up-front so the picker can annotate the current binding. + let existing: GetBindingResponse | null = null try { - existing = await WorkspaceApi.getBindingForRemote(remote) + existing = await WorkspaceApi.getBindingForProject(identifier) } catch (err) { if (err instanceof NotConfiguredError) { UI.error(err.message) @@ -69,92 +72,95 @@ export const LinkCommand = cmd({ return } prompts.log.warn( - `Could not reach the workspace service (${err instanceof Error ? err.message : String(err)}). Continuing anyway.`, + `Could not reach the workspace service to look up existing bindings (${err instanceof Error ? err.message : String(err)}). Continuing without the currently-linked marker.`, ) } - if (existing) { - const choice = await prompts.select<"attach" | "relink" | "cancel">({ - message: `This project is already linked to workspace "${existing.datamate.name}".`, - options: [ - { value: "attach", label: "Keep this binding (no changes)" }, - { value: "relink", label: "Re-link to a different workspace" }, - { value: "cancel", label: "Cancel" }, - ], - initialValue: "attach", - }) - if (prompts.isCancel(choice) || choice === "cancel" || choice === "attach") { - prompts.outro(choice === "attach" ? `Kept "${existing.datamate.name}".` : "No changes.") - return - } - await pickAndBind(remote, "relink", existing.datamate.id, args.directory) + const spin = prompts.spinner() + spin.start("Loading workspaces...") + let list: DatamateRef[] + try { + list = await WorkspaceApi.listDatamates() + } catch (err) { + spin.stop("Could not load workspaces.", 1) + prompts.log.error(err instanceof Error ? err.message : String(err)) + process.exitCode = 1 return } + spin.stop(`Found ${list.length} workspace${list.length === 1 ? "" : "s"}.`) + + const autoName = identifier.repoRemote + ? projectNameFromRemote(identifier.repoRemote) + : projectNameFromPath(identifier.projectPath) + const currentId = existing?.datamate.id + const currentName = existing?.datamate.name - const choice = await prompts.select<"create" | "link" | "cancel">({ - message: "Set up a workspace for this project?", - options: [ - { - value: "create", - label: "Create a new workspace", - hint: "Opens a browser to configure integrations and knowledge.", - }, - { - value: "link", - label: "Link to an existing workspace", - hint: "Attach this project to a workspace you already own.", - }, - { value: "cancel", label: "Cancel" }, - ], - initialValue: "create", + const options: Array<{ value: string; label: string; hint?: string }> = [ + { + value: CREATE_NEW_SENTINEL, + label: `＋ Create a new workspace "${autoName}"`, + hint: "Named from this project; rename in the SaaS after.", + }, + ...list.map((dm) => ({ + value: String(dm.id), + label: dm.id === currentId ? `● ${dm.name}` : ` ${dm.name}`, + hint: dm.id === currentId ? "currently linked here" : undefined, + })), + ] + + const pick = await prompts.select({ + message: existing + ? `Currently linked to "${currentName}". 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, }) - if (prompts.isCancel(choice) || choice === "cancel") { - prompts.outro("No workspace was linked.") + + if (prompts.isCancel(pick)) { + prompts.outro("No changes.") + return + } + + if (pick === CREATE_NEW_SENTINEL) { + await createAndBind(identifier, autoName, args.directory) return } - if (choice === "create") { - await createAndBind(remote, args.directory) + const targetId = Number(pick) + if (targetId === currentId) { + prompts.outro(`Kept "${currentName}" — nothing changed.`) return } - await pickAndBind(remote, "attach", undefined, args.directory) + + await bindOrRebind(identifier, targetId, currentId, args.directory) }, }) -async function createAndBind(remote: string, directory: string): Promise { - const defaultName = projectNameFromRemote(remote) - const nameInput = await prompts.text({ - message: "Name this workspace", - placeholder: defaultName, - defaultValue: defaultName, - }) - if (prompts.isCancel(nameInput)) { - prompts.outro("No workspace was created.") - return - } - const name = String(nameInput).trim() || defaultName - +async function createAndBind( + identifier: ProjectIdentifier, + name: string, + directory: string, +): Promise { const spin = prompts.spinner() spin.start(`Creating workspace "${name}"...`) try { - const res = await WorkspaceApi.createAndBind({ name, repoRemote: remote }) + const res = await WorkspaceApi.createAndBind({ name, identifier }) await recordApprovedBinding(directory, { datamateId: res.datamate.id, datamateName: res.datamate.name, repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, linkedAt: Date.now(), }) spin.stop(`Workspace "${res.datamate.name}" created and linked.`) prompts.log.info(`Manage it at: ${res.manage_url}`) - // Best-effort browser open — never blocks. If it fails the user has the - // URL above to copy manually. await open(res.manage_url).catch(() => undefined) prompts.outro("Done.") } catch (err) { spin.stop("Failed to create workspace.", 1) if (err instanceof ConflictError) { prompts.log.error( - `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` and pick "Re-link" if you want to move it.`, + `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` to switch.`, ) } else { prompts.log.error(err instanceof Error ? err.message : String(err)) @@ -163,75 +169,57 @@ async function createAndBind(remote: string, directory: string): Promise { } } -async function pickAndBind( - remote: string, - mode: "attach" | "relink", - expectedCurrentDatamateId: number | undefined, +async function bindOrRebind( + identifier: ProjectIdentifier, + targetDatamateId: number, + currentDatamateId: number | undefined, directory: string, ): Promise { + const isRebind = currentDatamateId !== undefined const spin = prompts.spinner() - spin.start("Loading workspaces...") - let list: DatamateRef[] - try { - list = await WorkspaceApi.listDatamates() - } catch (err) { - spin.stop("Could not load workspaces.", 1) - prompts.log.error(err instanceof Error ? err.message : String(err)) - process.exitCode = 1 - return - } - spin.stop(`Found ${list.length} workspace${list.length === 1 ? "" : "s"}.`) - - if (list.length === 0) { - prompts.log.warn( - "You don't have any workspaces yet. Re-run and pick \"Create a new workspace\" instead.", - ) - prompts.outro("No workspace to link.") - return - } - - const pick = await prompts.select({ - message: mode === "attach" ? "Pick a workspace to attach to" : "Pick a workspace to re-link to", - options: [ - ...list.map((dm) => ({ value: dm.id as number | "cancel", label: dm.name })), - { value: "cancel" as const, label: "Cancel" }, - ], - }) - if (prompts.isCancel(pick) || pick === "cancel") { - prompts.outro("No changes.") - return - } - - const target = list.find((dm) => dm.id === pick)! - const spin2 = prompts.spinner() - spin2.start(`${mode === "attach" ? "Linking" : "Re-linking"} to "${target.name}"...`) + spin.start(isRebind ? `Re-linking to workspace...` : `Linking to workspace...`) try { - const res = - mode === "attach" - ? await WorkspaceApi.bindExisting(pick, remote) - : await WorkspaceApi.rebindByRemote({ - remote, - targetDatamateId: pick, - expectedCurrentDatamateId, - }) + const res = await (async () => { + if (isRebind) { + // Rebind endpoint depends on which identifier is present. Prefer remote + // (stronger identity — survives directory moves); fall back to path. + return identifier.repoRemote + ? WorkspaceApi.rebindByRemote({ + remote: identifier.repoRemote, + targetDatamateId, + expectedCurrentDatamateId: currentDatamateId, + }) + : WorkspaceApi.rebindByPath({ + projectPath: identifier.projectPath!, + targetDatamateId, + expectedCurrentDatamateId: currentDatamateId, + }) + } + return WorkspaceApi.bindExisting(targetDatamateId, identifier) + })() await recordApprovedBinding(directory, { datamateId: res.binding.datamate_id, datamateName: res.binding.datamate_name, repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, linkedAt: Date.now(), }) - spin2.stop(`${mode === "attach" ? "Linked" : "Re-linked"} to "${res.binding.datamate_name}".`) + spin.stop( + isRebind + ? `Re-linked to "${res.binding.datamate_name}".` + : `Linked to "${res.binding.datamate_name}".`, + ) prompts.outro("Done.") } catch (err) { - spin2.stop(`${mode === "attach" ? "Link" : "Re-link"} failed.`, 1) + spin.stop(isRebind ? `Re-link failed.` : `Link failed.`, 1) if (err instanceof ConflictError) { prompts.log.error( - `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` and pick "Re-link".`, + `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}".`, ) } else if (err instanceof PreconditionFailedError) { - prompts.log.error("Someone else re-linked this project — reload and try again.") + prompts.log.error("Someone else re-linked this project — re-run and try again.") } else if (err instanceof NotFoundError) { - prompts.log.error("No existing binding to re-link. Try again and pick Create or Link.") + prompts.log.error("No existing binding to re-link. Re-run and pick again.") } else if (err instanceof ForbiddenError) { prompts.log.error("Only the workspace owner can attach projects to it.") } else { diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 47f00b6e8f..cfdc26d985 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -25,7 +25,7 @@ import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import { createHash } from "node:crypto" import open from "open" -import { createSignal, onMount, Show } from "solid-js" +import { createSignal, onMount } from "solid-js" import { ConflictError, ForbiddenError, @@ -34,8 +34,13 @@ import { WorkspaceApi, type DatamateRef, type GetBindingResponse, + type ProjectIdentifier, } from "@/altimate/workspace/api-client" -import { detectProjectRemote, projectNameFromRemote } from "@/altimate/workspace/detect" +import { + projectNameFromPath, + projectNameFromRemote, + resolveProjectIdentifier, +} from "@/altimate/workspace/detect" import { readLocalBinding, recordApprovedBinding } from "@/altimate/workspace/state" import { Log } from "@/altimate/util/log" @@ -52,20 +57,21 @@ const log = Log.create({ service: "altimate-workspace" }) const SKIP_TTL_MS = 7 * 24 * 60 * 60 * 1000 const KV_SKIP_PREFIX = "altimate.workspace.postScan.skip." -function skipKey(remote: string): string { - // Hash to keep the KV keyspace bounded and avoid embedding a URL (which can - // contain userinfo even after scrubbing edge cases) into a persisted key. - return KV_SKIP_PREFIX + createHash("sha1").update(remote).digest("hex") +/** Latch key from the project's primary identifier (remote > path). Path-only + * projects also get a latch — sample-scaffold users are still users. */ +function skipKey(id: ProjectIdentifier): string { + const primary = id.repoRemote ?? id.projectPath ?? "" + return KV_SKIP_PREFIX + createHash("sha1").update(primary).digest("hex") } -function isSkipActive(api: TuiPluginApi, remote: string, nowMs: number): boolean { - const rec = api.kv.get<{ skippedAt: number }>(skipKey(remote)) +function isSkipActive(api: TuiPluginApi, id: ProjectIdentifier, nowMs: number): boolean { + const rec = api.kv.get<{ skippedAt: number }>(skipKey(id)) if (!rec || typeof rec.skippedAt !== "number") return false return nowMs - rec.skippedAt < SKIP_TTL_MS } -function recordSkip(api: TuiPluginApi, remote: string, nowMs: number): void { - api.kv.set(skipKey(remote), { skippedAt: nowMs }) +function recordSkip(api: TuiPluginApi, id: ProjectIdentifier, nowMs: number): void { + api.kv.set(skipKey(id), { skippedAt: nowMs }) } // ───────────────────────────────────────────────────────────────────────────── @@ -76,20 +82,21 @@ function recordSkip(api: TuiPluginApi, remote: string, nowMs: number): void { interface OfferProps { api: TuiPluginApi - remote: string + identifier: ProjectIdentifier defaultName: string suppressLatch?: boolean // altimate link on-demand skips the Skip latch } function OfferDialog(props: OfferProps) { + const identLabel = () => props.identifier.repoRemote ?? props.identifier.projectPath ?? "this project" return ( { if (option.value === "skip") { - if (!props.suppressLatch) recordSkip(props.api, props.remote, Date.now()) + if (!props.suppressLatch) recordSkip(props.api, props.identifier, Date.now()) props.api.ui.dialog.clear() return } if (option.value === "create") { - props.api.ui.dialog.replace(() => ( - - )) + // Auto-name from git repo — no name prompt. The SaaS UI is the place to + // rename / configure; the CLI's job is just to establish the binding. + void createAndBindInline(props.api, props.identifier, props.defaultName) return } - // link → picker + // link → picker (fresh-project attach path) props.api.ui.dialog.replace(() => ( - + )) }} /> ) } +async function createAndBindInline( + api: TuiPluginApi, + identifier: ProjectIdentifier, + name: string, +): Promise { + api.ui.dialog.clear() + try { + const res = await WorkspaceApi.createAndBind({ name, identifier }) + await recordApprovedBinding(api.state.path.directory, { + datamateId: res.datamate.id, + datamateName: res.datamate.name, + repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, + linkedAt: Date.now(), + }) + try { + await open(res.manage_url) + api.ui.toast({ + variant: "success", + message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, + }) + } catch { + api.ui.toast({ + variant: "info", + message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, + duration: 10_000, + }) + } + } catch (err) { + if (err instanceof ConflictError) { + 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.`, + }) + } else { + api.ui.toast({ + variant: "error", + message: err instanceof Error ? err.message : "Failed to create workspace", + }) + } + } +} + interface AlreadyLinkedProps { api: TuiPluginApi - remote: string + identifier: ProjectIdentifier workspaceName: string workspaceId: number hasDrift: boolean @@ -140,7 +190,8 @@ function AlreadyLinkedDialog(props: AlreadyLinkedProps) { // it puts the critical info in the user's field of view before they pick. const title = () => { const parts: string[] = [`Project is linked to workspace "${props.workspaceName}"`] - if (props.hasDrift && props.driftedWas) parts.push(`(was ${props.driftedWas}, now ${props.remote})`) + const now = props.identifier.repoRemote ?? props.identifier.projectPath + if (props.hasDrift && props.driftedWas) parts.push(`(was ${props.driftedWas}, now ${now})`) if (props.unverified) parts.push("(⚠ unverified — server unreachable, showing cached value)") return parts.join(" ") } @@ -175,7 +226,7 @@ function AlreadyLinkedDialog(props: AlreadyLinkedProps) { props.api.ui.dialog.replace(() => ( @@ -187,7 +238,7 @@ function AlreadyLinkedDialog(props: AlreadyLinkedProps) { interface PickerProps { api: TuiPluginApi - remote: string + identifier: ProjectIdentifier mode: "attach" | "relink" expectedCurrentDatamateId?: number } @@ -211,11 +262,12 @@ function PickerDialog(props: PickerProps) { async function pick(datamateId: number) { try { if (props.mode === "attach") { - const res = await WorkspaceApi.bindExisting(datamateId, props.remote) + const res = await WorkspaceApi.bindExisting(datamateId, props.identifier) await recordApprovedBinding(props.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(), }) props.api.ui.toast({ @@ -223,15 +275,25 @@ function PickerDialog(props: PickerProps) { message: `Linked to workspace "${res.binding.datamate_name}".`, }) } else { - const res = await WorkspaceApi.rebindByRemote({ - remote: props.remote, - targetDatamateId: datamateId, - expectedCurrentDatamateId: props.expectedCurrentDatamateId, - }) + // Rebind: pick the endpoint that matches the identifier we have. + // Prefer remote (stronger identity) when available; else fall back to + // path — matches the pre-check ordering in getBindingForProject. + const res = props.identifier.repoRemote + ? await WorkspaceApi.rebindByRemote({ + remote: props.identifier.repoRemote, + targetDatamateId: datamateId, + expectedCurrentDatamateId: props.expectedCurrentDatamateId, + }) + : await WorkspaceApi.rebindByPath({ + projectPath: props.identifier.projectPath!, + targetDatamateId: datamateId, + expectedCurrentDatamateId: props.expectedCurrentDatamateId, + }) await recordApprovedBinding(props.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(), }) props.api.ui.toast({ @@ -292,106 +354,210 @@ function PickerDialog(props: PickerProps) { ) } -interface CreateProps { +// Sentinel value for the "＋ Create a new workspace" row in the on-demand +// picker. Negative so it can't collide with any real datamate id (SERIAL PK). +const CREATE_NEW_SENTINEL = -2 + +interface OnDemandPickerProps { api: TuiPluginApi + identifier: ProjectIdentifier + currentlyLinkedDatamateId?: number + currentlyLinkedDatamateName?: string defaultName: string - remote: string } -function CreateDialog(props: CreateProps) { - const [busy, setBusy] = createSignal(false) - const [error, setError] = createSignal(null) +/** Picker-first flow for the on-demand `altimate.workspace.link` command. + * + * Skips the Create/Link/Skip funnel — the user already opted in by invoking + * the palette. Immediately lists workspaces; currently-linked one is marked; + * "＋ Create a new workspace" is the first row. No Skip option (user chose to + * be here). Auto-names any new workspace from the git repo. */ +function OnDemandPickerDialog(props: OnDemandPickerProps) { + const [datamates, setDatamates] = createSignal(null) + + onMount(async () => { + try { + const list = await WorkspaceApi.listDatamates() + setDatamates(list) + } catch (err) { + props.api.ui.toast({ + variant: "error", + message: err instanceof Error ? err.message : "Failed to load workspaces", + }) + props.api.ui.dialog.clear() + } + }) + + const options = () => { + const list = datamates() + if (!list) return [{ title: "Loading workspaces...", value: -1, disabled: true }] + // Deliberately short titles + hint in description so the dialog's narrow + // width doesn't truncate either. The "●" marker stays in the title (single + // char, cheap) so the currently-linked row is scannable at a glance. + return [ + { + title: "＋ Create a new workspace", + value: CREATE_NEW_SENTINEL, + description: `Auto-named "${props.defaultName}" from this repo — rename in the SaaS.`, + }, + ...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, + })), + ] + } + return ( - ( - - Name this workspace (defaults to your project name): - You can add integrations, knowledge, and guardrails in the browser after. - - {error()!} - - - )} - onConfirm={async (value) => { - if (busy()) return - const name = (value || "").trim() || props.defaultName || "Untitled Workspace" - setBusy(true) - setError(null) - try { - const res = await WorkspaceApi.createAndBind({ - name, - repoRemote: props.remote, - }) - await recordApprovedBinding(props.api.state.path.directory, { - datamateId: res.datamate.id, - datamateName: res.datamate.name, - repoRemote: res.binding.repo_remote, - linkedAt: Date.now(), + + title="Link this project to a workspace" + options={options()} + current={props.currentlyLinkedDatamateId ?? CREATE_NEW_SENTINEL} + onSelect={(option) => { + if (option.value === -1) { + props.api.ui.dialog.clear() + return + } + if (option.value === CREATE_NEW_SENTINEL) { + void createAndBindInline(props.api, props.identifier, props.defaultName) + return + } + // Picked an existing workspace. + if (option.value === props.currentlyLinkedDatamateId) { + // No-op — user picked the workspace this project is already linked to. + props.api.ui.toast({ + variant: "info", + message: `Already linked to "${props.currentlyLinkedDatamateName}" — nothing changed.`, }) - // Best-effort browser open. On failure, fall back to a toast with the - // URL so the user has a copy-pasteable path forward — silent failure - // was flagged by codex round 1. - try { - await open(res.manage_url) - props.api.ui.toast({ - variant: "success", - message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, - }) - } catch { - props.api.ui.toast({ - variant: "info", - message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} in your browser to configure it.`, - duration: 10_000, - }) - } props.api.ui.dialog.clear() - } catch (err) { - if (err instanceof ConflictError) { - setError( - `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Cancel and pick "Re-link" from the offer if you want to move it.`, - ) - } else { - setError(err instanceof Error ? err.message : "Failed to create workspace") - } - } finally { - setBusy(false) + return } + void bindOrRebindInline( + props.api, + props.identifier, + option.value, + props.currentlyLinkedDatamateId, + ) }} - onCancel={() => props.api.ui.dialog.clear()} /> ) } +async function bindOrRebindInline( + api: TuiPluginApi, + identifier: ProjectIdentifier, + targetDatamateId: number, + currentDatamateId: number | undefined, +): Promise { + api.ui.dialog.clear() + try { + const res = await (async () => { + if (currentDatamateId) { + // Rebind: use whichever identifier we have; remote-first for identity. + return identifier.repoRemote + ? WorkspaceApi.rebindByRemote({ + remote: identifier.repoRemote, + targetDatamateId, + expectedCurrentDatamateId: currentDatamateId, + }) + : WorkspaceApi.rebindByPath({ + projectPath: identifier.projectPath!, + targetDatamateId, + expectedCurrentDatamateId: currentDatamateId, + }) + } + return WorkspaceApi.bindExisting(targetDatamateId, identifier) + })() + 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(), + }) + api.ui.toast({ + variant: "success", + message: currentDatamateId + ? `Re-linked to workspace "${res.binding.datamate_name}".` + : `Linked to workspace "${res.binding.datamate_name}".`, + }) + } catch (err) { + let msg: string + if (err instanceof ConflictError) { + msg = `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}".` + } else if (err instanceof PreconditionFailedError) { + msg = "Someone else re-linked this project — reload and try again." + } else if (err instanceof NotFoundError) { + msg = "No existing binding to re-link. Try again." + } else if (err instanceof ForbiddenError) { + msg = "Only the workspace owner can attach projects to it." + } else { + msg = err instanceof Error ? err.message : "Failed to link workspace" + } + api.ui.toast({ variant: "error", message: msg }) + } +} + // ───────────────────────────────────────────────────────────────────────────── -// Flow orchestrator — runs on every `altimate.workspace.postScan` command -// dispatch AND on every `altimate.workspace.link` command dispatch. Idempotent -// re-entry: if a dialog is already up, the mount replaces it (that's fine — -// two rapid scan completions collapse to one visible offer). +// Flow orchestrators. +// +// `runFlow` — the post-scan trigger flow. Three-way Create/Link/Skip funnel +// for a user we're prompting FROM ZERO (they haven't opted in). Skip is a +// first-class outcome; Create branch auto-names. +// +// `runOnDemandPicker` — the palette / `/altimate.workspace.link` flow. User +// already opted in by invoking, so no Skip. Fetches the workspace list + +// pre-check binding, opens the picker with the currently-linked one marked. // ───────────────────────────────────────────────────────────────────────────── +async function runOnDemandPicker(api: TuiPluginApi, directory: string): Promise { + const identifier = resolveProjectIdentifier(directory) + // Pre-check for the currently-linked marker. Failures are non-fatal — we + // still show the picker without the "(currently linked here)" annotation. + let existing: GetBindingResponse | null = null + try { + existing = await WorkspaceApi.getBindingForProject(identifier) + } catch (err) { + log.warn("on-demand picker pre-check failed", { + err: err instanceof Error ? err.message : String(err), + }) + } + const defaultName = identifier.repoRemote + ? projectNameFromRemote(identifier.repoRemote) + : projectNameFromPath(identifier.projectPath) + api.ui.dialog.replace(() => ( + + )) +} + async function runFlow( api: TuiPluginApi, directory: string, opts: { suppressLatch?: boolean } = {}, ): Promise { - const remote = detectProjectRemote(directory) - if (!remote) { - // Not a git project (or git failed) — nothing to bind to. Silent per spec: - // the ticket says "handle failures without blocking the existing flow". - return - } - if (!opts.suppressLatch && isSkipActive(api, remote, Date.now())) { - log.info("workspace prompt suppressed by 7-day Skip latch", { remote }) + const identifier = resolveProjectIdentifier(directory) + // Path is always populated by resolveProjectIdentifier — projects without a + // git remote (sample dbt scaffolds, scratch dirs) still get a binding offer. + if (!opts.suppressLatch && isSkipActive(api, identifier, Date.now())) { + log.info("workspace prompt suppressed by 7-day Skip latch", { + identifier: identifier.repoRemote ?? identifier.projectPath, + }) return } + const defaultName = identifier.repoRemote + ? projectNameFromRemote(identifier.repoRemote) + : projectNameFromPath(identifier.projectPath) + let serverBinding: GetBindingResponse | null | undefined try { - serverBinding = await WorkspaceApi.getBindingForRemote(remote) + serverBinding = await WorkspaceApi.getBindingForProject(identifier) } catch (err) { log.warn("workspace pre-check server call failed, falling back to local cache", { err: err instanceof Error ? err.message : String(err), @@ -404,13 +570,14 @@ async function runFlow( await recordApprovedBinding(directory, { datamateId: serverBinding.datamate.id, datamateName: serverBinding.datamate.name, - repoRemote: remote, + repoRemote: serverBinding.binding.repo_remote, + projectPath: serverBinding.binding.project_path, linkedAt: Date.now(), }) api.ui.dialog.replace(() => ( ( )) @@ -435,14 +602,19 @@ async function runFlow( // Server unreachable — fall back to the local cache (marked as unverified). const local = await readLocalBinding(directory) if (local) { + // Drift = the identifier the cache remembers differs from what we're + // seeing now. Compare on the field that's actually populated in the cache. + const cachedIdent = local.repoRemote ?? local.projectPath ?? "" + const currentIdent = identifier.repoRemote ?? identifier.projectPath + const hasDrift = cachedIdent !== currentIdent api.ui.dialog.replace(() => ( )) @@ -457,8 +629,8 @@ async function runFlow( api.ui.dialog.replace(() => ( )) @@ -486,8 +658,9 @@ const tui: TuiPlugin = async (api) => { category: "Altimate", namespace: "palette", run() { - // User-initiated → bypass the Skip latch. - void runFlow(api, api.state.path.directory, { suppressLatch: true }) + // User-initiated → jump straight to picker (currently-linked marked, + // "＋ Create new" as the first row). No Skip funnel — they invoked. + void runOnDemandPicker(api, api.state.path.directory) }, }, ], diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index 4ba5328b2b..7749636ed1 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -118,6 +118,7 @@ describe("workspace binding cache", () => { datamateId: 42, datamateName: "Marketing", repoRemote: "git@github.com:acme/proj-a.git", + projectPath: "/work/proj-a", linkedAt: 1_700_000_000_000, }) @@ -132,6 +133,7 @@ describe("workspace binding cache", () => { datamateId: 1, datamateName: "X", repoRemote: "git@github.com:acme/x.git", + projectPath: "/work/proj-a", linkedAt: 1, }) expect(existsSync(cachePath())).toBe(true) @@ -144,6 +146,7 @@ describe("workspace binding cache", () => { datamateId: 42, datamateName: "Marketing", repoRemote: "git@github.com:acme/proj-a.git", + projectPath: "/work/proj-a", linkedAt: 1, }) @@ -160,6 +163,7 @@ describe("workspace binding cache", () => { datamateId: 42, datamateName: "Marketing", repoRemote: "git@github.com:acme/proj-a.git", + projectPath: "/work/proj-a", linkedAt: 1, }) @@ -175,6 +179,7 @@ describe("workspace binding cache", () => { datamateId: 42, datamateName: "Marketing", repoRemote: "git@github.com:acme/proj-a.git", + projectPath: "/work/proj-a", linkedAt: 1, }) const read = await readLocalBinding("/work/proj-b") @@ -194,38 +199,50 @@ describe("workspace binding cache", () => { // ───────────────────────────────────────────────────────────────────────────── describe("Skip latch", () => { - const remote = "git@github.com:acme/proj-a.git" + const ident = { repoRemote: "git@github.com:acme/proj-a.git", projectPath: "/work/proj-a" } test("no record → not active", () => { const api = { kv: makeKv() } as any - expect(isSkipActive(api, remote, Date.now())).toBe(false) + expect(isSkipActive(api, ident, Date.now())).toBe(false) }) test("recorded within 7 days → active", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, remote, now) - expect(isSkipActive(api, remote, now + 6 * 24 * 60 * 60 * 1000)).toBe(true) + recordSkip(api, ident, now) + expect(isSkipActive(api, ident, now + 6 * 24 * 60 * 60 * 1000)).toBe(true) }) test("recorded past 7 days → not active", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, remote, now) - expect(isSkipActive(api, remote, now + 8 * 24 * 60 * 60 * 1000)).toBe(false) + recordSkip(api, ident, now) + expect(isSkipActive(api, ident, now + 8 * 24 * 60 * 60 * 1000)).toBe(false) }) test("boundary at exactly 7 days → not active (>= rejects)", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, remote, now) - expect(isSkipActive(api, remote, now + 7 * 24 * 60 * 60 * 1000)).toBe(false) + recordSkip(api, ident, now) + expect(isSkipActive(api, ident, now + 7 * 24 * 60 * 60 * 1000)).toBe(false) }) test("different remotes have independent latches", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, "git@github.com:acme/one.git", now) - expect(isSkipActive(api, "git@github.com:acme/two.git", now)).toBe(false) + recordSkip(api, { repoRemote: "git@github.com:acme/one.git", projectPath: "/w/one" }, now) + expect( + isSkipActive(api, { repoRemote: "git@github.com:acme/two.git", projectPath: "/w/two" }, now), + ).toBe(false) + }) + + test("path-only projects (no remote) also get a latch — key derives from path", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + const pathOnly = { projectPath: "/scratch/sample-dbt" } + recordSkip(api, pathOnly, now) + expect(isSkipActive(api, pathOnly, now + 3 * 24 * 60 * 60 * 1000)).toBe(true) + // A different path is not affected. + expect(isSkipActive(api, { projectPath: "/scratch/other" }, now)).toBe(false) }) }) From ea8ce8f9b49a2a1348640a3462a6d5eda68a921a Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 03:53:11 +0530 Subject: [PATCH 03/14] =?UTF-8?q?fix(workspace):=20consensus=20review=20?= =?UTF-8?q?=E2=80=94=20flag=20gating,=20orphan-safe=20create,=20matched-id?= =?UTF-8?q?entifier=20rebind,=20req()=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review findings that belong to this PR's commits (7c7e17f193 + 76de5a93cd). The three remaining findings introduced by the stacked browser-handoff PR are fixed on that branch. - Gate the LinkCommand registration in src/index.ts AND the Workspace TUI plugin registration behind Flag.ALTIMATE_WORKSPACE. Previously the flag gated only the post-scan trigger publish, so the palette command, altimate-code link subcommand, and post-scan handler shipped to 100% of users regardless of the flag setting. (M1) - createAndBindInline / createAndBind now accept an "already linked" outcome and rebind after create. Before this, "＋ Create a new workspace" on an already-linked project silently orphaned the freshly-created workspace in the SaaS — a real (billable) resource the CLI knew nothing about. On rebind failure the error message tells the user the workspace exists and how to recover. (M2) - getBindingForProject now returns which identifier arm matched (remote or path) via a new ``matchedBy`` field. AlreadyLinkedDialog, PickerDialog, bindOrRebindInline, and cli/cmd/link.ts all use matched-identifier for the rebind endpoint — not the CURRENT identifier — so a repo whose remote was renamed still repairs via its path binding instead of 404'ing on rebindByRemote. hasDrift is now computed from matched-vs-current identifier instead of hardcoded false. (M3) - listDatamates now routes through req() (via a new ``base`` option) so it inherits the 15s abort, typed error mapping, empty-body guard, and detail parsing every other endpoint gets. Non-integer / non-positive ids are filtered out at the boundary. (M5) - req() throws WorkspaceApiError on an empty 2xx body (previously returned undefined as T, producing a downstream TypeError the typed switches couldn't classify). ``allowEmptyBody`` opt-in for 204 endpoints. (m7) - AbortError is now distinguished from a network failure — the 15s abort produces "Request timed out after 15s" instead of the generic "Cannot reach" message. (m8) - Session-idle listener now captures the unsubscribe from events.listen() and tears itself down when the pending-sessions Set drains. Previously the listener was permanently installed for the process lifetime, and a failed install could leave a duplicate handler behind that fired workspace prompts twice. (m4) - Failed pre-check in cli/cmd/link.ts now retries a bindExisting → 409 as an unconditional rebind, so a user whose pre-check network-flaked isn't stuck at "Already linked to X" with no next step. (m10) Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../altimate/plugin/onboarding-telemetry.ts | 70 ++++-- .../src/altimate/workspace/api-client.ts | 85 +++++-- packages/opencode/src/cli/cmd/link.ts | 181 +++++++++++--- packages/opencode/src/index.ts | 11 +- .../opencode/src/plugin/tui/altimate/index.ts | 8 +- .../src/plugin/tui/altimate/workspace.tsx | 234 +++++++++++++----- 6 files changed, 441 insertions(+), 148 deletions(-) diff --git a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts index 88414342b9..416f71188d 100644 --- a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts +++ b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts @@ -23,6 +23,9 @@ import { AppRuntime } from "@/effect/app-runtime" import { EventV2Bridge } from "@/event-v2-bridge" import { TuiEvent } from "@/server/tui-event" import { Event as SessionEvent } from "@/session/status" +import { Log } from "@/altimate/util/log" + +const workspaceLog = Log.create({ service: "altimate-workspace" }) /** * Publish the workspace-postScan command AFTER the session goes idle, not on @@ -32,36 +35,55 @@ import { Event as SessionEvent } from "@/session/status" * session.idle costs a few seconds of latency but sidesteps the race entirely — * the dialog appears once things are quiet. * - * One-shot per sessionID: the listener unregisters as soon as the target - * session emits idle. A pending arm is dropped if a second project_scan fires - * in the same session (unlikely but handled). + * One-shot per sessionID: pending sessions live in a Set, and when a session + * emits idle its id is removed. When the Set drains, the EventV2 listener is + * torn down via the unsubscribe returned by ``events.listen()`` so a + * permanently-installed no-op handler isn't left behind for the process + * lifetime (m4 in the consensus review). A pending arm is dropped if a + * second project_scan fires in the same session. */ const pendingWorkspacePromptSessions = new Set() -let workspacePromptListenerArmed = false +let workspacePromptUnsubscribe: (() => void) | null = null async function armWorkspacePromptOnSessionIdle(sessionID: string): Promise { pendingWorkspacePromptSessions.add(sessionID) - if (workspacePromptListenerArmed) return - workspacePromptListenerArmed = true - await AppRuntime.runPromise( - EventV2Bridge.Service.use((events) => - events.listen((event) => - Effect.gen(function* () { - if (event.type !== SessionEvent.Idle.type) return - const sid = (event.data as { sessionID?: string } | undefined)?.sessionID - if (!sid || !pendingWorkspacePromptSessions.has(sid)) return - pendingWorkspacePromptSessions.delete(sid) - yield* events.publish(TuiEvent.CommandExecute, { - command: "altimate.workspace.postScan", - }) - }), + if (workspacePromptUnsubscribe) return + try { + const unsubscribe = await AppRuntime.runPromise( + EventV2Bridge.Service.use((events) => + events.listen((event) => + Effect.gen(function* () { + if (event.type !== SessionEvent.Idle.type) return + const sid = (event.data as { sessionID?: string } | undefined)?.sessionID + if (!sid || !pendingWorkspacePromptSessions.has(sid)) return + pendingWorkspacePromptSessions.delete(sid) + yield* events.publish(TuiEvent.CommandExecute, { + command: "altimate.workspace.postScan", + }) + // Once the Set drains, tear the listener down. A later scan + // that adds a new pending session re-arms it from scratch. + if (pendingWorkspacePromptSessions.size === 0 && workspacePromptUnsubscribe) { + const teardown = workspacePromptUnsubscribe + workspacePromptUnsubscribe = null + try { + teardown() + } catch (err) { + workspaceLog.warn("session-idle listener teardown failed", { + err: String(err), + }) + } + } + }), + ), ), - ), - ).catch((err) => { - // eslint-disable-next-line no-console - console.error("[altimate-workspace] session-idle listener install failed:", err) - workspacePromptListenerArmed = false - }) + ) + workspacePromptUnsubscribe = unsubscribe as unknown as () => void + } catch (err) { + // Install failed — drop the pending session so the next scan retries + // from scratch instead of accumulating a stale id that will never fire. + pendingWorkspacePromptSessions.delete(sessionID) + workspaceLog.warn("session-idle listener install failed", { err: String(err) }) + } } // altimate_change end diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 8078bd2cf5..75a899cb52 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -52,6 +52,17 @@ export interface GetBindingResponse { datamate: DatamateRef } +/** Which identifier arm the pre-check lookup actually matched on. Callers use + * this to pick the correct rebind endpoint (``/by-remote`` vs ``/by-path``) + * regardless of what the CURRENT identifier has — a repo whose remote was + * renamed still resolves via its ``project_path``, and a later ``rebindByRemote`` + * would 404 because no binding exists under the new remote. (M3) */ +export type MatchedIdentifier = "remote" | "path" + +export interface ProjectBindingLookup extends GetBindingResponse { + matchedBy: MatchedIdentifier +} + export interface ConflictDetail { message: string existing_datamate_id?: number @@ -120,11 +131,24 @@ async function creds(): Promise<{ url: string; instance: string; apiKey: string async function req( method: string, subpath: string, - opts: { body?: unknown; query?: Record } = {}, + opts: { + body?: unknown + query?: Record + /** Override the base path prefix. Defaults to + * ``/datamate-project-bindings`` (this module's namespace). Pass e.g. + * ``/datamates`` to hit the sibling datamates_router through the same + * timeout / typed-error / empty-body machinery. */ + base?: string + /** If true, a 2xx with an empty body returns ``undefined`` typed as T + * instead of throwing. Only set for endpoints known to return 204 or a + * bare 200 with no payload. */ + allowEmptyBody?: boolean + } = {}, ): Promise { const { url, instance, apiKey } = await creds() const qs = opts.query ? "?" + new URLSearchParams(opts.query).toString() : "" - const target = `${url}/datamate-project-bindings${subpath}${qs}` + const basePath = opts.base ?? "/datamate-project-bindings" + const target = `${url}${basePath}${subpath}${qs}` const controller = new AbortController() const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS) let res: Response @@ -140,6 +164,15 @@ async function req( ...(opts.body !== undefined ? { body: JSON.stringify(opts.body) } : {}), }) } catch (err) { + // Distinguish "we hit our 15s abort" from "network stack failed" so the + // caller can decide differently (retry, longer timeout, offline banner). + // (m8 in the consensus review.) + const name = (err as { name?: string } | undefined)?.name + if (name === "AbortError") { + throw new WorkspaceApiError( + `Request to ${target} timed out after ${Math.round(REQUEST_TIMEOUT_MS / 1000)}s`, + ) + } const msg = err instanceof Error ? err.message : String(err) throw new WorkspaceApiError(`Cannot reach ${target}: ${msg}`) } finally { @@ -177,6 +210,18 @@ async function req( res.status, ) } + // A 2xx with an empty (or unparseable) body is not the same as a resource. + // Callers dereference the return immediately (``.binding``, ``.datamate``, + // ``.manage_url``), so silently handing back ``undefined as T`` produces a + // ``TypeError`` inside caller code that the typed-error switches can't + // classify. Surface it as a WorkspaceApiError instead — unless the caller + // opted in via ``allowEmptyBody`` (e.g. 204 endpoints). (m7) + if (json === undefined && !opts.allowEmptyBody) { + throw new WorkspaceApiError( + `Empty ${res.status} body from ${target} — expected JSON payload`, + res.status, + ) + } return json as T } @@ -203,15 +248,18 @@ export namespace WorkspaceApi { } /** Tries remote first (stronger identity), then path. Returns the first hit - * or null. Both fields on the identifier are optional but at least one must - * be present. */ - export async function getBindingForProject(id: ProjectIdentifier): Promise { + * TAGGED with which identifier matched, so a caller that later rebinds + * picks the right endpoint even if the current identifier's remote has + * changed since the binding was created (M3). Both fields on the + * identifier are optional but at least one must be present. */ + export async function getBindingForProject(id: ProjectIdentifier): Promise { if (id.repoRemote) { const hit = await getBindingForRemote(id.repoRemote) - if (hit) return hit + if (hit) return { ...hit, matchedBy: "remote" } } if (id.projectPath) { - return await getBindingForPath(id.projectPath) + const hit = await getBindingForPath(id.projectPath) + if (hit) return { ...hit, matchedBy: "path" } } return null } @@ -279,15 +327,20 @@ export namespace WorkspaceApi { } /** Populates the "link to existing workspace" picker. Reuses the existing - * /datamates/ list endpoint on the datamates_router. */ + * ``/datamates/`` list endpoint on the datamates_router — routed through + * the shared ``req()`` machinery so it inherits the 15s abort, typed + * error mapping, empty-body guard, and detail-parsing everyone else + * gets. (M5) Filters out non-integer / non-positive ids so a corrupt row + * doesn't reach the picker as a "NaN" label that the caller then binds + * against. */ export async function listDatamates(): Promise { - const { url, instance, apiKey } = await creds() - const res = await fetch(`${url}/datamates/`, { - headers: { Authorization: `Bearer ${apiKey}`, "x-tenant": instance }, - }) - if (!res.ok) - throw new WorkspaceApiError(`Failed to list workspaces (status ${res.status})`, res.status) - const body = (await res.json()) as { datamates?: Array<{ id: number | string; name: string }> } - return (body.datamates ?? []).map((d) => ({ id: Number(d.id), name: d.name })) + const body = await req<{ datamates?: Array<{ id: number | string; name: string }> }>( + "GET", + "/", + { base: "/datamates" }, + ) + return (body.datamates ?? []) + .map((d) => ({ id: Number(d.id), name: d.name })) + .filter((d) => Number.isInteger(d.id) && d.id > 0) } } diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 434e1306bd..f639fd21e2 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -23,7 +23,8 @@ import { NotFoundError, PreconditionFailedError, type DatamateRef, - type GetBindingResponse, + type MatchedIdentifier, + type ProjectBindingLookup, type ProjectIdentifier, } from "@/altimate/workspace/api-client" import { @@ -62,7 +63,11 @@ export const LinkCommand = cmd({ // Pre-check for the currently-linked marker + workspace list. Both are // fetched up-front so the picker can annotate the current binding. - let existing: GetBindingResponse | null = null + // ``preCheckOk = false`` means the pre-check itself failed (network, + // 5xx) rather than "not linked" — used later to retry a 409 as a rebind + // instead of surfacing "already linked to X" with no next step. (m10) + let existing: ProjectBindingLookup | null = null + let preCheckOk = true try { existing = await WorkspaceApi.getBindingForProject(identifier) } catch (err) { @@ -71,6 +76,7 @@ export const LinkCommand = cmd({ process.exitCode = 1 return } + preCheckOk = false prompts.log.warn( `Could not reach the workspace service to look up existing bindings (${err instanceof Error ? err.message : String(err)}). Continuing without the currently-linked marker.`, ) @@ -99,7 +105,9 @@ export const LinkCommand = cmd({ { value: CREATE_NEW_SENTINEL, label: `＋ Create a new workspace "${autoName}"`, - hint: "Named from this project; rename in the SaaS after.", + hint: existing + ? "Creates a new workspace and repoints this project to it." + : "Named from this project; rename in the SaaS after.", }, ...list.map((dm) => ({ value: String(dm.id), @@ -122,7 +130,7 @@ export const LinkCommand = cmd({ } if (pick === CREATE_NEW_SENTINEL) { - await createAndBind(identifier, autoName, args.directory) + await createThenBindOrRebind(identifier, autoName, args.directory, existing) return } @@ -132,71 +140,136 @@ export const LinkCommand = cmd({ return } - await bindOrRebind(identifier, targetId, currentId, args.directory) + await bindOrRebind(identifier, targetId, existing, preCheckOk, args.directory) }, }) -async function createAndBind( +/** "＋ Create a new workspace" flow. When the project is already linked, this + * MUST rebind after create — otherwise the new workspace is a 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( identifier: ProjectIdentifier, name: string, directory: string, + existing: ProjectBindingLookup | null, ): Promise { const spin = prompts.spinner() spin.start(`Creating workspace "${name}"...`) + let created: Awaited> try { - const res = await WorkspaceApi.createAndBind({ name, identifier }) - await recordApprovedBinding(directory, { - datamateId: res.datamate.id, - datamateName: res.datamate.name, - repoRemote: res.binding.repo_remote, - projectPath: res.binding.project_path, - linkedAt: Date.now(), - }) - spin.stop(`Workspace "${res.datamate.name}" created and linked.`) - prompts.log.info(`Manage it at: ${res.manage_url}`) - await open(res.manage_url).catch(() => undefined) - prompts.outro("Done.") + 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. if (err instanceof ConflictError) { prompts.log.error( - `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` to switch.`, + `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` to switch to a different workspace.`, ) } else { prompts.log.error(err instanceof Error ? err.message : String(err)) } process.exitCode = 1 + return } + spin.stop(`Workspace "${created.datamate.name}" 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. + if (existing) { + const rebindSpin = prompts.spinner() + rebindSpin.start(`Repointing project at "${created.datamate.name}"...`) + try { + await rebindByMatchedIdentifier({ + identifier, + targetDatamateId: created.datamate.id, + expectedCurrentDatamateId: existing.datamate.id, + matchedBy: existing.matchedBy, + }) + rebindSpin.stop(`Project is now linked to "${created.datamate.name}".`) + } catch (err) { + rebindSpin.stop("Could not repoint the project.", 1) + prompts.log.error( + `Workspace "${created.datamate.name}" was CREATED but could not be linked to this project. ${err instanceof Error ? err.message : String(err)} — re-run \`altimate-code link\` to retry (or delete the workspace in the SaaS).`, + ) + process.exitCode = 1 + return + } + } + await recordApprovedBinding(directory, { + datamateId: created.datamate.id, + datamateName: created.datamate.name, + repoRemote: created.binding.repo_remote, + projectPath: created.binding.project_path, + linkedAt: Date.now(), + }) + prompts.log.info(`Manage it at: ${created.manage_url}`) + await open(created.manage_url).catch(() => undefined) + prompts.outro("Done.") } async function bindOrRebind( identifier: ProjectIdentifier, targetDatamateId: number, - currentDatamateId: number | undefined, + existing: ProjectBindingLookup | null, + preCheckOk: boolean, directory: string, ): Promise { - const isRebind = currentDatamateId !== undefined + const isRebind = existing !== null const spin = prompts.spinner() spin.start(isRebind ? `Re-linking to workspace...` : `Linking to workspace...`) try { - const res = await (async () => { - if (isRebind) { - // Rebind endpoint depends on which identifier is present. Prefer remote - // (stronger identity — survives directory moves); fall back to path. - return identifier.repoRemote - ? WorkspaceApi.rebindByRemote({ - remote: identifier.repoRemote, - targetDatamateId, - expectedCurrentDatamateId: currentDatamateId, - }) - : WorkspaceApi.rebindByPath({ - projectPath: identifier.projectPath!, - targetDatamateId, - expectedCurrentDatamateId: currentDatamateId, - }) + let res + if (isRebind) { + res = await rebindByMatchedIdentifier({ + identifier, + targetDatamateId, + expectedCurrentDatamateId: existing.datamate.id, + matchedBy: existing.matchedBy, + }) + } 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) + } catch (err) { + if (err instanceof ConflictError && !preCheckOk) { + // Pre-check failed and the server confirms this project IS linked + // already. Retry as an unconditional rebind — we don't have an + // ``expected_current_datamate_id`` (pre-check gave us nothing) so + // this is last-writer-wins. Callers who need optimistic concurrency + // should re-run once the network is back and the pre-check succeeds. + spin.stop("Pre-check missed an existing binding — retrying as re-link.", 1) + const rebindSpin = prompts.spinner() + rebindSpin.start("Re-linking...") + try { + res = identifier.repoRemote + ? await WorkspaceApi.rebindByRemote({ + remote: identifier.repoRemote, + targetDatamateId, + }) + : await WorkspaceApi.rebindByPath({ + projectPath: identifier.projectPath!, + targetDatamateId, + }) + rebindSpin.stop(`Re-linked to "${res.binding.datamate_name}".`) + } catch (retryErr) { + rebindSpin.stop("Re-link failed.", 1) + throw retryErr + } + } else { + throw err + } } - return WorkspaceApi.bindExisting(targetDatamateId, identifier) - })() + } await recordApprovedBinding(directory, { datamateId: res.binding.datamate_id, datamateName: res.binding.datamate_name, @@ -214,7 +287,7 @@ async function bindOrRebind( spin.stop(isRebind ? `Re-link failed.` : `Link failed.`, 1) if (err instanceof ConflictError) { prompts.log.error( - `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}".`, + `Already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Re-run \`altimate-code link\` to switch.`, ) } else if (err instanceof PreconditionFailedError) { prompts.log.error("Someone else re-linked this project — re-run and try again.") @@ -228,3 +301,33 @@ async function bindOrRebind( process.exitCode = 1 } } + +/** Pick the rebind endpoint that matches which identifier the pre-check + * resolved the binding on — NOT which identifier the current call happens to + * carry. A repo whose remote was renamed still has a binding under its path; + * rebindByRemote against the new remote would 404 with no repair path from + * the CLI. (M3) */ +async function rebindByMatchedIdentifier(input: { + identifier: ProjectIdentifier + targetDatamateId: number + expectedCurrentDatamateId: number + matchedBy: MatchedIdentifier +}) { + if (input.matchedBy === "remote" && input.identifier.repoRemote) { + return WorkspaceApi.rebindByRemote({ + remote: input.identifier.repoRemote, + targetDatamateId: input.targetDatamateId, + expectedCurrentDatamateId: input.expectedCurrentDatamateId, + }) + } + if (input.matchedBy === "path" && input.identifier.projectPath) { + return WorkspaceApi.rebindByPath({ + projectPath: input.identifier.projectPath, + targetDatamateId: input.targetDatamateId, + expectedCurrentDatamateId: input.expectedCurrentDatamateId, + }) + } + throw new Error( + `Cannot rebind — the pre-check matched on ${input.matchedBy} but that field is not present on the current project identifier.`, + ) +} diff --git a/packages/opencode/src/index.ts b/packages/opencode/src/index.ts index 82ef4cb443..d2fe2b3506 100644 --- a/packages/opencode/src/index.ts +++ b/packages/opencode/src/index.ts @@ -10,6 +10,7 @@ import { UninstallCommand } from "./cli/cmd/uninstall" import { ModelsCommand } from "./cli/cmd/models" import { UI } from "./cli/ui" import { InstallationVersion, InstallationLocal } from "@opencode-ai/core/installation/version" +import { Flag } from "@opencode-ai/core/flag/flag" import { FormatError } from "./cli/error" import { ServeCommand } from "./cli/cmd/serve" // altimate_change start — workspace-serve: dev-only workspace serve command @@ -173,8 +174,14 @@ let cli = yargs(args) // altimate_change start — check: register deterministic SQL check command .command(CheckCommand) // altimate_change end - // altimate_change start — link: workspace-binding subcommand - .command(LinkCommand) + +// altimate_change start — link: gated on Flag.ALTIMATE_WORKSPACE (pilot) +// so the command isn't registered — and doesn't show in --help — for users +// who haven't opted in to the workspaces feature via ALTIMATE_WORKSPACE=1. +// (M1 in the consensus review.) +if (Flag.ALTIMATE_WORKSPACE) { + cli = cli.command(LinkCommand) +} // altimate_change end // altimate_change start — workspace-serve: register dev-only workspace serve command diff --git a/packages/opencode/src/plugin/tui/altimate/index.ts b/packages/opencode/src/plugin/tui/altimate/index.ts index 900e06fe38..adb5662e6b 100644 --- a/packages/opencode/src/plugin/tui/altimate/index.ts +++ b/packages/opencode/src/plugin/tui/altimate/index.ts @@ -9,6 +9,7 @@ // plugin list in ../internal.ts. import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import type { RuntimeFlags } from "@/effect/runtime-flags" +import { Flag } from "@opencode-ai/core/flag/flag" import ProviderCredentials from "./provider-credentials" import PromptEnhance from "./prompt-enhance" import SkillOps from "./skill-ops" @@ -24,6 +25,11 @@ import Workspace from "./workspace" // import TraceViewer from "./trace-viewer" // import Workspace from "./workspace" export function altimateTuiPlugins(_flags: Pick): BuiltinTuiPlugin[] { - return [ProviderCredentials, PromptEnhance, SkillOps, TraceViewer, Workspace] + const base = [ProviderCredentials, PromptEnhance, SkillOps, TraceViewer] + // Workspace TUI plugin is pilot-gated: only registered for users who + // opted into ALTIMATE_WORKSPACE. Otherwise the post-scan dialog + the + // altimate.workspace.link palette command would ship to 100% of users + // regardless of the flag setting. (M1 in the consensus review.) + return Flag.ALTIMATE_WORKSPACE ? [...base, Workspace] : base } // altimate_change end diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index cfdc26d985..0da23708ed 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -33,7 +33,8 @@ import { PreconditionFailedError, WorkspaceApi, type DatamateRef, - type GetBindingResponse, + type MatchedIdentifier, + type ProjectBindingLookup, type ProjectIdentifier, } from "@/altimate/workspace/api-client" import { @@ -135,30 +136,16 @@ async function createAndBindInline( api: TuiPluginApi, identifier: ProjectIdentifier, name: string, + /** When present, this project is already bound to another workspace. + * createAndBind succeeds but leaves the binding pointing at the OLD + * workspace; without this rebind step the new workspace is an orphaned + * (billable) SaaS resource the CLI knows nothing about (M2). */ + rebindFrom?: { expectedCurrentDatamateId: number; matchedBy: MatchedIdentifier }, ): Promise { api.ui.dialog.clear() + let res: Awaited> try { - const res = await WorkspaceApi.createAndBind({ name, identifier }) - await recordApprovedBinding(api.state.path.directory, { - datamateId: res.datamate.id, - datamateName: res.datamate.name, - repoRemote: res.binding.repo_remote, - projectPath: res.binding.project_path, - linkedAt: Date.now(), - }) - try { - await open(res.manage_url) - api.ui.toast({ - variant: "success", - message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, - }) - } catch { - api.ui.toast({ - variant: "info", - message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, - duration: 10_000, - }) - } + res = await WorkspaceApi.createAndBind({ name, identifier }) } catch (err) { if (err instanceof ConflictError) { api.ui.toast({ @@ -171,9 +158,83 @@ async function createAndBindInline( message: err instanceof Error ? err.message : "Failed to create workspace", }) } + return + } + + 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. + try { + await rebindByMatchedIdentifier({ + identifier, + targetDatamateId: res.datamate.id, + expectedCurrentDatamateId: rebindFrom.expectedCurrentDatamateId, + matchedBy: rebindFrom.matchedBy, + }) + } catch (err) { + api.ui.toast({ + variant: "error", + message: `Workspace "${res.datamate.name}" was CREATED but could not be linked to this project (${err instanceof Error ? err.message : String(err)}). Run \`altimate-code link\` to retry.`, + duration: 15_000, + }) + return + } + } + + await recordApprovedBinding(api.state.path.directory, { + datamateId: res.datamate.id, + datamateName: res.datamate.name, + repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, + linkedAt: Date.now(), + }) + try { + await open(res.manage_url) + api.ui.toast({ + variant: "success", + message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, + }) + } catch { + api.ui.toast({ + variant: "info", + message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, + duration: 10_000, + }) } } +/** Pick the rebind endpoint that matches which identifier the pre-check + * resolved the binding on. Shared with cli/cmd/link.ts through duplicated + * code (M3) — the modules deliberately don't cross-import so the CLI + * subcommand stays self-contained. */ +async function rebindByMatchedIdentifier(input: { + identifier: ProjectIdentifier + targetDatamateId: number + expectedCurrentDatamateId: number + matchedBy: MatchedIdentifier +}) { + if (input.matchedBy === "remote" && input.identifier.repoRemote) { + return WorkspaceApi.rebindByRemote({ + remote: input.identifier.repoRemote, + targetDatamateId: input.targetDatamateId, + expectedCurrentDatamateId: input.expectedCurrentDatamateId, + }) + } + if (input.matchedBy === "path" && input.identifier.projectPath) { + return WorkspaceApi.rebindByPath({ + projectPath: input.identifier.projectPath, + targetDatamateId: input.targetDatamateId, + expectedCurrentDatamateId: input.expectedCurrentDatamateId, + }) + } + throw new Error( + `Cannot rebind — pre-check matched on ${input.matchedBy} but that field is not present on the current project identifier.`, + ) +} + interface AlreadyLinkedProps { api: TuiPluginApi identifier: ProjectIdentifier @@ -182,6 +243,12 @@ interface AlreadyLinkedProps { hasDrift: boolean driftedWas?: string | null unverified?: boolean + /** Which identifier arm resolved the binding — remote-matched projects + * rebind via ``/by-remote``, path-matched via ``/by-path``. Not the same + * as ``identifier.repoRemote`` / ``identifier.projectPath``, which reflect + * the CURRENT project, not the binding's origin. Threaded into PickerDialog + * so a re-link picks the correct endpoint. (M3) */ + matchedBy: MatchedIdentifier } function AlreadyLinkedDialog(props: AlreadyLinkedProps) { @@ -222,13 +289,15 @@ function AlreadyLinkedDialog(props: AlreadyLinkedProps) { return } // relink → picker with the current workspace id as expected_current so - // a concurrent re-link by another client 412s cleanly. + // a concurrent re-link by another client 412s cleanly. matchedBy + // determines which rebind endpoint the picker will call (M3). props.api.ui.dialog.replace(() => ( )) }} @@ -241,6 +310,9 @@ interface PickerProps { identifier: ProjectIdentifier mode: "attach" | "relink" expectedCurrentDatamateId?: number + /** Set for ``mode: "relink"`` — which identifier arm the pre-check matched + * on so we pick the correct rebind endpoint. (M3) */ + matchedBy?: MatchedIdentifier } function PickerDialog(props: PickerProps) { @@ -275,20 +347,20 @@ function PickerDialog(props: PickerProps) { message: `Linked to workspace "${res.binding.datamate_name}".`, }) } else { - // Rebind: pick the endpoint that matches the identifier we have. - // Prefer remote (stronger identity) when available; else fall back to - // path — matches the pre-check ordering in getBindingForProject. - const res = props.identifier.repoRemote - ? await WorkspaceApi.rebindByRemote({ - remote: props.identifier.repoRemote, - targetDatamateId: datamateId, - expectedCurrentDatamateId: props.expectedCurrentDatamateId, - }) - : await WorkspaceApi.rebindByPath({ - projectPath: props.identifier.projectPath!, - targetDatamateId: datamateId, - expectedCurrentDatamateId: props.expectedCurrentDatamateId, - }) + // Rebind: pick the endpoint that matches which identifier the + // pre-check RESOLVED the binding on — not what the current identifier + // happens to carry. A repo whose remote was renamed still has its + // binding under its path; rebindByRemote against the new remote would + // 404 with no repair path from the TUI. (M3) + if (!props.matchedBy || !props.expectedCurrentDatamateId) { + throw new Error("relink picker opened without matchedBy / expectedCurrentDatamateId") + } + const res = await rebindByMatchedIdentifier({ + identifier: props.identifier, + targetDatamateId: datamateId, + expectedCurrentDatamateId: props.expectedCurrentDatamateId, + matchedBy: props.matchedBy, + }) await recordApprovedBinding(props.api.state.path.directory, { datamateId: res.binding.datamate_id, datamateName: res.binding.datamate_name, @@ -363,6 +435,10 @@ interface OnDemandPickerProps { identifier: ProjectIdentifier currentlyLinkedDatamateId?: number currentlyLinkedDatamateName?: string + /** Which identifier arm the pre-check matched on. Required to pick the + * correct rebind endpoint when the user swaps workspaces or picks Create + * on an already-linked project. (M3) */ + matchedBy?: MatchedIdentifier defaultName: string } @@ -419,7 +495,18 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { return } if (option.value === CREATE_NEW_SENTINEL) { - void createAndBindInline(props.api, props.identifier, props.defaultName) + // If already linked, thread the pre-check outcome so createAndBind + // is followed by a rebind — otherwise the new workspace is a real + // (billable) SaaS resource left orphaned while the project is + // still bound to the OLD workspace. (M2) + const rebindFrom = + props.currentlyLinkedDatamateId !== undefined && props.matchedBy + ? { + expectedCurrentDatamateId: props.currentlyLinkedDatamateId, + matchedBy: props.matchedBy, + } + : undefined + void createAndBindInline(props.api, props.identifier, props.defaultName, rebindFrom) return } // Picked an existing workspace. @@ -432,12 +519,11 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { props.api.ui.dialog.clear() return } - void bindOrRebindInline( - props.api, - props.identifier, - option.value, - props.currentlyLinkedDatamateId, - ) + const existing = + props.currentlyLinkedDatamateId !== undefined && props.matchedBy + ? { datamateId: props.currentlyLinkedDatamateId, matchedBy: props.matchedBy } + : undefined + void bindOrRebindInline(props.api, props.identifier, option.value, existing) }} /> ) @@ -447,24 +533,22 @@ async function bindOrRebindInline( api: TuiPluginApi, identifier: ProjectIdentifier, targetDatamateId: number, - currentDatamateId: number | undefined, + /** Pre-check outcome. Absent means "not linked / pre-check missed" and + * we call bindExisting; present means "linked" and we rebind via the + * matched-identifier endpoint (M3). */ + existing: { datamateId: number; matchedBy: MatchedIdentifier } | undefined, ): Promise { api.ui.dialog.clear() + const isRebind = existing !== undefined try { const res = await (async () => { - if (currentDatamateId) { - // Rebind: use whichever identifier we have; remote-first for identity. - return identifier.repoRemote - ? WorkspaceApi.rebindByRemote({ - remote: identifier.repoRemote, - targetDatamateId, - expectedCurrentDatamateId: currentDatamateId, - }) - : WorkspaceApi.rebindByPath({ - projectPath: identifier.projectPath!, - targetDatamateId, - expectedCurrentDatamateId: currentDatamateId, - }) + if (existing) { + return rebindByMatchedIdentifier({ + identifier, + targetDatamateId, + expectedCurrentDatamateId: existing.datamateId, + matchedBy: existing.matchedBy, + }) } return WorkspaceApi.bindExisting(targetDatamateId, identifier) })() @@ -477,7 +561,7 @@ async function bindOrRebindInline( }) api.ui.toast({ variant: "success", - message: currentDatamateId + message: isRebind ? `Re-linked to workspace "${res.binding.datamate_name}".` : `Linked to workspace "${res.binding.datamate_name}".`, }) @@ -514,7 +598,7 @@ async function runOnDemandPicker(api: TuiPluginApi, directory: string): Promise< const identifier = resolveProjectIdentifier(directory) // Pre-check for the currently-linked marker. Failures are non-fatal — we // still show the picker without the "(currently linked here)" annotation. - let existing: GetBindingResponse | null = null + let existing: ProjectBindingLookup | null = null try { existing = await WorkspaceApi.getBindingForProject(identifier) } catch (err) { @@ -531,6 +615,7 @@ async function runOnDemandPicker(api: TuiPluginApi, directory: string): Promise< identifier={identifier} currentlyLinkedDatamateId={existing?.datamate.id} currentlyLinkedDatamateName={existing?.datamate.name} + matchedBy={existing?.matchedBy} defaultName={defaultName} /> )) @@ -555,7 +640,7 @@ async function runFlow( ? projectNameFromRemote(identifier.repoRemote) : projectNameFromPath(identifier.projectPath) - let serverBinding: GetBindingResponse | null | undefined + let serverBinding: ProjectBindingLookup | null | undefined try { serverBinding = await WorkspaceApi.getBindingForProject(identifier) } catch (err) { @@ -574,13 +659,27 @@ async function runFlow( projectPath: serverBinding.binding.project_path, linkedAt: Date.now(), }) + // Drift = the identifier the server matched on doesn't equal the + // corresponding identifier this project currently has. E.g. we matched + // on remote but the current remote differs from what the binding + // stored — the repo was renamed / remote swapped. The dialog surfaces + // this so the user isn't silently attached to a stale binding. (M3) + const boundIdent = + serverBinding.matchedBy === "remote" + ? serverBinding.binding.repo_remote + : serverBinding.binding.project_path + const currentIdent = + serverBinding.matchedBy === "remote" ? identifier.repoRemote : identifier.projectPath + const hasDrift = boundIdent != null && currentIdent != null && boundIdent !== currentIdent api.ui.dialog.replace(() => ( )) return @@ -602,17 +701,20 @@ async function runFlow( // Server unreachable — fall back to the local cache (marked as unverified). const local = await readLocalBinding(directory) if (local) { - // Drift = the identifier the cache remembers differs from what we're - // seeing now. Compare on the field that's actually populated in the cache. + // Prefer whichever identifier the cache remembers as populated. Same + // ordering as the server-side pre-check: remote first, path fallback. + const cachedMatchedBy: MatchedIdentifier = local.repoRemote ? "remote" : "path" const cachedIdent = local.repoRemote ?? local.projectPath ?? "" - const currentIdent = identifier.repoRemote ?? identifier.projectPath - const hasDrift = cachedIdent !== currentIdent + const currentIdent = + cachedMatchedBy === "remote" ? identifier.repoRemote : identifier.projectPath + const hasDrift = cachedIdent !== "" && currentIdent != null && cachedIdent !== currentIdent api.ui.dialog.replace(() => ( Date: Mon, 17 Aug 2026 09:26:30 +0530 Subject: [PATCH 04/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=201?= =?UTF-8?q?=20=E2=80=94=20GitGuardian=20unblock=20+=20null-body=20guard=20?= =?UTF-8?q?+=20safe=20manage=5Furl=20open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace token-shaped documentation example in detect.ts with a generic / placeholder so GitGuardian's "Basic Auth String" detector stops flagging the comment. Not a real credential; the swap is cosmetic + pipeline-unblocking. (CR + GitGuardian) - req() empty-body guard now uses ``== null`` so a literal JSON ``null`` response (which parses to the JS null, not undefined) is rejected too. Previously ``json === undefined`` missed the null case and returned ``null as T``, producing a downstream ``TypeError: Cannot read properties of null`` that the typed switches couldn't classify. (CR) - Both open(manage_url) call sites now validate the URL parses as http(s) before handing to open(). ``open`` delegates to the OS scheme handler, so a rogue server-supplied protocol could launch an unrelated application. Extracted a tiny ``isSafeHttpUrl`` helper (duplicated in each file — the modules deliberately don't cross-import). (CR) Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../src/altimate/workspace/api-client.ts | 7 +++- .../opencode/src/altimate/workspace/detect.ts | 2 +- packages/opencode/src/cli/cmd/link.ts | 22 +++++++++- .../src/plugin/tui/altimate/workspace.tsx | 40 ++++++++++++++----- 4 files changed, 57 insertions(+), 14 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 75a899cb52..073dc64059 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -215,8 +215,11 @@ async function req( // ``.manage_url``), so silently handing back ``undefined as T`` produces a // ``TypeError`` inside caller code that the typed-error switches can't // classify. Surface it as a WorkspaceApiError instead — unless the caller - // opted in via ``allowEmptyBody`` (e.g. 204 endpoints). (m7) - if (json === undefined && !opts.allowEmptyBody) { + // opted in via ``allowEmptyBody`` (e.g. 204 endpoints). Use ``== null`` so a + // literal ``JSON.parse("null")`` (which sets json to null, not undefined) + // is treated as an empty body too — otherwise ``null as T`` reaches callers + // and .foo throws in a way the typed switches can't classify. (m7 + CR) + if (json == null && !opts.allowEmptyBody) { throw new WorkspaceApiError( `Empty ${res.status} body from ${target} — expected JSON payload`, res.status, diff --git a/packages/opencode/src/altimate/workspace/detect.ts b/packages/opencode/src/altimate/workspace/detect.ts index b731c36480..c5d66f378e 100644 --- a/packages/opencode/src/altimate/workspace/detect.ts +++ b/packages/opencode/src/altimate/workspace/detect.ts @@ -5,7 +5,7 @@ // the `altimate link` CLI subcommand so both entry points identify projects // identically. Reuses the credential-scrubbing helper the ProjectScan tool // exports (../tools/project-scan.ts) so an HTTPS remote with embedded -// basic-auth (e.g. `https://x-access-token:ghp_xxx@github.com/...`) never +// basic-auth (e.g. `https://:@github.com/...`) never // reaches the server or the local cache in clear. import { spawnSync } from "node:child_process" import { realpathSync } from "node:fs" diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index f639fd21e2..fe94d0b1cd 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -211,10 +211,30 @@ async function createThenBindOrRebind( linkedAt: Date.now(), }) prompts.log.info(`Manage it at: ${created.manage_url}`) - await open(created.manage_url).catch(() => undefined) + // 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.`) + } prompts.outro("Done.") } +/** True when the URL parses and its protocol is exactly ``http:`` or ``https:``. + * Used before handing a server-supplied URL to ``open()`` (which would otherwise + * dispatch to whatever OS scheme handler matches the protocol). */ +function isSafeHttpUrl(url: string): boolean { + try { + const u = new URL(url) + return u.protocol === "http:" || u.protocol === "https:" + } catch { + return false + } +} + async function bindOrRebind( identifier: ProjectIdentifier, targetDatamateId: number, diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 0da23708ed..f8b2df1707 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -191,18 +191,38 @@ async function createAndBindInline( projectPath: res.binding.project_path, linkedAt: Date.now(), }) + // Guard against a non-http(s) manage_url — ``open`` dispatches to whatever + // OS handler matches the protocol, so a rogue value could launch an + // unrelated app. Fall through to the info toast (with the URL for manual + // copy) if the URL isn't a safe http/https link. + if (isSafeHttpUrl(res.manage_url)) { + try { + await open(res.manage_url) + api.ui.toast({ + variant: "success", + message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, + }) + return + } catch { + /* fall through to the "open manually" toast below */ + } + } + api.ui.toast({ + variant: "info", + message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, + duration: 10_000, + }) +} + +/** True when the URL parses and its protocol is exactly ``http:`` or ``https:``. + * Used before handing a server-supplied URL to ``open()`` (which would otherwise + * dispatch to whatever OS scheme handler matches the protocol). */ +function isSafeHttpUrl(url: string): boolean { try { - await open(res.manage_url) - api.ui.toast({ - variant: "success", - message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, - }) + const u = new URL(url) + return u.protocol === "http:" || u.protocol === "https:" } catch { - api.ui.toast({ - variant: "info", - message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, - duration: 10_000, - }) + return false } } From 432dd4f9bc551f2ad1b50f5e730bd562eaab91f9 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 09:38:21 +0530 Subject: [PATCH 05/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=202?= =?UTF-8?q?=20=E2=80=94=20timeout-through-body-read,=20cache=20shape=20val?= =?UTF-8?q?idation,=20listener=20install=20race,=20fire-and-forget=20catch?= =?UTF-8?q?,=20test=20isolation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Keep the AbortController timeout ACTIVE while ``req()`` reads the response body. ``fetch()`` resolves after headers arrive; a server can send headers and then stall the body stream forever, and clearing the timer in the first ``finally`` broke the 15s cap. Move ``res.text()`` inside the same try/finally so both the fetch AND the body read fire the same ``AbortError``. (CR) - ``readCache()`` runs a runtime shape check on the parsed JSON before returning — validates version, string tenant/apiUrl, object bindings, and each binding's field types. Previously ``{"version":1,"bindings":null}`` would pass the type assertion and then throw a ``TypeError`` on ``cache.bindings[k]``. (CR) - ``armWorkspacePromptOnSessionIdle`` serializes concurrent install attempts via a shared in-flight promise. Previously two concurrent scans could both pass the ``!workspacePromptUnsubscribe`` check before either install completed, both would install a listener, and the later assignment would overwrite the first disposer — leaking the first listener for the process lifetime. (CR) - The keymap ``run()`` callbacks now attach a ``.catch(reportFlowFailure)`` to the returned promises instead of dropping them with ``void``. An unhandled rejection from ``recordApprovedBinding`` / ``readLocalBinding`` / anything else awaited inside would otherwise terminate the TUI process. (CR) - Test isolation: workspace.test.ts now restores ``XDG_STATE_HOME`` in ``afterAll`` and cleans up its SANDBOX tempdir; ``detectProjectRemote`` test uses a freshly-created empty dir under SANDBOX instead of ``os.tmpdir()`` (which can be inside a git worktree, causing the "not a git repo" assertion to fail on ``git remote get-url``). (CR) Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../altimate/plugin/onboarding-telemetry.ts | 81 +++++++++++-------- .../src/altimate/workspace/api-client.ts | 10 ++- .../opencode/src/altimate/workspace/state.ts | 30 ++++++- .../src/plugin/tui/altimate/workspace.tsx | 20 ++++- .../test/altimate/plugin/workspace.test.ts | 26 +++++- 5 files changed, 123 insertions(+), 44 deletions(-) diff --git a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts index 416f71188d..839050db8d 100644 --- a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts +++ b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts @@ -44,46 +44,59 @@ const workspaceLog = Log.create({ service: "altimate-workspace" }) */ const pendingWorkspacePromptSessions = new Set() let workspacePromptUnsubscribe: (() => void) | null = null +// Guard against two concurrent scans passing the ``!workspacePromptUnsubscribe`` +// check before either install completes — both would then install a listener +// and the later assignment would overwrite the first disposer, leaking the +// first listener for the process lifetime. Store the in-flight install as a +// shared promise so concurrent callers await the same result. (CR round 2.) +let workspacePromptInstall: Promise | null = null async function armWorkspacePromptOnSessionIdle(sessionID: string): Promise { pendingWorkspacePromptSessions.add(sessionID) if (workspacePromptUnsubscribe) return - try { - const unsubscribe = await AppRuntime.runPromise( - EventV2Bridge.Service.use((events) => - events.listen((event) => - Effect.gen(function* () { - if (event.type !== SessionEvent.Idle.type) return - const sid = (event.data as { sessionID?: string } | undefined)?.sessionID - if (!sid || !pendingWorkspacePromptSessions.has(sid)) return - pendingWorkspacePromptSessions.delete(sid) - yield* events.publish(TuiEvent.CommandExecute, { - command: "altimate.workspace.postScan", - }) - // Once the Set drains, tear the listener down. A later scan - // that adds a new pending session re-arms it from scratch. - if (pendingWorkspacePromptSessions.size === 0 && workspacePromptUnsubscribe) { - const teardown = workspacePromptUnsubscribe - workspacePromptUnsubscribe = null - try { - teardown() - } catch (err) { - workspaceLog.warn("session-idle listener teardown failed", { - err: String(err), - }) + if (workspacePromptInstall) return workspacePromptInstall + + workspacePromptInstall = (async () => { + try { + const unsubscribe = await AppRuntime.runPromise( + EventV2Bridge.Service.use((events) => + events.listen((event) => + Effect.gen(function* () { + if (event.type !== SessionEvent.Idle.type) return + const sid = (event.data as { sessionID?: string } | undefined)?.sessionID + if (!sid || !pendingWorkspacePromptSessions.has(sid)) return + pendingWorkspacePromptSessions.delete(sid) + yield* events.publish(TuiEvent.CommandExecute, { + command: "altimate.workspace.postScan", + }) + // Once the Set drains, tear the listener down. A later scan + // that adds a new pending session re-arms it from scratch. + if (pendingWorkspacePromptSessions.size === 0 && workspacePromptUnsubscribe) { + const teardown = workspacePromptUnsubscribe + workspacePromptUnsubscribe = null + try { + teardown() + } catch (err) { + workspaceLog.warn("session-idle listener teardown failed", { + err: String(err), + }) + } } - } - }), + }), + ), ), - ), - ) - workspacePromptUnsubscribe = unsubscribe as unknown as () => void - } catch (err) { - // Install failed — drop the pending session so the next scan retries - // from scratch instead of accumulating a stale id that will never fire. - pendingWorkspacePromptSessions.delete(sessionID) - workspaceLog.warn("session-idle listener install failed", { err: String(err) }) - } + ) + workspacePromptUnsubscribe = unsubscribe as unknown as () => void + } catch (err) { + // Install failed — drop the pending session so the next scan retries + // from scratch instead of accumulating a stale id that will never fire. + pendingWorkspacePromptSessions.delete(sessionID) + workspaceLog.warn("session-idle listener install failed", { err: String(err) }) + } finally { + workspacePromptInstall = null + } + })() + return workspacePromptInstall } // altimate_change end diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 073dc64059..8598f03fc1 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -152,6 +152,7 @@ async function req( const controller = new AbortController() const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS) let res: Response + let text: string try { res = await fetch(target, { method, @@ -163,10 +164,16 @@ async function req( signal: controller.signal, ...(opts.body !== undefined ? { body: JSON.stringify(opts.body) } : {}), }) + // Keep the AbortController timeout ACTIVE while we read the response body. + // ``fetch()`` resolves after headers arrive; a server can send headers then + // stall the body stream indefinitely, so pulling the body inside the same + // try/finally is the difference between our 15s cap and hanging until TCP + // gives up. (CR bot-review round 2.) + text = await res.text().catch(() => "") } catch (err) { // Distinguish "we hit our 15s abort" from "network stack failed" so the // caller can decide differently (retry, longer timeout, offline banner). - // (m8 in the consensus review.) + // The abort fires equally when it kills the fetch OR the body read. (m8) const name = (err as { name?: string } | undefined)?.name if (name === "AbortError") { throw new WorkspaceApiError( @@ -179,7 +186,6 @@ async function req( clearTimeout(timeout) } let json: unknown = undefined - const text = await res.text().catch(() => "") if (text) { try { json = JSON.parse(text) diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 477a472b0a..64a99d03a0 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -43,12 +43,38 @@ export function cachePath(): string { return path.join(Global.Path.state, "altimate-workspace-bindings.json") } +/** Runtime shape check for a parsed cache file — the JSON blob comes from + * disk and could be anything (older CLI version, hand-edited, corrupted + * mid-write). The type assertion alone doesn't guard against e.g. + * ``{"version": 1, "bindings": null}`` which then throws on + * ``cache.bindings[k]``. Discard anything that fails the shape check so + * readers always get a valid ``CacheFile`` or null. (CR round 2.) */ +function isValidCacheFile(raw: unknown): raw is CacheFile { + if (!raw || typeof raw !== "object") return false + const r = raw as Record + if (r.version !== CACHE_VERSION) return false + if (typeof r.tenant !== "string" || !r.tenant) return false + if (typeof r.apiUrl !== "string" || !r.apiUrl) return false + if (!r.bindings || typeof r.bindings !== "object" || Array.isArray(r.bindings)) return false + for (const [k, v] of Object.entries(r.bindings)) { + if (typeof k !== "string") return false + if (!v || typeof v !== "object") return false + const b = v as Record + if (typeof b.datamateId !== "number" || !Number.isInteger(b.datamateId)) return false + if (typeof b.datamateName !== "string") return false + if (b.repoRemote !== null && typeof b.repoRemote !== "string") return false + if (b.projectPath !== null && typeof b.projectPath !== "string") return false + if (typeof b.linkedAt !== "number") return false + } + return true +} + function readCache(): CacheFile | null { const p = cachePath() if (!existsSync(p)) return null try { - const raw = JSON.parse(readFileSync(p, "utf8")) as CacheFile - if (!raw || raw.version !== CACHE_VERSION) return null + const raw = JSON.parse(readFileSync(p, "utf8")) as unknown + if (!isValidCacheFile(raw)) return null return raw } catch (err) { log.warn("workspace binding cache is corrupt, discarding", { diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index f8b2df1707..98e8878ab2 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -762,6 +762,20 @@ async function runFlow( // Plugin registration // ───────────────────────────────────────────────────────────────────────────── +/** Report a fire-and-forget flow failure. The keymap ``run()`` callbacks + * discard the returned promise with ``void``, so any rejection from + * ``recordApprovedBinding`` / ``readLocalBinding`` / anything else awaited + * inside would otherwise surface as an unhandled rejection and terminate + * the TUI process. Log + surface a toast so the user knows the workspace + * flow bailed. (CR round 2.) */ +function reportFlowFailure(api: TuiPluginApi, err: unknown): void { + log.error("workspace flow failed", { err: err instanceof Error ? err.message : String(err) }) + api.ui.toast({ + variant: "error", + message: "Workspace setup failed — see the CLI log for details.", + }) +} + const tui: TuiPlugin = async (api) => { api.keymap.registerLayer({ commands: [ @@ -771,7 +785,7 @@ const tui: TuiPlugin = async (api) => { category: "Altimate", namespace: "internal", run() { - void runFlow(api, api.state.path.directory) + runFlow(api, api.state.path.directory).catch((err) => reportFlowFailure(api, err)) }, }, { @@ -782,7 +796,9 @@ const tui: TuiPlugin = async (api) => { run() { // User-initiated → jump straight to picker (currently-linked marked, // "＋ Create new" as the first row). No Skip funnel — they invoked. - void runOnDemandPicker(api, api.state.path.directory) + runOnDemandPicker(api, api.state.path.directory).catch((err) => + reportFlowFailure(api, err), + ) }, }, ], diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index 7749636ed1..dac334f71c 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -5,16 +5,28 @@ // they need a running TUI harness and are covered by the manual smoke plan. // This file focuses on the deterministic layer: URL parsing, git detection, // state read/write + chmod, latch semantics, and error classification. -import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test" import { existsSync, mkdirSync, rmSync, statSync } from "node:fs" import path from "node:path" import os from "node:os" // Redirect Global.Path.state BEFORE importing the module under test so its -// module-level cachePath() resolves inside the sandbox. +// module-level cachePath() resolves inside the sandbox. Restore the original +// XDG_STATE_HOME in afterAll so parallel test files aren't polluted by our +// process-scoped tempdir. (CR round 2 — test isolation.) +const ORIGINAL_XDG_STATE_HOME = process.env.XDG_STATE_HOME const SANDBOX = path.join(os.tmpdir(), `altimate-workspace-test-${process.pid}-${Date.now()}`) mkdirSync(path.join(SANDBOX, "state"), { recursive: true }) process.env.XDG_STATE_HOME = path.join(SANDBOX, "state") +afterAll(() => { + if (ORIGINAL_XDG_STATE_HOME === undefined) delete process.env.XDG_STATE_HOME + else process.env.XDG_STATE_HOME = ORIGINAL_XDG_STATE_HOME + try { + rmSync(SANDBOX, { recursive: true, force: true }) + } catch { + /* best effort */ + } +}) const { isSkipActive, recordSkip } = await import( "../../../src/plugin/tui/altimate/workspace" @@ -98,8 +110,14 @@ describe("projectNameFromRemote", () => { describe("detectProjectRemote", () => { test("returns undefined when directory is not a git repo", () => { - // /tmp is never a git repo in a stock macOS install. - const result = detectProjectRemote(os.tmpdir()) + // Use a FRESHLY-created empty directory inside SANDBOX rather than + // ``os.tmpdir()`` — an ancestor of the system tmpdir can be inside a + // git worktree (e.g. when the whole /tmp is on a repo-managed volume), + // and ``git remote get-url`` would then walk up and return the parent + // repo's remote. (CR round 2.) + const emptyDir = path.join(SANDBOX, `empty-${Date.now()}`) + mkdirSync(emptyDir, { recursive: true }) + const result = detectProjectRemote(emptyDir) expect(result).toBeUndefined() }) }) From 6d91a3e28ec702c76948a268d958a17cb1a09fa7 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 10:00:23 +0530 Subject: [PATCH 06/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=203?= =?UTF-8?q?=20=E2=80=94=20listDatamates=20shape,=20teardown=20Effect,=20bo?= =?UTF-8?q?dy-read=20abort,=20cache=20best-effort,=20skip-latch=20tenant?= =?UTF-8?q?=20scope?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - listDatamates() now accepts three response envelopes — today's {datamates: [...]}, a bare array, and a generic {data: [...]} — so a backend contract change or compat layer doesn't silently empty the workspace picker. Also filters non-string names alongside the existing integer/positive id guard. (cubic P1) - events.listen() returns an Effect, not a callable — the earlier teardown cast to (() => void) would have thrown on drain, leaving the listener installed. Store the Effect and run it via AppRuntime.runPromise on teardown. Also drain EVERY session that awaited the shared install promise on install failure, not just the one caller — later waiters see success from the promise and stop retrying, leaving permanently-stale entries otherwise. (cubic P2) - req() body-read: dropped the .catch(() => "") wrapper on res.text(). It swallowed the AbortError from the timeout firing during the body read and turned a stalled response into a false "empty body". Any read rejection now rethrows into the outer catch and is classified there (AbortError → timeout WorkspaceApiError). (cubic P2) - recordApprovedBinding() is now best-effort: cache-write failures (read-only state dir, disk full) are logged and swallowed so the caller doesn't report the server-side link as failed and prompt a duplicate retry. (cubic P2) - isValidCacheFile rejects rows with BOTH repoRemote and projectPath null/empty — the offline-fallback render path would otherwise present a phantom workspace with no identity to verify against. (cubic P2) - Skip latch key now includes (tenant, apiUrl) scope, matching the local binding cache. Otherwise a Skip in one Altimate account suppresses the post-scan prompt for the same project in every other account for 7 days. Scope is resolved once by runFlow (currentLatchScope) and threaded into OfferDialog so its sync onSelect can call recordSkip without a mid-render await. (cubic P3) - projectNameFromRemote handles foo.git/ (trailing slash after .git) — earlier .git$ → /$ pipeline missed it because the final / wasn't .git any more. (cubic P2) - Test isolation follow-up: use GIT_CEILING_DIRECTORIES in the detectProjectRemote test so git can't walk up out of SANDBOX and return an ancestor repo's remote. New cross-tenant Skip-latch test. Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../altimate/plugin/onboarding-telemetry.ts | 33 ++++++--- .../src/altimate/workspace/api-client.ts | 32 ++++++--- .../opencode/src/altimate/workspace/detect.ts | 12 +++- .../opencode/src/altimate/workspace/state.ts | 33 +++++++-- .../src/plugin/tui/altimate/workspace.tsx | 72 ++++++++++++++++--- .../test/altimate/plugin/workspace.test.ts | 69 +++++++++++++----- 6 files changed, 195 insertions(+), 56 deletions(-) diff --git a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts index 839050db8d..d37888b04d 100644 --- a/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts +++ b/packages/opencode/src/altimate/plugin/onboarding-telemetry.ts @@ -43,7 +43,12 @@ const workspaceLog = Log.create({ service: "altimate-workspace" }) * second project_scan fires in the same session. */ const pendingWorkspacePromptSessions = new Set() -let workspacePromptUnsubscribe: (() => void) | null = null +/** The unsubscribe returned by ``events.listen()`` is an Effect (not a plain + * function) — running it removes the listener. Store the Effect and execute + * it through ``AppRuntime.runPromise`` on teardown; earlier code cast it to + * ``() => void`` and called it directly, which threw because Effects are not + * callable as functions. (cubic-dev-ai round 3.) */ +let workspacePromptUnsubscribe: Effect.Effect | null = null // Guard against two concurrent scans passing the ``!workspacePromptUnsubscribe`` // check before either install completes — both would then install a listener // and the later assignment would overwrite the first disposer, leaking the @@ -56,6 +61,13 @@ async function armWorkspacePromptOnSessionIdle(sessionID: string): Promise if (workspacePromptUnsubscribe) return if (workspacePromptInstall) return workspacePromptInstall + // Capture the set of pending sessions at install-time so an install + // failure drains EVERY caller that awaited this install, not just the + // one whose sessionID we happen to be handling. Later waiters would + // otherwise see success from the shared promise and stop retrying, + // leaving permanently-stale entries in the pending set. (cubic round 3.) + const armingSessions = new Set(pendingWorkspacePromptSessions) + workspacePromptInstall = (async () => { try { const unsubscribe = await AppRuntime.runPromise( @@ -71,26 +83,29 @@ async function armWorkspacePromptOnSessionIdle(sessionID: string): Promise }) // Once the Set drains, tear the listener down. A later scan // that adds a new pending session re-arms it from scratch. + // ``teardown`` is an Effect — run it through the app runtime, + // don't call it as a function. (cubic round 3.) if (pendingWorkspacePromptSessions.size === 0 && workspacePromptUnsubscribe) { const teardown = workspacePromptUnsubscribe workspacePromptUnsubscribe = null - try { - teardown() - } catch (err) { + AppRuntime.runPromise(teardown).catch((err) => { workspaceLog.warn("session-idle listener teardown failed", { err: String(err), }) - } + }) } }), ), ), ) - workspacePromptUnsubscribe = unsubscribe as unknown as () => void + workspacePromptUnsubscribe = unsubscribe } catch (err) { - // Install failed — drop the pending session so the next scan retries - // from scratch instead of accumulating a stale id that will never fire. - pendingWorkspacePromptSessions.delete(sessionID) + // Install failed — drop every session that was waiting on this install + // so the next scan retries from scratch. Dropping only the current + // caller's ID would leave later waiters (already resolved by the + // shared install promise) with permanently-stale pending entries. + // (cubic round 3.) + for (const sid of armingSessions) pendingWorkspacePromptSessions.delete(sid) workspaceLog.warn("session-idle listener install failed", { err: String(err) }) } finally { workspacePromptInstall = null diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 8598f03fc1..fdc2311b6b 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -168,8 +168,11 @@ async function req( // ``fetch()`` resolves after headers arrive; a server can send headers then // stall the body stream indefinitely, so pulling the body inside the same // try/finally is the difference between our 15s cap and hanging until TCP - // gives up. (CR bot-review round 2.) - text = await res.text().catch(() => "") + // gives up. (CR round 2.) Do NOT wrap in ``.catch(() => "")`` — that + // swallows the AbortError from the timeout firing during the body read + // and turns a stalled response into a false "empty body". Rejection + // rethrows into the outer catch and is classified there. (cubic round 3.) + text = await res.text() } catch (err) { // Distinguish "we hit our 15s abort" from "network stack failed" so the // caller can decide differently (retry, longer timeout, offline banner). @@ -343,13 +346,24 @@ export namespace WorkspaceApi { * doesn't reach the picker as a "NaN" label that the caller then binds * against. */ export async function listDatamates(): Promise { - const body = await req<{ datamates?: Array<{ id: number | string; name: string }> }>( - "GET", - "/", - { base: "/datamates" }, - ) - return (body.datamates ?? []) + // Accept THREE response envelopes — today's ``{datamates: [...]}``, a + // bare ``[...]``, and a generic ``{data: [...]}`` — so a backend + // contract change (or compat layer) doesn't silently empty the picker. + // (cubic-dev-ai round 3.) + type Row = { id: number | string; name: string } + const body = await req("GET", "/", { + base: "/datamates", + }) + let rows: Row[] + if (Array.isArray(body)) { + rows = body + } else if (body && typeof body === "object") { + rows = body.datamates ?? body.data ?? [] + } else { + rows = [] + } + return rows .map((d) => ({ id: Number(d.id), name: d.name })) - .filter((d) => Number.isInteger(d.id) && d.id > 0) + .filter((d) => Number.isInteger(d.id) && d.id > 0 && typeof d.name === "string") } } diff --git a/packages/opencode/src/altimate/workspace/detect.ts b/packages/opencode/src/altimate/workspace/detect.ts index c5d66f378e..71a7d6cc74 100644 --- a/packages/opencode/src/altimate/workspace/detect.ts +++ b/packages/opencode/src/altimate/workspace/detect.ts @@ -52,9 +52,17 @@ export function resolveProjectIdentifier(directory: string): { /** Sensible default workspace name from a remote URL — best-effort. Used to * prefill the workspace-name prompt in the CreateDialog and `altimate link`. - * ``github.com/foo/bar.git`` → ``bar`` ; ``git@github.com:foo/bar.git`` → ``bar``. */ + * ``github.com/foo/bar.git`` → ``bar`` ; ``git@github.com:foo/bar.git`` → ``bar`` ; + * ``https://x/foo/bar.git/`` (trailing slash after .git) → ``bar``. */ export function projectNameFromRemote(remote: string): string { - const trimmed = remote.replace(/\.git$/, "").replace(/\/$/, "") + // Strip trailing slashes FIRST, then the ``.git`` suffix, then any + // trailing slashes the suffix strip exposed. Order matters — the + // previous ``.git$`` → ``/$`` pipeline missed ``.git/`` because the + // final ``/`` wasn't ``.git`` any more. (cubic round 3.) + const trimmed = remote + .replace(/[/]+$/, "") + .replace(/\.git$/, "") + .replace(/[/]+$/, "") const parts = trimmed.split(/[/:]/) return parts[parts.length - 1] || "workspace" } diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 64a99d03a0..5718b58153 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -64,6 +64,13 @@ function isValidCacheFile(raw: unknown): raw is CacheFile { if (typeof b.datamateName !== "string") return false if (b.repoRemote !== null && typeof b.repoRemote !== "string") return false if (b.projectPath !== null && typeof b.projectPath !== "string") return false + // At least one identity — otherwise the cached row can never be verified + // against a project and would surface as a "phantom" workspace on the + // offline-fallback render path. (cubic round 3.) + const hasIdentity = + (typeof b.repoRemote === "string" && b.repoRemote.length > 0) || + (typeof b.projectPath === "string" && b.projectPath.length > 0) + if (!hasIdentity) return false if (typeof b.linkedAt !== "number") return false } return true @@ -122,11 +129,23 @@ export async function recordApprovedBinding( ): Promise { const key = await tenantKey() if (!key) return - const existing = readCache() - const cache: CacheFile = - existing && existing.tenant === key.tenant && existing.apiUrl === key.apiUrl - ? existing - : { version: CACHE_VERSION, tenant: key.tenant, apiUrl: key.apiUrl, bindings: {} } - cache.bindings[directory] = binding - writeCache(cache) + // Best-effort: cache persistence is a UX convenience, not the source of + // truth (the server-side binding is). If the state directory is read-only + // or the disk is full, callers otherwise report "link failed" and prompt + // duplicate retries against a workspace that IS bound server-side. + // (cubic round 3.) + try { + const existing = readCache() + const cache: CacheFile = + existing && existing.tenant === key.tenant && existing.apiUrl === key.apiUrl + ? existing + : { version: CACHE_VERSION, tenant: key.tenant, apiUrl: key.apiUrl, bindings: {} } + cache.bindings[directory] = binding + writeCache(cache) + } catch (err) { + log.warn("could not persist workspace binding cache", { + code: (err as NodeJS.ErrnoException)?.code, + err: String(err), + }) + } } diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 98e8878ab2..d4423687bf 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -43,6 +43,7 @@ import { resolveProjectIdentifier, } from "@/altimate/workspace/detect" import { readLocalBinding, recordApprovedBinding } from "@/altimate/workspace/state" +import { AltimateApi } from "@/altimate/api/client" import { Log } from "@/altimate/util/log" const PLUGIN_ID = "altimate:workspace" @@ -58,21 +59,60 @@ const log = Log.create({ service: "altimate-workspace" }) const SKIP_TTL_MS = 7 * 24 * 60 * 60 * 1000 const KV_SKIP_PREFIX = "altimate.workspace.postScan.skip." -/** Latch key from the project's primary identifier (remote > path). Path-only - * projects also get a latch — sample-scaffold users are still users. */ -function skipKey(id: ProjectIdentifier): string { +/** (tenant, apiUrl) scope for the Skip latch — matches the local binding + * cache's top-level scoping. Without this a Skip in one Altimate account + * suppresses the post-scan prompt for the same project in every other + * account for 7 days. Sync-provided by the caller because ``recordSkip`` is + * invoked inside the dialog's synchronous ``onSelect`` handler. Null means + * "unscoped" (used only when credentials are unavailable). (cubic round 3.) */ +export interface LatchScope { + tenant: string + apiUrl: string +} + +/** Latch key from (tenant, apiUrl, primary identifier). Path-only projects + * also get a latch — sample-scaffold users are still users. */ +function skipKey(id: ProjectIdentifier, scope: LatchScope | null): string { const primary = id.repoRemote ?? id.projectPath ?? "" - return KV_SKIP_PREFIX + createHash("sha1").update(primary).digest("hex") + const scopeString = scope ? `${scope.tenant}|${scope.apiUrl}|` : "" + return ( + KV_SKIP_PREFIX + + createHash("sha1") + .update(scopeString + primary) + .digest("hex") + ) } -function isSkipActive(api: TuiPluginApi, id: ProjectIdentifier, nowMs: number): boolean { - const rec = api.kv.get<{ skippedAt: number }>(skipKey(id)) +function isSkipActive( + api: TuiPluginApi, + id: ProjectIdentifier, + scope: LatchScope | null, + nowMs: number, +): boolean { + const rec = api.kv.get<{ skippedAt: number }>(skipKey(id, scope)) if (!rec || typeof rec.skippedAt !== "number") return false return nowMs - rec.skippedAt < SKIP_TTL_MS } -function recordSkip(api: TuiPluginApi, id: ProjectIdentifier, nowMs: number): void { - api.kv.set(skipKey(id), { skippedAt: nowMs }) +function recordSkip( + api: TuiPluginApi, + id: ProjectIdentifier, + scope: LatchScope | null, + nowMs: number, +): void { + api.kv.set(skipKey(id, scope), { skippedAt: nowMs }) +} + +/** Best-effort ``LatchScope`` from the current CLI credentials. Returns null + * on any credential failure — the latch then falls back to an unscoped key. */ +async function currentLatchScope(): Promise { + try { + if (!(await AltimateApi.isConfigured().catch(() => false))) return null + const creds = await AltimateApi.getCredentials() + return { tenant: creds.altimateInstanceName, apiUrl: creds.altimateUrl } + } catch { + return null + } } // ───────────────────────────────────────────────────────────────────────────── @@ -86,6 +126,11 @@ interface OfferProps { identifier: ProjectIdentifier defaultName: string suppressLatch?: boolean // altimate link on-demand skips the Skip latch + /** (tenant, apiUrl) scope for the Skip latch. Resolved once by the caller + * so the sync ``onSelect`` handler can call ``recordSkip`` without a + * mid-render await. Null when creds are unavailable — latch falls back + * to unscoped. (cubic round 3.) */ + latchScope: LatchScope | null } function OfferDialog(props: OfferProps) { @@ -113,7 +158,8 @@ function OfferDialog(props: OfferProps) { current="create" onSelect={(option) => { if (option.value === "skip") { - if (!props.suppressLatch) recordSkip(props.api, props.identifier, Date.now()) + if (!props.suppressLatch) + recordSkip(props.api, props.identifier, props.latchScope, Date.now()) props.api.ui.dialog.clear() return } @@ -647,9 +693,13 @@ async function runFlow( opts: { suppressLatch?: boolean } = {}, ): Promise { const identifier = resolveProjectIdentifier(directory) + // Resolve latch scope ONCE — passed to isSkipActive here + threaded into + // OfferDialog so its sync onSelect can call recordSkip without awaiting. + // (cubic round 3.) + const latchScope = await currentLatchScope() // Path is always populated by resolveProjectIdentifier — projects without a // git remote (sample dbt scaffolds, scratch dirs) still get a binding offer. - if (!opts.suppressLatch && isSkipActive(api, identifier, Date.now())) { + if (!opts.suppressLatch && isSkipActive(api, identifier, latchScope, Date.now())) { log.info("workspace prompt suppressed by 7-day Skip latch", { identifier: identifier.repoRemote ?? identifier.projectPath, }) @@ -713,6 +763,7 @@ async function runFlow( identifier={identifier} defaultName={defaultName} suppressLatch={opts.suppressLatch} + latchScope={latchScope} /> )) return @@ -754,6 +805,7 @@ async function runFlow( identifier={identifier} defaultName={defaultName} suppressLatch={opts.suppressLatch} + latchScope={latchScope} /> )) } diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index dac334f71c..02172c67e7 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -110,15 +110,23 @@ describe("projectNameFromRemote", () => { describe("detectProjectRemote", () => { test("returns undefined when directory is not a git repo", () => { - // Use a FRESHLY-created empty directory inside SANDBOX rather than - // ``os.tmpdir()`` — an ancestor of the system tmpdir can be inside a - // git worktree (e.g. when the whole /tmp is on a repo-managed volume), - // and ``git remote get-url`` would then walk up and return the parent - // repo's remote. (CR round 2.) + // Cubic round 3 caught that "empty dir under SANDBOX" still shares + // ``os.tmpdir()``'s ancestor chain — if any ancestor is a git worktree, + // ``git remote get-url`` walks up and returns that repo's remote. Set + // ``GIT_CEILING_DIRECTORIES`` to stop the walk at SANDBOX so this test + // is deterministic regardless of where ``os.tmpdir()`` lives on the + // runner. const emptyDir = path.join(SANDBOX, `empty-${Date.now()}`) mkdirSync(emptyDir, { recursive: true }) - const result = detectProjectRemote(emptyDir) - expect(result).toBeUndefined() + const prevCeiling = process.env.GIT_CEILING_DIRECTORIES + process.env.GIT_CEILING_DIRECTORIES = SANDBOX + try { + const result = detectProjectRemote(emptyDir) + expect(result).toBeUndefined() + } finally { + if (prevCeiling === undefined) delete process.env.GIT_CEILING_DIRECTORIES + else process.env.GIT_CEILING_DIRECTORIES = prevCeiling + } }) }) @@ -218,39 +226,50 @@ describe("workspace binding cache", () => { describe("Skip latch", () => { const ident = { repoRemote: "git@github.com:acme/proj-a.git", projectPath: "/work/proj-a" } + const scope = { tenant: "acme", apiUrl: "https://api.acme.example.com" } test("no record → not active", () => { const api = { kv: makeKv() } as any - expect(isSkipActive(api, ident, Date.now())).toBe(false) + expect(isSkipActive(api, ident, scope, Date.now())).toBe(false) }) test("recorded within 7 days → active", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, ident, now) - expect(isSkipActive(api, ident, now + 6 * 24 * 60 * 60 * 1000)).toBe(true) + recordSkip(api, ident, scope, now) + expect(isSkipActive(api, ident, scope, now + 6 * 24 * 60 * 60 * 1000)).toBe(true) }) test("recorded past 7 days → not active", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, ident, now) - expect(isSkipActive(api, ident, now + 8 * 24 * 60 * 60 * 1000)).toBe(false) + recordSkip(api, ident, scope, now) + expect(isSkipActive(api, ident, scope, now + 8 * 24 * 60 * 60 * 1000)).toBe(false) }) test("boundary at exactly 7 days → not active (>= rejects)", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, ident, now) - expect(isSkipActive(api, ident, now + 7 * 24 * 60 * 60 * 1000)).toBe(false) + recordSkip(api, ident, scope, now) + expect(isSkipActive(api, ident, scope, now + 7 * 24 * 60 * 60 * 1000)).toBe(false) }) test("different remotes have independent latches", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 - recordSkip(api, { repoRemote: "git@github.com:acme/one.git", projectPath: "/w/one" }, now) + recordSkip( + api, + { repoRemote: "git@github.com:acme/one.git", projectPath: "/w/one" }, + scope, + now, + ) expect( - isSkipActive(api, { repoRemote: "git@github.com:acme/two.git", projectPath: "/w/two" }, now), + isSkipActive( + api, + { repoRemote: "git@github.com:acme/two.git", projectPath: "/w/two" }, + scope, + now, + ), ).toBe(false) }) @@ -258,9 +277,21 @@ describe("Skip latch", () => { const api = { kv: makeKv() } as any const now = 1_700_000_000_000 const pathOnly = { projectPath: "/scratch/sample-dbt" } - recordSkip(api, pathOnly, now) - expect(isSkipActive(api, pathOnly, now + 3 * 24 * 60 * 60 * 1000)).toBe(true) + recordSkip(api, pathOnly, scope, now) + expect(isSkipActive(api, pathOnly, scope, now + 3 * 24 * 60 * 60 * 1000)).toBe(true) // A different path is not affected. - expect(isSkipActive(api, { projectPath: "/scratch/other" }, now)).toBe(false) + expect(isSkipActive(api, { projectPath: "/scratch/other" }, scope, now)).toBe(false) + }) + + test("different tenant scopes are independent latches (cubic round 3)", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordSkip(api, ident, { tenant: "acme", apiUrl: "https://api.acme.example.com" }, now) + expect( + isSkipActive(api, ident, { tenant: "other", apiUrl: "https://api.acme.example.com" }, now), + ).toBe(false) + expect( + isSkipActive(api, ident, { tenant: "acme", apiUrl: "https://api.other.example.com" }, now), + ).toBe(false) }) }) From fdfc9a3801f069525c5429c2aa3fada9fce7b803 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 10:11:49 +0530 Subject: [PATCH 07/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=204?= =?UTF-8?q?=20=E2=80=94=20Array.isArray=20guard=20on=20listDatamates=20env?= =?UTF-8?q?elope=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If ``/datamates`` returns ``{datamates: }`` or ``{data: }`` (object, string, null — e.g. from a legacy proxy or a schema mismatch), the round-3 unguarded assignment would let a non-array reach ``.map`` and crash the picker before it rendered. ``Array.isArray`` on each envelope field falls back to ``[]`` instead. (cubic round 4.) Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- packages/opencode/src/altimate/workspace/api-client.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index fdc2311b6b..aa1bd3473c 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -358,7 +358,15 @@ export namespace WorkspaceApi { if (Array.isArray(body)) { rows = body } else if (body && typeof body === "object") { - rows = body.datamates ?? body.data ?? [] + // Guard each envelope field with Array.isArray — a non-array + // ``datamates`` or ``data`` value (object / string / null) would + // otherwise slip through and throw on ``.map`` below, taking the + // picker down before it renders. (cubic round 4.) + rows = Array.isArray(body.datamates) + ? body.datamates + : Array.isArray(body.data) + ? body.data + : [] } else { rows = [] } From 910710ee890eae0df57261c4d7274b72fc247a3e Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 10:42:19 +0530 Subject: [PATCH 08/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=205?= =?UTF-8?q?=20=E2=80=94=20pre-map=20null-row=20guard,=20contain=20fire-and?= =?UTF-8?q?-forget=20rejection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cycle-5 findings on files shared with #1100: - **api-client.ts listDatamates** (Kilo warning) — a single ``null`` element in an otherwise-valid rows array threw ``TypeError`` on ``d.id`` before the post-map filter could drop it. That's the exact picker-down failure the round-3/4 envelope guards were added to prevent, just per-element. Filter valid row objects BEFORE the map. - **workspace.tsx createAndBindInline** (Kilo warning) — the post-success tail (``recordApprovedBinding`` + ``open()`` + toasts) sat outside any try inside a fire-and-forget entry point. An unhandled rejection could take the TUI down. Contain the tail in a try/catch that falls back to a plain info toast so the user still sees the URL. Test suite green (33 pass in workspace suites, no regressions in the wider altimate test set). Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../src/altimate/workspace/api-client.ts | 7 ++ .../src/plugin/tui/altimate/workspace.tsx | 67 ++++++++++++------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index aa1bd3473c..10a429e385 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -370,7 +370,14 @@ export namespace WorkspaceApi { } else { rows = [] } + // Filter valid row objects BEFORE map (Kilo cycle 5) — a single ``null`` + // (or non-object) element in an otherwise-valid array would otherwise + // throw ``TypeError: Cannot read properties of null`` on ``d.id`` before + // the post-map filter can drop it. That's the exact picker-down failure + // the round-3/4 envelope guards were added to prevent, just from a + // per-element rather than per-envelope malformed value. return rows + .filter((d): d is Row => d !== null && typeof d === "object") .map((d) => ({ id: Number(d.id), name: d.name })) .filter((d) => Number.isInteger(d.id) && d.id > 0 && typeof d.name === "string") } diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index d4423687bf..f727c005ef 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -230,34 +230,49 @@ 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, - linkedAt: Date.now(), - }) - // Guard against a non-http(s) manage_url — ``open`` dispatches to whatever - // OS handler matches the protocol, so a rogue value could launch an - // unrelated app. Fall through to the info toast (with the URL for manual - // copy) if the URL isn't a safe http/https link. - if (isSafeHttpUrl(res.manage_url)) { - try { - await open(res.manage_url) - api.ui.toast({ - variant: "success", - message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, - }) - return - } catch { - /* fall through to the "open manually" toast below */ + // 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 + // fallout inside the function itself: ``recordApprovedBinding`` already + // swallows its own errors (state.ts is best-effort), but ``open()`` and + // the toast APIs can reject unexpectedly. Fall back to a plain info + // toast so the user still sees the URL. (Kilo cycle 5.) + try { + await recordApprovedBinding(api.state.path.directory, { + datamateId: res.datamate.id, + datamateName: res.datamate.name, + repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, + linkedAt: Date.now(), + }) + // Guard against a non-http(s) manage_url — ``open`` dispatches to whatever + // OS handler matches the protocol, so a rogue value could launch an + // unrelated app. Fall through to the info toast (with the URL for manual + // copy) if the URL isn't a safe http/https link. + if (isSafeHttpUrl(res.manage_url)) { + try { + await open(res.manage_url) + api.ui.toast({ + variant: "success", + message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, + }) + return + } catch { + /* fall through to the "open manually" toast below */ + } } + api.ui.toast({ + variant: "info", + message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, + duration: 10_000, + }) + } catch (err) { + api.ui.toast({ + variant: "info", + message: `Workspace "${res.datamate.name}" created and linked.`, + }) + void err } - api.ui.toast({ - variant: "info", - message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, - duration: 10_000, - }) } /** True when the URL parses and its protocol is exactly ``http:`` or ``https:``. From 7207840e69648697366605f4da3948f601e2317d Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 12:06:08 +0530 Subject: [PATCH 09/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=206?= =?UTF-8?q?=20=E2=80=94=20flag=20opt-in,=20skip=20clock=20rewind,=20cred?= =?UTF-8?q?=20guard,=20canonical=20cache=20key,=20placeholder=20rows,=2040?= =?UTF-8?q?9-fallback=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six correctness fixes landed on the branch; a seventh finding (already-linked "Create new" → orphaned workspace) is documented as deferred because a CLI-only fix isn't possible without a new backend endpoint. - **``ALTIMATE_WORKSPACE`` opt-in only** (Kilo warning) — was routed through ``enabledByExperimental`` and silently inherited ``OPENCODE_EXPERIMENTAL``. Users opted into other experimental features were getting the Workspaces pilot turned on for them, contradicting the "off by default" rollout. Swap to bare ``truthy("ALTIMATE_WORKSPACE")``. - **Skip latch rejects future timestamps** (CodeRabbit minor) — a clock rewind after ``recordSkip`` would produce ``nowMs - skippedAt < 0``, trivially under the 7-day TTL, and suppress the prompt indefinitely. Treat future timestamps as corrupt and re-offer on the next scan. - **``tenantKey`` guards ``getCredentials``** (Kilo warning) — the helper can throw ``SyntaxError`` / ``ZodError`` / raw ``Error`` on corrupt or drifted credentials; those were escaping the "best effort" contract of the state module and terminating fire-and-forget callers. Wrap in try/catch and log-warn. - **``link.ts`` cache key uses canonical identifier** (Kilo warning) — was ``recordApprovedBinding(args.directory, ...)`` which stored under the raw --directory arg; ``altimate-code link -d ./myproj`` and its symlink-resolved twin produced two separate cache rows. Prefer ``identifier.projectPath`` (canonicalized by ``resolveProjectIdentifier``). - **Placeholder rows no longer filtered** (Kilo warning) — ``DialogSelect`` drops ``disabled: true`` options, so the "Loading workspaces..." and "No workspaces yet..." rows never rendered and the picker showed an empty list. Remove ``disabled: true``; the ``value === -1`` guard in ``onSelect`` already closes the dialog on selection. - **409-fallback rebind picks endpoint from conflict detail** (Kilo warning) — was keying off the current project identifier, reproducing the M3 hazard: a path-keyed legacy binding hit ``rebindByRemote`` and 404'd. Derive the endpoint from ``err.detail.project_path`` / ``err.detail.repo_remote`` which the server sends for exactly this purpose. **Deferred to follow-up ticket:** - chatgpt-codex P1 "Create new workspace when already-linked → orphan" needs either a new backend endpoint that creates without binding, or a CLI refactor that calls the plain ``POST /datamates/`` route + rebind. Both are more than a bot-review-cycle fix. Noting so the ticket can be scheduled explicitly. Test suite: green (4058 pass, 0 fail across the altimate suite). Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- packages/core/src/flag/flag.ts | 7 ++- .../opencode/src/altimate/workspace/state.ts | 20 +++++-- packages/opencode/src/cli/cmd/link.ts | 52 +++++++++++++++---- .../src/plugin/tui/altimate/workspace.tsx | 25 ++++++--- 4 files changed, 82 insertions(+), 22 deletions(-) diff --git a/packages/core/src/flag/flag.ts b/packages/core/src/flag/flag.ts index 519d212721..71caffb6c9 100644 --- a/packages/core/src/flag/flag.ts +++ b/packages/core/src/flag/flag.ts @@ -66,8 +66,13 @@ export const Flag = { // altimate_change start — pilot flag for the Workspaces feature (post-scan prompt + // altimate link subcommand). Read as a getter so tests and the runtime `--` middleware // can flip it between plugin activation and command execution. + // + // Opt-in only — deliberately does NOT inherit ``OPENCODE_EXPERIMENTAL`` (as + // ``enabledByExperimental`` would). The pilot ships behind its own explicit + // gate so users already opted into other experimental features don't get + // this one turned on for them. (Kilo cycle 6.) get ALTIMATE_WORKSPACE() { - return enabledByExperimental("ALTIMATE_WORKSPACE") + return truthy("ALTIMATE_WORKSPACE") }, // altimate_change end diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 5718b58153..a14faf4c81 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -107,9 +107,23 @@ function writeCache(cache: CacheFile): void { } async function tenantKey(): Promise<{ tenant: string; apiUrl: string } | null> { - if (!(await AltimateApi.isConfigured())) return null - const c = await AltimateApi.getCredentials() - return { tenant: c.altimateInstanceName, apiUrl: c.altimateUrl } + // Best-effort: ``AltimateApi.getCredentials`` can throw ``SyntaxError`` on + // a corrupt credentials JSON, ``ZodError`` on schema drift, or a raw + // ``Error`` on an unresolvable ``${env:...}`` reference — anything the + // credential-loader library can produce. This helper is the last gate + // between those errors and callers who treat their failures as fatal (the + // TUI's fire-and-forget bind path terminates on unhandled rejections), so + // swallow them and treat as "no credentials". (Kilo cycle 6.) + try { + if (!(await AltimateApi.isConfigured())) return null + const c = await AltimateApi.getCredentials() + return { tenant: c.altimateInstanceName, apiUrl: c.altimateUrl } + } catch (err) { + log.warn("could not resolve workspace credentials for cache scoping", { + err: String(err), + }) + return null + } } /** Read the local binding for ``directory`` — only returns a hit when the diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index fe94d0b1cd..13499ced36 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -203,7 +203,10 @@ async function createThenBindOrRebind( return } } - await recordApprovedBinding(directory, { + // 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). + await recordApprovedBinding(identifier.projectPath ?? directory, { datamateId: created.datamate.id, datamateName: created.datamate.name, repoRemote: created.binding.repo_remote, @@ -267,19 +270,45 @@ async function bindOrRebind( // ``expected_current_datamate_id`` (pre-check gave us nothing) so // this is last-writer-wins. Callers who need optimistic concurrency // should re-run once the network is back and the pre-check succeeds. + // + // Pick the rebind endpoint from the CONFLICT DETAIL, not from the + // current identifier — the existing binding may be keyed by a + // different identifier than the project's current one (path-keyed + // legacy binding + newly-added remote, or vice versa). Keying off + // the current identifier reproduces the M3 hazard on this fallback + // path. (Kilo cycle 6.) spin.stop("Pre-check missed an existing binding — retrying as re-link.", 1) const rebindSpin = prompts.spinner() rebindSpin.start("Re-linking...") try { - res = identifier.repoRemote - ? await WorkspaceApi.rebindByRemote({ - remote: identifier.repoRemote, - targetDatamateId, - }) - : await WorkspaceApi.rebindByPath({ - projectPath: identifier.projectPath!, - targetDatamateId, - }) + // detail.project_path present → the conflicting binding is + // path-keyed; use /by-path. Else the conflict was on repo_remote. + const conflictPath = err.detail.project_path + const conflictRemote = err.detail.repo_remote + if (conflictPath) { + res = await WorkspaceApi.rebindByPath({ + projectPath: conflictPath, + targetDatamateId, + }) + } else if (conflictRemote) { + res = await WorkspaceApi.rebindByRemote({ + remote: conflictRemote, + targetDatamateId, + }) + } else { + // Server didn't tell us which identifier owned the conflict — + // fall back to the current identifier's preference (better than + // nothing, but shouldn't happen with a well-formed 409 body). + res = identifier.repoRemote + ? await WorkspaceApi.rebindByRemote({ + remote: identifier.repoRemote, + targetDatamateId, + }) + : await WorkspaceApi.rebindByPath({ + projectPath: identifier.projectPath!, + targetDatamateId, + }) + } rebindSpin.stop(`Re-linked to "${res.binding.datamate_name}".`) } catch (retryErr) { rebindSpin.stop("Re-link failed.", 1) @@ -290,7 +319,8 @@ async function bindOrRebind( } } } - await recordApprovedBinding(directory, { + // Prefer the canonicalized identifier over the raw --directory (Kilo cycle 6). + await recordApprovedBinding(identifier.projectPath ?? directory, { datamateId: res.binding.datamate_id, datamateName: res.binding.datamate_name, repoRemote: res.binding.repo_remote, diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index f727c005ef..36261c6ef9 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -91,7 +91,14 @@ function isSkipActive( ): boolean { const rec = api.kv.get<{ skippedAt: number }>(skipKey(id, scope)) if (!rec || typeof rec.skippedAt !== "number") return false - return nowMs - rec.skippedAt < SKIP_TTL_MS + // Reject records timestamped in the future — a system-clock rewind after + // ``recordSkip`` would otherwise produce ``nowMs - rec.skippedAt < 0``, + // trivially below the 7-day TTL, and suppress the prompt indefinitely. + // Treat future timestamps as "corrupt, retry" so the next scan re-offers. + // (CodeRabbit cycle 6.) + const delta = nowMs - rec.skippedAt + if (delta < 0) return false + return delta < SKIP_TTL_MS } function recordSkip( @@ -475,18 +482,21 @@ function PickerDialog(props: PickerProps) { } } - // While loading (or on error before dialog closes), render an empty select as - // a placeholder — DialogSelect requires the options array up front and doesn't - // have a native busy state; the empty list closes to a "no workspaces" message. + // While loading (or on error before dialog closes), render a placeholder + // row the user can dismiss with Enter. NOTE: DialogSelect's ``filtered()`` + // drops rows with ``disabled: true`` (packages/tui/src/ui/dialog-select.tsx), + // so the placeholder MUST be rendered without that flag — otherwise the + // picker shows an empty list, hiding both the loading state and the + // "no workspaces yet" hint. Selection is dispatched to ``value === -1`` + // in ``onSelect`` below and simply clears the dialog. (Kilo cycle 6.) const options = () => { const list = datamates() - if (!list) return [{ title: "Loading workspaces...", value: -1, disabled: true }] + if (!list) return [{ title: "Loading workspaces...", value: -1 }] if (list.length === 0) return [ { title: "No workspaces yet — cancel and pick 'Create a new workspace' instead.", value: -1, - disabled: true, }, ] return list.map((dm: DatamateRef) => ({ title: dm.name, value: dm.id })) @@ -547,7 +557,8 @@ function OnDemandPickerDialog(props: OnDemandPickerProps) { const options = () => { const list = datamates() - if (!list) return [{ title: "Loading workspaces...", value: -1, disabled: true }] + // No ``disabled: true`` — DialogSelect filters those out (Kilo cycle 6). + if (!list) return [{ title: "Loading workspaces...", value: -1 }] // Deliberately short titles + hint in description so the dialog's narrow // width doesn't truncate either. The "●" marker stays in the title (single // char, cheap) so the currently-linked row is scannable at a glance. From afdcd46e714327f1c51231759573b392625aaddf Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 14 Aug 2026 17:36:57 +0530 Subject: [PATCH 10/14] feat(workspace): browser-based workspace creation handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a browser handoff for creating and linking a Workspace: CLI opens the SaaS approval modal on `.ws.myaltimate.com/create-and-link` with the current project's context (git remote or path + auto-derived name), user approves, the SaaS creates a workspace and delivers its ID back to the CLI via a loopback callback (same pattern as gateway sign-in). CLI then binds the current project to that workspace via the existing `POST /bind`. Additive to `feat/agent-workspaces` — every pre-existing option in the post-scan dialog and `altimate-code link` picker (Create quick workspace, Link to existing, Skip, workspace-picker rows) continues to work unchanged. The new "Set up in browser" option auto-hides when the deployment isn't supported (localhost, enterprise, custom domain) — freemium only for pilot. - New `packages/opencode/src/altimate/workspace/browser-handoff.ts`: loopback listener (own instance per flow, port walk 7317..7325 with natural fallback past a live OAuth listener), tenant-mismatch guard, typed failure reasons. Duplicates the loopback pattern from `altimate.ts` deliberately — shared-helper refactor is a follow-up ticket once both flows have prod experience. - Post-scan `OfferDialog`: adds "Set up in browser (recommended)" as the default when available, sitting alongside the existing options. - `altimate-code link` picker: adds "＋ Set up in browser" as the first row when available. - Handles browser-open failures with a copy-URL fallback; 15-min timeout; explicit cancel via SaaS-delivered `?error=cancelled`. Tests: 14 new unit tests for browser-handoff (URL resolution, pre-flight failures, end-to-end via dependency-injected browser opener, port walk past a squatting listener). 32/32 workspace + plugin tests pass. --- .../src/altimate/workspace/browser-handoff.ts | 341 ++++++++++++++++++ packages/opencode/src/cli/cmd/link.ts | 135 ++++++- .../src/plugin/tui/altimate/workspace.tsx | 199 +++++++++- .../workspace/browser-handoff.test.ts | 261 ++++++++++++++ 4 files changed, 907 insertions(+), 29 deletions(-) create mode 100644 packages/opencode/src/altimate/workspace/browser-handoff.ts create mode 100644 packages/opencode/test/altimate/workspace/browser-handoff.test.ts diff --git a/packages/opencode/src/altimate/workspace/browser-handoff.ts b/packages/opencode/src/altimate/workspace/browser-handoff.ts new file mode 100644 index 0000000000..78a0bc7abf --- /dev/null +++ b/packages/opencode/src/altimate/workspace/browser-handoff.ts @@ -0,0 +1,341 @@ +// altimate_change - new file +// +// Browser-based workspace creation handoff. CLI opens Ralph's SaaS approval +// modal at ``.ws.myaltimate.com/create-and-link`` with the current +// project's context, user approves, the SaaS creates a workspace and delivers +// its ID back to the CLI via a loopback callback. The CLI then binds the +// current project to that workspace via the existing +// ``POST /datamate-project-bindings/bind`` endpoint. +// +// This module deliberately DUPLICATES the loopback listener pattern from +// ``altimate/plugin/altimate.ts`` rather than sharing a helper — the two flows +// are similar enough that a naive extraction would trade duplication for +// coupling on state/global lifecycle. Refactor to a shared helper is a +// follow-up ticket once both flows have prod experience; the port range +// (7317..7325) is walked independently by each listener instance so a live +// OAuth server on 7317 forces workspace-handoff to bind 7318 without either +// close operation affecting the other. +// +// See docs `workspace-browser-handoff-plan-v3.md` for the design context. +import { createServer, type Server } from "http" +import { randomBytes } from "crypto" +import open from "open" + +import { AltimateApi } from "@/altimate/api/client" +import { Log } from "@/altimate/util/log" + +import type { ProjectIdentifier } from "./api-client" + +// Freemium is the only deployment served by the workspace stack today. When +// altimate-backend goes multi-deployment (enterprise), extend this to a small +// mapping. Returning null means "not supported here" — the CLI hides the +// browser-handoff option entirely rather than open a broken URL. +const FREEMIUM_API_HOST = "api.myaltimate.com" +const FREEMIUM_WORKSPACE_HOST = "ws.myaltimate.com" + +// Loopback port range for the workspace-bound callback. Shared with the OAuth +// sign-in listener in altimate.ts — each listener walks independently, so a +// live OAuth server on 7317 forces us to 7318 (or later) transparently. +const CALLBACK_PORT_MIN = 7317 +const CALLBACK_PORT_MAX = 7325 + +const DEFAULT_TIMEOUT_MS = 15 * 60 * 1000 +const DELIVERY_HTML_SUCCESS = `Altimate Code + +

Workspace ready

Return to your terminal to finish linking.

+` + +const log = Log.create({ service: "altimate-workspace-handoff" }) + +function escapeHtml(s: string): string { + return s.replace( + /[&<>"']/g, + (c) => ({ "&": "&", "<": "<", ">": ">", '"': """, "'": "'" })[c] as string, + ) +} + +function htmlError(msg: string): string { + return `Altimate Code + +

Workspace handoff failed

${escapeHtml(msg)}

+

Please return to your terminal and try again.

` +} + +export type HandoffFailureReason = + | "unavailable" // resolveWorkspaceWebUrl returned null (not freemium) + | "not_configured" // CLI credentials not present + | "timeout" // 15-min window expired + | "cancelled" // user hit Cancel in the browser + | "tenant_mismatch" // callback tenant != credentials tenant + | "port_exhausted" // 7317..7325 all in use + | "browser_open_failed" + | "error" + +export interface HandoffSuccess { + ok: true + workspaceId: number + tenant: string +} +export interface HandoffFailure { + ok: false + reason: HandoffFailureReason + message?: string + authorizeUrl?: string // set for browser_open_failed so caller can copy-paste +} +export type HandoffResult = HandoffSuccess | HandoffFailure + +/** Compute the workspace-stack URL for a given API host + tenant, or null if + * this deployment isn't supported (localhost, enterprise, custom domain). + * + * Dev escape hatch: ``ALTIMATE_WORKSPACE_WEB_URL`` overrides the map lookup + * when set (must be a well-formed URL). Used for local integration testing + * against a non-freemium SaaS instance. Not something production users touch. */ +export function resolveWorkspaceWebUrl(altimateUrl: string, tenant: string): URL | null { + const override = process.env["ALTIMATE_WORKSPACE_WEB_URL"] + if (override) { + try { + return new URL(override) + } catch { + return null + } + } + try { + const apiHost = new URL(altimateUrl).host + if (apiHost !== FREEMIUM_API_HOST) return null + return new URL(`https://${tenant}.${FREEMIUM_WORKSPACE_HOST}`) + } catch { + return null + } +} + +interface HandoffPending { + state: string + expectedTenant: string + resolve: (v: HandoffSuccess) => void + reject: (err: Error & { handoffReason?: HandoffFailureReason }) => void +} + +function markReason(err: E, reason: HandoffFailureReason): E & { handoffReason: HandoffFailureReason } { + return Object.assign(err, { handoffReason: reason }) +} + +/** Start a per-flow loopback listener on the first available port in the + * shared 7317..7325 range. Own server, own pending map — no coupling to the + * OAuth listener in altimate.ts. */ +async function startListener(pending: HandoffPending): Promise<{ server: Server; port: number }> { + const server = createServer((req, res) => { + const port = (server.address() as { port?: number } | null)?.port ?? CALLBACK_PORT_MIN + const url = new URL(req.url || "/", `http://127.0.0.1:${port}`) + if (url.pathname !== "/workspace-bound") { + res.writeHead(404) + res.end("Not found") + return + } + + const respond = (status: number, body: string) => { + res.writeHead(status, { "Content-Type": "text/html" }) + res.end(body) + } + + // Validate state FIRST — a request without the right state can neither + // cancel nor deliver anything. + const state = url.searchParams.get("state") + if (!state || state !== pending.state) { + respond(400, htmlError("Invalid or unknown workspace-handoff state")) + return + } + + // Respond BEFORE resolving/rejecting the pending flow — the reject path + // closes the listener via closeListener(), which can race with the + // response flush and leave the client fetch hanging. Order matters. + const error = url.searchParams.get("error") + if (error) { + const reason: HandoffFailureReason = error === "cancelled" ? "cancelled" : "error" + respond(200, htmlError(error === "cancelled" ? "Cancelled by user" : error)) + pending.reject(markReason(new Error(error), reason)) + return + } + + const workspaceIdRaw = url.searchParams.get("workspace_id") + const tenant = url.searchParams.get("tenant") + if (!workspaceIdRaw || !tenant) { + const msg = "Missing workspace_id or tenant in callback" + respond(400, htmlError(msg)) + pending.reject(markReason(new Error(msg), "error")) + return + } + + if (tenant !== pending.expectedTenant) { + // Cross-tenant defence: user created the workspace in a tenant that + // doesn't match the CLI's credentials. Refuse the bind — the workspace + // ID is tenant-schema-local so binding here would 404 or, worse, hit an + // unrelated workspace in the CLI's tenant. + const msg = `Workspace was created in tenant "${tenant}" but the CLI is signed into "${pending.expectedTenant}"` + respond(400, htmlError(msg)) + pending.reject(markReason(new Error(msg), "tenant_mismatch")) + return + } + + const workspaceId = Number(workspaceIdRaw) + if (!Number.isFinite(workspaceId) || workspaceId <= 0) { + const msg = `Invalid workspace_id: ${workspaceIdRaw}` + respond(400, htmlError(msg)) + pending.reject(markReason(new Error(msg), "error")) + return + } + + respond(200, DELIVERY_HTML_SUCCESS) + pending.resolve({ ok: true, workspaceId, tenant }) + }) + + // Walk 7317..7325 — each server instance is independent, so a squatting + // OAuth listener on 7317 just makes us bind 7318. + const tried: number[] = [] + let lastErr: NodeJS.ErrnoException | undefined + for (let port = CALLBACK_PORT_MIN; port <= CALLBACK_PORT_MAX; port++) { + tried.push(port) + try { + await new Promise((resolve, reject) => { + const onErr = (err: NodeJS.ErrnoException) => reject(err) + server.once("error", onErr) + server.listen(port, "127.0.0.1", () => { + server.removeListener("error", onErr) + resolve() + }) + }) + return { server, port } + } catch (err) { + lastErr = err as NodeJS.ErrnoException + // Defensive cleanup in case any listeners linger after a rejected bind. + server.removeAllListeners("error") + if (lastErr.code !== "EADDRINUSE") break + } + } + + server.close() + const code = lastErr?.code + throw markReason( + new Error( + code === "EADDRINUSE" + ? `Every port in ${CALLBACK_PORT_MIN}-${CALLBACK_PORT_MAX} is in use (tried ${tried.join(", ")}). Close what's using them (e.g. \`lsof -i :${CALLBACK_PORT_MIN}\`) and try again.` + : `Could not start the workspace-handoff server: ${lastErr instanceof Error ? lastErr.message : String(lastErr)}`, + ), + "port_exhausted", + ) +} + +export interface OpenBrowserHandoffInput { + identifier: ProjectIdentifier + projectName: string +} + +/** Full browser-handoff flow. Returns the created/picked workspace ID on + * success, or a typed failure reason on any error path. Never throws — every + * error is expressed as ``{ok: false, reason}`` so the caller can toast the + * appropriate message. */ +export async function openWorkspaceBrowserHandoff(input: OpenBrowserHandoffInput): Promise { + return runHandoffWithOpener(input, (url) => open(url).then(() => undefined)) +} + +/** Same as ``openWorkspaceBrowserHandoff`` but takes the browser-open callback + * as a dependency so tests can inject a fake that fires the loopback callback + * synchronously instead of launching a real browser. Not exported from the + * package barrel — only tests import this directly. */ +export async function runHandoffWithOpener( + input: OpenBrowserHandoffInput, + openBrowser: (url: string) => Promise, +): Promise { + if (!(await AltimateApi.isConfigured().catch(() => false))) { + return { ok: false, reason: "not_configured" } + } + const creds = await AltimateApi.getCredentials() + const webUrl = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) + if (!webUrl) return { ok: false, reason: "unavailable" } + + const state = randomBytes(16).toString("hex") + + // Register pending, then bind the listener. Timeout owns rejection with + // reason "timeout"; the listener's own reject paths mark their own reasons. + let listenerHandle: { server: Server; port: number } | undefined + const closeListener = () => { + if (listenerHandle) { + try { + listenerHandle.server.close() + } catch { + /* best effort */ + } + listenerHandle = undefined + } + } + + const settled = new Promise((resolve) => { + const pending: HandoffPending = { + state, + expectedTenant: creds.altimateInstanceName, + resolve: (v) => { + closeListener() + clearTimeout(timeoutHandle) + resolve(v) + }, + reject: (err) => { + closeListener() + clearTimeout(timeoutHandle) + const reason = (err as { handoffReason?: HandoffFailureReason }).handoffReason ?? "error" + const authorizeUrl = (err as { authorizeUrl?: string }).authorizeUrl + resolve({ + ok: false, + reason, + message: err.message, + ...(authorizeUrl ? { authorizeUrl } : {}), + }) + }, + } + const timeoutHandle = setTimeout(() => { + pending.reject(markReason(new Error("Timed out waiting for browser workspace handoff"), "timeout")) + }, DEFAULT_TIMEOUT_MS) + + ;(async () => { + try { + listenerHandle = await startListener(pending) + } catch (err) { + const reason = (err as { handoffReason?: HandoffFailureReason }).handoffReason ?? "error" + pending.reject(markReason(err as Error, reason)) + return + } + + // Import buildCliContext lazily so this module doesn't pull altimate.ts + // into every consumer's import graph at load time. + const { buildCliContext } = await import("../plugin/altimate") + const cliContext = await buildCliContext().catch((err) => { + log.warn("buildCliContext failed; proceeding without", { err: String(err) }) + return "" + }) + + const redirect = `http://127.0.0.1:${listenerHandle.port}/workspace-bound` + const target = new URL("/create-and-link", webUrl) + target.searchParams.set("client", "altimate-code") + target.searchParams.set("redirect", redirect) + target.searchParams.set("state", state) + if (input.identifier.repoRemote) target.searchParams.set("project_remote", input.identifier.repoRemote) + if (input.identifier.projectPath) target.searchParams.set("project_path", input.identifier.projectPath) + target.searchParams.set("project_name", input.projectName) + const authorizeUrl = cliContext + ? `${target.toString()}#cli_context=${encodeURIComponent(cliContext)}` + : target.toString() + + try { + await openBrowser(authorizeUrl) + } catch (err) { + // Browser open failed. Preserve the URL so the caller can copy-paste. + pending.reject( + Object.assign( + markReason(new Error(`Could not open browser: ${err instanceof Error ? err.message : String(err)}`), "browser_open_failed"), + { authorizeUrl }, + ), + ) + } + })() + }) + + return settled +} diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 13499ced36..0cfe11d4d5 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -32,9 +32,15 @@ import { projectNameFromRemote, resolveProjectIdentifier, } from "@/altimate/workspace/detect" +import { + openWorkspaceBrowserHandoff, + resolveWorkspaceWebUrl, + type HandoffResult, +} from "@/altimate/workspace/browser-handoff" import { recordApprovedBinding } from "@/altimate/workspace/state" const CREATE_NEW_SENTINEL = "__create_new__" +const SET_UP_IN_BROWSER_SENTINEL = "__browser_handoff__" export const LinkCommand = cmd({ command: "link", @@ -101,13 +107,29 @@ export const LinkCommand = cmd({ const currentId = existing?.datamate.id const currentName = existing?.datamate.name + // Only offer the browser-based handoff when the deployment supports it + // (freemium only today). Enterprise / localhost / custom-domain callers + // silently fall back to the CLI-side quick create. + const creds = await AltimateApi.getCredentials() + const browserAvailable = + resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) !== null + const options: Array<{ value: string; label: string; hint?: string }> = [ + ...(browserAvailable + ? [ + { + value: SET_UP_IN_BROWSER_SENTINEL, + label: `＋ Set up in browser "${autoName}"`, + hint: "Approve in the Altimate SaaS; CLI links your project automatically.", + }, + ] + : []), { value: CREATE_NEW_SENTINEL, - label: `＋ Create a new workspace "${autoName}"`, + label: `＋ Create a quick workspace "${autoName}" here`, hint: existing - ? "Creates a new workspace and repoints this project to it." - : "Named from this project; rename in the SaaS after.", + ? "Creates a new workspace and repoints this project to it (no browser step)." + : "No browser step; configure integrations later in the SaaS.", }, ...list.map((dm) => ({ value: String(dm.id), @@ -129,6 +151,11 @@ export const LinkCommand = cmd({ return } + if (pick === SET_UP_IN_BROWSER_SENTINEL) { + await runBrowserHandoff(identifier, autoName, args.directory) + return + } + if (pick === CREATE_NEW_SENTINEL) { await createThenBindOrRebind(identifier, autoName, args.directory, existing) return @@ -144,12 +171,102 @@ export const LinkCommand = cmd({ }, }) -/** "＋ Create a new workspace" flow. When the project is already linked, this - * MUST rebind after create — otherwise the new workspace is a 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. */ +/** Browser-based create-and-bind flow. Same handoff module the TUI post-scan + * dialog uses; on success, the CLI calls the existing bind endpoint to link + * the current project to the newly-created workspace. When the project is + * already linked, bindExisting will 409; the caller re-runs and picks + * "＋ Create a quick workspace here" instead to trigger the create-and-rebind + * path. (Full create-then-rebind via the browser flow is deferred — the + * SaaS approval screen doesn't yet know how to receive a "rebind after + * create" instruction from the CLI.) */ +async function runBrowserHandoff( + identifier: ProjectIdentifier, + projectName: string, + directory: string, +): Promise { + const spin = prompts.spinner() + spin.start("Waiting for browser approval...") + const result: HandoffResult = await openWorkspaceBrowserHandoff({ identifier, projectName }) + if (!result.ok) { + spin.stop(handoffFailureMessage(result), 1) + process.exitCode = 1 + return + } + spin.stop(`Workspace approved. Binding to project...`) + const bindSpin = prompts.spinner() + bindSpin.start("Linking workspace...") + try { + const res = await WorkspaceApi.bindExisting(result.workspaceId, identifier) + await recordApprovedBinding(directory, { + datamateId: res.binding.datamate_id, + datamateName: res.binding.datamate_name, + repoRemote: res.binding.repo_remote, + projectPath: res.binding.project_path, + linkedAt: Date.now(), + }) + bindSpin.stop(`Linked to "${res.binding.datamate_name}".`) + const manageUrl = await manageUrlFor(res.binding.datamate_id) + if (manageUrl) prompts.log.info(`Manage it at: ${manageUrl}`) + prompts.outro("Done.") + } catch (err) { + bindSpin.stop("Link failed.", 1) + if (err instanceof ConflictError) { + prompts.log.error( + `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Workspace "${projectName}" was created but is not linked — re-run \`altimate-code link\` and pick a different action to switch, or delete the new workspace in the SaaS.`, + ) + } else if (err instanceof NotFoundError) { + prompts.log.error("Workspace not found — the tenant or workspace may have changed.") + } else if (err instanceof ForbiddenError) { + prompts.log.error("Only the workspace owner can bind projects to it.") + } else { + prompts.log.error(err instanceof Error ? err.message : String(err)) + } + process.exitCode = 1 + } +} + +/** Best-effort manage-workspace URL for the current credentials. Returns null + * on BYOK / unresolvable deployments — callers omit the "Manage it at" line. */ +async function manageUrlFor(workspaceId: number): Promise { + try { + const creds = await AltimateApi.getCredentials() + const base = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) + if (!base) return null + return `${base.toString().replace(/\/$/, "")}/w/${workspaceId}` + } catch { + return null + } +} + +function handoffFailureMessage(result: Extract): string { + switch (result.reason) { + case "unavailable": + return "Browser handoff isn't available for this deployment." + case "not_configured": + return "Altimate credentials not configured — sign in first." + case "timeout": + return "Timed out waiting for browser approval (15 min)." + case "cancelled": + return "Cancelled by user." + case "tenant_mismatch": + return result.message ?? "Workspace was set up in a different tenant than the CLI's credentials." + case "port_exhausted": + return result.message ?? "Loopback ports 7317-7325 all in use." + case "browser_open_failed": + return `Could not open browser${result.authorizeUrl ? `. Open manually: ${result.authorizeUrl}` : "."}` + case "aborted": + return result.message ?? "Browser handoff was cancelled." + default: + return result.message ?? "Browser handoff failed." + } +} + +/** "＋ Create a quick workspace here" flow. When the project is already + * linked, this MUST rebind after create — otherwise the new workspace is a + * 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( identifier: ProjectIdentifier, name: string, diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 36261c6ef9..c9e08bdcab 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -37,6 +37,11 @@ import { type ProjectBindingLookup, type ProjectIdentifier, } from "@/altimate/workspace/api-client" +import { + openWorkspaceBrowserHandoff, + resolveWorkspaceWebUrl, + type HandoffResult, +} from "@/altimate/workspace/browser-handoff" import { projectNameFromPath, projectNameFromRemote, @@ -50,6 +55,16 @@ const PLUGIN_ID = "altimate:workspace" const log = Log.create({ service: "altimate-workspace" }) +/** True when the browser-based workspace-creation handoff is available for + * the current credentials (freemium only today). Wrapped so both the post-scan + * flow and the on-demand `altimate-code link` picker can hide the option + * consistently when the deployment isn't supported. */ +async function isBrowserHandoffAvailable(): Promise { + if (!(await AltimateApi.isConfigured().catch(() => false))) return false + const creds = await AltimateApi.getCredentials() + return resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) !== null +} + // ───────────────────────────────────────────────────────────────────────────── // Skip latch (TUI-only). Uses TuiPluginApi.kv — persistent across sessions // via packages/tui/src/context/kv.tsx (state/kv.json). The `altimate link` @@ -133,6 +148,12 @@ interface OfferProps { identifier: ProjectIdentifier defaultName: string suppressLatch?: boolean // altimate link on-demand skips the Skip latch + /** True when this deployment supports the browser-based workspace-creation + * handoff (i.e. ``resolveWorkspaceWebUrl`` returned non-null for the current + * credentials). Resolved by the caller so the dialog doesn't need to await + * on mount. When false, the "Set up in browser" option is hidden and the + * dialog falls back to the pre-browser-handoff behavior. */ + browserAvailable: boolean /** (tenant, apiUrl) scope for the Skip latch. Resolved once by the caller * so the sync ``onSelect`` handler can call ``recordSkip`` without a * mid-render await. Null when creds are unavailable — latch falls back @@ -142,27 +163,38 @@ interface OfferProps { function OfferDialog(props: OfferProps) { const identLabel = () => props.identifier.repoRemote ?? props.identifier.projectPath ?? "this project" + const options = [ + ...(props.browserAvailable + ? [ + { + title: "Set up in browser (recommended)", + value: "browser", + description: `Approve and name "${props.defaultName}" in the Altimate SaaS; the CLI links your project automatically.`, + }, + ] + : []), + { + title: "Create quick workspace here", + value: "create", + description: `Auto-named "${props.defaultName}" from this repo — no browser step. Configure integrations later in the SaaS.`, + }, + { + title: "Link to an existing workspace", + value: "link", + description: "Attach this project to a workspace you already own.", + }, + { + title: "Skip for now", + value: "skip", + description: props.suppressLatch ? "Close this prompt." : "Won't ask again for 7 days.", + }, + ] + const defaultValue = props.browserAvailable ? "browser" : "create" return ( { if (option.value === "skip") { if (!props.suppressLatch) @@ -170,9 +202,14 @@ function OfferDialog(props: OfferProps) { props.api.ui.dialog.clear() return } + if (option.value === "browser") { + void runBrowserHandoff(props.api, props.identifier, props.defaultName) + return + } if (option.value === "create") { - // Auto-name from git repo — no name prompt. The SaaS UI is the place to - // rename / configure; the CLI's job is just to establish the binding. + // 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) return } @@ -185,6 +222,120 @@ function OfferDialog(props: OfferProps) { ) } +/** Post-scan / on-demand browser-handoff runner. Opens the SaaS approval + * modal, waits for the callback, and binds the current project to the + * returned workspace via the existing ``POST /bind`` endpoint. Every failure + * mode surfaces as a toast; the user can always fall back to another option + * by re-invoking the dialog. */ +async function runBrowserHandoff( + api: TuiPluginApi, + identifier: ProjectIdentifier, + projectName: string, +): Promise { + api.ui.dialog.clear() + api.ui.toast({ + variant: "info", + message: "Opening browser to set up your workspace — return here once you approve.", + }) + const result: HandoffResult = await openWorkspaceBrowserHandoff({ identifier, projectName }) + if (!result.ok) { + toastHandoffFailure(api, result) + return + } + // Handoff succeeded — bind the project to the returned workspace via the + // existing bind endpoint. Same code path as PickerDialog's attach mode. + try { + const res = await WorkspaceApi.bindExisting(result.workspaceId, identifier) + 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(), + }) + api.ui.toast({ + variant: "success", + message: `Linked to workspace "${res.binding.datamate_name}".`, + }) + } catch (err) { + if (err instanceof ConflictError) { + api.ui.toast({ + variant: "warning", + message: `This project is already linked to "${err.detail.existing_datamate_name ?? "another workspace"}". Run \`altimate-code link\` to change.`, + }) + } else if (err instanceof NotFoundError) { + api.ui.toast({ + variant: "error", + message: "Workspace not found — the tenant or workspace may have changed. Try again.", + }) + } else if (err instanceof ForbiddenError) { + api.ui.toast({ + variant: "error", + message: "Only the workspace owner can bind projects to it.", + }) + } else { + api.ui.toast({ + variant: "error", + message: err instanceof Error ? err.message : "Failed to bind workspace", + }) + } + } +} + +function toastHandoffFailure(api: TuiPluginApi, result: Extract): void { + switch (result.reason) { + case "unavailable": + // Should not happen if browserAvailable was checked, but guard anyway. + api.ui.toast({ + variant: "warning", + message: "Browser-based workspace setup isn't available for this deployment. Use \"Create quick workspace here\" instead.", + }) + break + case "not_configured": + api.ui.toast({ + variant: "error", + message: "Altimate credentials not configured — sign in first, then re-run.", + }) + break + case "timeout": + api.ui.toast({ + variant: "warning", + message: "Workspace setup timed out (15 min). Re-run when you're ready.", + }) + break + case "cancelled": + api.ui.toast({ + variant: "info", + message: "Workspace setup cancelled.", + }) + break + case "tenant_mismatch": + api.ui.toast({ + variant: "error", + message: result.message ?? "Workspace was set up under a different account than the CLI is signed into.", + }) + break + case "port_exhausted": + api.ui.toast({ + variant: "error", + message: result.message ?? "Local ports 7317-7325 all in use — free one and try again.", + }) + break + case "browser_open_failed": + api.ui.toast({ + variant: "error", + message: `Could not open browser. ${result.authorizeUrl ? `Open this URL manually: ${result.authorizeUrl}` : ""}`, + duration: 15_000, + }) + break + default: + api.ui.toast({ + variant: "error", + message: result.message ?? "Workspace setup failed.", + }) + } +} + async function createAndBindInline( api: TuiPluginApi, identifier: ProjectIdentifier, @@ -736,6 +887,12 @@ async function runFlow( ? projectNameFromRemote(identifier.repoRemote) : projectNameFromPath(identifier.projectPath) + // Whether the browser-based handoff is available for this deployment. The + // OfferDialog hides the "Set up in browser" option when false, silently + // falling back to the pre-browser-handoff behavior. Compute here (once, + // async) so the dialog itself stays sync. + const browserAvailable = await isBrowserHandoffAvailable() + let serverBinding: ProjectBindingLookup | null | undefined try { serverBinding = await WorkspaceApi.getBindingForProject(identifier) @@ -789,6 +946,7 @@ async function runFlow( identifier={identifier} defaultName={defaultName} suppressLatch={opts.suppressLatch} + browserAvailable={browserAvailable} latchScope={latchScope} /> )) @@ -831,6 +989,7 @@ async function runFlow( identifier={identifier} defaultName={defaultName} suppressLatch={opts.suppressLatch} + browserAvailable={browserAvailable} latchScope={latchScope} /> )) diff --git a/packages/opencode/test/altimate/workspace/browser-handoff.test.ts b/packages/opencode/test/altimate/workspace/browser-handoff.test.ts new file mode 100644 index 0000000000..386e89b589 --- /dev/null +++ b/packages/opencode/test/altimate/workspace/browser-handoff.test.ts @@ -0,0 +1,261 @@ +// altimate_change - new file +// Unit coverage for the browser-based workspace-creation handoff. +// (packages/opencode/src/altimate/workspace/browser-handoff.ts.) +// +// Uses ``runHandoffWithOpener`` (dependency-injected browser-open callback) +// so tests fire a synthetic callback at the live loopback listener instead of +// launching a real browser. The listener itself binds to 127.0.0.1, walks +// 7317..7325, and processes real HTTP requests — this is genuine end-to-end +// coverage for the callback validation path. +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { createServer } from "node:net" + +import { AltimateApi } from "../../../src/altimate/api/client" +import { + openWorkspaceBrowserHandoff, + resolveWorkspaceWebUrl, + runHandoffWithOpener, +} from "../../../src/altimate/workspace/browser-handoff" + +// ── credential stubbing ───────────────────────────────────────────────────── +const originalIsConfigured = AltimateApi.isConfigured +const originalGetCreds = AltimateApi.getCredentials +type Creds = Awaited> +function stubCreds(tenant: string, apiUrl: string) { + ;(AltimateApi as unknown as { isConfigured: () => Promise }).isConfigured = + async () => true + ;(AltimateApi as unknown as { getCredentials: () => Promise }).getCredentials = + async () => + ({ + altimateInstanceName: tenant, + altimateUrl: apiUrl, + altimateApiKey: "dummy", + }) as Creds +} +function unstubCreds() { + ;(AltimateApi as unknown as { isConfigured: typeof originalIsConfigured }).isConfigured = + originalIsConfigured + ;(AltimateApi as unknown as { getCredentials: typeof originalGetCreds }).getCredentials = + originalGetCreds +} + +// ── helpers ───────────────────────────────────────────────────────────────── + +/** Parse the authorize URL the CLI wants to open; extract the loopback port + * and CSRF state so tests can fire the crafted callback at the right address. */ +function parseHandoffUrl(url: string): { port: number; state: string; redirect: string } { + const u = new URL(url) + const redirect = u.searchParams.get("redirect")! + const state = u.searchParams.get("state")! + const port = Number(new URL(redirect).port) + return { port, state, redirect } +} + +async function fireCallback(redirect: string, params: Record): Promise { + const target = new URL(redirect) + for (const [k, v] of Object.entries(params)) target.searchParams.set(k, v) + const res = await fetch(target.toString(), { method: "GET" }) + // Drain body so the connection can close and let the CLI's `close()` + // proceed without hanging on lingering sockets. + await res.text().catch(() => "") +} + +// ───────────────────────────────────────────────────────────────────────────── +// resolveWorkspaceWebUrl — the deployment-support gate +// ───────────────────────────────────────────────────────────────────────────── + +describe("resolveWorkspaceWebUrl", () => { + test("freemium API host resolves to .ws.myaltimate.com", () => { + const url = resolveWorkspaceWebUrl("https://api.myaltimate.com", "acme") + expect(url).not.toBeNull() + expect(url!.toString()).toBe("https://acme.ws.myaltimate.com/") + }) + + test("localhost API returns null (browser flow not supported in dev)", () => { + expect(resolveWorkspaceWebUrl("http://localhost:5001", "acme")).toBeNull() + }) + + test("enterprise API host returns null", () => { + expect(resolveWorkspaceWebUrl("https://acme.getaltimate.com", "acme")).toBeNull() + }) + + test("malformed URL returns null instead of throwing", () => { + expect(resolveWorkspaceWebUrl("not-a-url", "acme")).toBeNull() + expect(resolveWorkspaceWebUrl("", "acme")).toBeNull() + }) +}) + +// ───────────────────────────────────────────────────────────────────────────── +// openWorkspaceBrowserHandoff — pre-flight failures (do not open a browser) +// ───────────────────────────────────────────────────────────────────────────── + +describe("openWorkspaceBrowserHandoff pre-flight", () => { + afterEach(() => unstubCreds()) + + test("returns {unavailable} for localhost credentials", async () => { + stubCreds("acme", "http://localhost:5001") + const result = await openWorkspaceBrowserHandoff({ + identifier: { repoRemote: "git@github.com:acme/x.git", projectPath: "/x" }, + projectName: "x", + }) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.reason).toBe("unavailable") + }) + + test("returns {not_configured} when credentials are missing", async () => { + ;(AltimateApi as unknown as { isConfigured: () => Promise }).isConfigured = + async () => false + const result = await openWorkspaceBrowserHandoff({ + identifier: { projectPath: "/x" }, + projectName: "x", + }) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.reason).toBe("not_configured") + }) +}) + +// ───────────────────────────────────────────────────────────────────────────── +// End-to-end via runHandoffWithOpener — real loopback, injected browser-open +// ───────────────────────────────────────────────────────────────────────────── + +describe("runHandoffWithOpener end-to-end", () => { + beforeEach(() => stubCreds("acme", "https://api.myaltimate.com")) + afterEach(() => unstubCreds()) + + test("happy path: valid callback resolves with workspaceId + tenant", async () => { + const result = await runHandoffWithOpener( + { + identifier: { repoRemote: "git@github.com:acme/x.git", projectPath: "/x" }, + projectName: "x", + }, + async (url) => { + const { state, redirect } = parseHandoffUrl(url) + await fireCallback(redirect, { workspace_id: "42", state, tenant: "acme" }) + }, + ) + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.workspaceId).toBe(42) + expect(result.tenant).toBe("acme") + } + }) + + test("tenant mismatch is refused", async () => { + const result = await runHandoffWithOpener( + { identifier: { projectPath: "/x" }, projectName: "x" }, + async (url) => { + const { state, redirect } = parseHandoffUrl(url) + await fireCallback(redirect, { workspace_id: "42", state, tenant: "not-acme" }) + }, + ) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.reason).toBe("tenant_mismatch") + }) + + test("?error=cancelled callback resolves as {cancelled}", async () => { + const result = await runHandoffWithOpener( + { identifier: { projectPath: "/x" }, projectName: "x" }, + async (url) => { + const { state, redirect } = parseHandoffUrl(url) + await fireCallback(redirect, { state, error: "cancelled", tenant: "acme" }) + }, + ) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.reason).toBe("cancelled") + }) + + test("missing workspace_id in callback resolves as {error}", async () => { + const result = await runHandoffWithOpener( + { identifier: { projectPath: "/x" }, projectName: "x" }, + async (url) => { + const { state, redirect } = parseHandoffUrl(url) + await fireCallback(redirect, { state, tenant: "acme" }) + }, + ) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.reason).toBe("error") + }) + + test("invalid workspace_id (non-numeric) resolves as {error}", async () => { + const result = await runHandoffWithOpener( + { identifier: { projectPath: "/x" }, projectName: "x" }, + async (url) => { + const { state, redirect } = parseHandoffUrl(url) + await fireCallback(redirect, { workspace_id: "not-a-number", state, tenant: "acme" }) + }, + ) + expect(result.ok).toBe(false) + if (!result.ok) expect(result.reason).toBe("error") + }) + + test("browser open failure resolves as {browser_open_failed} with authorizeUrl", async () => { + const result = await runHandoffWithOpener( + { identifier: { projectPath: "/x" }, projectName: "x" }, + async () => { + throw new Error("mock: no browser available") + }, + ) + expect(result.ok).toBe(false) + if (!result.ok) { + expect(result.reason).toBe("browser_open_failed") + expect(result.authorizeUrl).toContain("/create-and-link") + expect(result.authorizeUrl).toContain("client=altimate-code") + expect(result.authorizeUrl).toContain("project_name=x") + } + }) + + test("URL includes project_remote + project_path + project_name from input", async () => { + let observed = "" + await runHandoffWithOpener( + { + identifier: { repoRemote: "git@github.com:acme/foo.git", projectPath: "/w/foo" }, + projectName: "foo", + }, + async (url) => { + observed = url + // fire callback so the flow doesn't hang for 15 min + const { state, redirect } = parseHandoffUrl(url) + await fireCallback(redirect, { workspace_id: "1", state, tenant: "acme" }) + }, + ) + const u = new URL(observed) + expect(u.searchParams.get("project_remote")).toBe("git@github.com:acme/foo.git") + expect(u.searchParams.get("project_path")).toBe("/w/foo") + expect(u.searchParams.get("project_name")).toBe("foo") + expect(u.pathname).toBe("/create-and-link") + }) +}) + +// ───────────────────────────────────────────────────────────────────────────── +// Port walk: a squatting listener on 7317 forces handoff to 7318+ +// ───────────────────────────────────────────────────────────────────────────── + +describe("port walk", () => { + beforeEach(() => stubCreds("acme", "https://api.myaltimate.com")) + afterEach(() => unstubCreds()) + + test("stale listener on 7317 forces handoff to 7318", async () => { + const squatter = createServer() + await new Promise((resolve, reject) => { + squatter.once("error", reject) + squatter.listen(7317, "127.0.0.1", () => resolve()) + }) + + try { + let observedPort = -1 + const result = await runHandoffWithOpener( + { identifier: { projectPath: "/x" }, projectName: "x" }, + async (url) => { + const { port, state, redirect } = parseHandoffUrl(url) + observedPort = port + await fireCallback(redirect, { workspace_id: "1", state, tenant: "acme" }) + }, + ) + expect(result.ok).toBe(true) + expect(observedPort).toBeGreaterThan(7317) + expect(observedPort).toBeLessThanOrEqual(7325) + } finally { + await new Promise((r) => squatter.close(() => r())) + } + }) +}) From 80a8b2e03fdc10a3a1a0950bf9853d3739f33b88 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 00:16:47 +0530 Subject: [PATCH 11/14] feat(workspace): top-level nav handoff + confirmation dialog + sidebar tile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Deliver workspace handoff to CLI loopback via top-level navigation (matches OAuth sign-in pattern), bypassing HTTPS→loopback Private Network Access restrictions that would gate a subresource fetch in prod. Cancel uses the same mechanism; loopback bounces the browser back to the SaaS workspace page on success and workspace home on cancel. - Replace transient success toasts with a persistent post-bind `WorkspaceLinkedDialog` (workspace name + manage URL + "Continue editing in browser" / "Done"). Wired into all five bind success paths (browser handoff, inline create, picker attach, picker rebind, on-demand palette). - New right-pane sidebar tile showing the currently-linked workspace + manage URL, polling the local cache every 3s so a fresh bind surfaces without a TUI reload. Falls back to "Not linked — run /link" for unbound projects. - Canonicalize local binding cache keys via `realpathSync` on both write and read paths, with a scan fallback for pre-existing entries. Fixes the macOS `/tmp` → `/private/tmp` symlink mismatch that caused the sidebar and by-path lookups to miss bindings the CLI itself had written. - `altimate-code link` subcommand: show manage URL on success, cancel via top-level nav for reliability. Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../src/altimate/workspace/browser-handoff.ts | 49 ++++- .../opencode/src/altimate/workspace/state.ts | 31 +++- packages/opencode/src/cli/cmd/link.ts | 2 + .../opencode/src/plugin/tui/altimate/index.ts | 12 +- .../plugin/tui/altimate/workspace-sidebar.tsx | 93 ++++++++++ .../src/plugin/tui/altimate/workspace.tsx | 171 +++++++++++++----- 6 files changed, 298 insertions(+), 60 deletions(-) create mode 100644 packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx diff --git a/packages/opencode/src/altimate/workspace/browser-handoff.ts b/packages/opencode/src/altimate/workspace/browser-handoff.ts index 78a0bc7abf..7f04505d88 100644 --- a/packages/opencode/src/altimate/workspace/browser-handoff.ts +++ b/packages/opencode/src/altimate/workspace/browser-handoff.ts @@ -40,10 +40,23 @@ const CALLBACK_PORT_MIN = 7317 const CALLBACK_PORT_MAX = 7325 const DEFAULT_TIMEOUT_MS = 15 * 60 * 1000 -const DELIVERY_HTML_SUCCESS = `Altimate Code + +/** Loopback success page. We land the user back on the SaaS workspace page via + * top-level navigation — matches the OAuth sign-in pattern in altimate.ts, + * which is proven in prod. The rationale over a subresource fetch: HTTPS→HTTP + * loopback subresource fetches trigger Chrome/Safari Private Network Access + * checks (preflight OPTIONS with Access-Control-Request-Private-Network); a + * top-level navigation from an HTTP 302 or ``window.location.href`` bypasses + * PNA entirely. Meta refresh + JS assign for belt-and-suspenders. */ +function deliverySuccessHtml(manageUrl: string): string { + const safe = escapeHtml(manageUrl) + return `Altimate Code + -

Workspace ready

Return to your terminal to finish linking.

-` +

Workspace ready

Returning you to the workspace page…

+

Continue if you're not redirected automatically.

+` +} const log = Log.create({ service: "altimate-workspace-handoff" }) @@ -61,6 +74,20 @@ function htmlError(msg: string): string {

Please return to your terminal and try again.

` } +/** Cancel-path response: bounce the browser back to the SaaS workspace home + * so the user doesn't get stranded on the plain loopback page. Same top-level + * navigation mechanism as ``deliverySuccessHtml``. */ +function cancelHtml(workspaceWebBase: URL): string { + const home = workspaceWebBase.toString().replace(/\/$/, "") + "/" + const safe = escapeHtml(home) + return `Altimate Code + + +

Cancelled

Returning you to the workspace home…

+

Continue if you're not redirected automatically.

+` +} + export type HandoffFailureReason = | "unavailable" // resolveWorkspaceWebUrl returned null (not freemium) | "not_configured" // CLI credentials not present @@ -111,6 +138,9 @@ export function resolveWorkspaceWebUrl(altimateUrl: string, tenant: string): URL interface HandoffPending { state: string expectedTenant: string + /** Base URL for the tenant's SaaS workspace stack, used to build the + * ``/w/:id`` bounce target that the loopback success HTML redirects to. */ + workspaceWebBase: URL resolve: (v: HandoffSuccess) => void reject: (err: Error & { handoffReason?: HandoffFailureReason }) => void } @@ -151,7 +181,11 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; const error = url.searchParams.get("error") if (error) { const reason: HandoffFailureReason = error === "cancelled" ? "cancelled" : "error" - respond(200, htmlError(error === "cancelled" ? "Cancelled by user" : error)) + // Cancel bounces the browser back to the SaaS workspace home so the user + // isn't stranded on the plain loopback page; hard errors keep the plain + // error card (there's no useful place to bounce them to). + const body = error === "cancelled" ? cancelHtml(pending.workspaceWebBase) : htmlError(error) + respond(200, body) pending.reject(markReason(new Error(error), reason)) return } @@ -184,7 +218,11 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; return } - respond(200, DELIVERY_HTML_SUCCESS) + // Bounce the browser back to the SaaS workspace page. Loopback constructs + // the URL itself (no need to trust a `return` query param) — the base is + // deterministic from the tenant we already validated above. + const manageUrl = `${pending.workspaceWebBase.toString().replace(/\/$/, "")}/w/${workspaceId}` + respond(200, deliverySuccessHtml(manageUrl)) pending.resolve({ ok: true, workspaceId, tenant }) }) @@ -272,6 +310,7 @@ export async function runHandoffWithOpener( const pending: HandoffPending = { state, expectedTenant: creds.altimateInstanceName, + workspaceWebBase: webUrl, resolve: (v) => { closeListener() clearTimeout(timeoutHandle) diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index a14faf4c81..5366d51696 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -10,7 +10,7 @@ // ``Global.Path.state`` at 0o600 — chmod is applied post-write since // ``Filesystem.writeJsonAtomic`` does not chmod (see filesystem.ts:294 for // why; codex round-2 flagged this gap). -import { chmodSync, existsSync, readFileSync } from "node:fs" +import { chmodSync, existsSync, readFileSync, realpathSync } from "node:fs" import path from "node:path" import { AltimateApi } from "@/altimate/api/client" import { Global } from "@/global" @@ -106,6 +106,18 @@ function writeCache(cache: CacheFile): void { } } +/** Canonicalize a directory path so cache lookups survive symlink differences + * (macOS ``/tmp`` → ``/private/tmp`` is the common case). Writers and readers + * must both funnel through this or a shell-cwd write silently misses when the + * TUI's canonicalized ``state.path.directory`` looks it back up. */ +function canonicalizeKey(directory: string): string { + try { + return realpathSync(path.resolve(directory)) + } catch { + return path.resolve(directory) + } +} + async function tenantKey(): Promise<{ tenant: string; apiUrl: string } | null> { // Best-effort: ``AltimateApi.getCredentials`` can throw ``SyntaxError`` on // a corrupt credentials JSON, ``ZodError`` on schema drift, or a raw @@ -134,7 +146,17 @@ export async function readLocalBinding(directory: string): Promise): BuiltinTuiPlugin[] { const base = [ProviderCredentials, PromptEnhance, SkillOps, TraceViewer] - // Workspace TUI plugin is pilot-gated: only registered for users who - // opted into ALTIMATE_WORKSPACE. Otherwise the post-scan dialog + the - // altimate.workspace.link palette command would ship to 100% of users - // regardless of the flag setting. (M1 in the consensus review.) - return Flag.ALTIMATE_WORKSPACE ? [...base, Workspace] : base + // Workspace TUI plugin + right-pane sidebar tile are pilot-gated: only + // registered for users who opted into ALTIMATE_WORKSPACE. Otherwise the + // post-scan dialog, the altimate.workspace.link palette command, and the + // sidebar's 30s poll would ship to 100% of users regardless of the flag + // setting. (M1 in the consensus review.) + return Flag.ALTIMATE_WORKSPACE ? [...base, Workspace, WorkspaceSidebar] : base } // altimate_change end diff --git a/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx b/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx new file mode 100644 index 0000000000..712b8d54d2 --- /dev/null +++ b/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx @@ -0,0 +1,93 @@ +// altimate_change - new file +// Right-pane sidebar tile that shows the workspace the current project +// directory is bound to (or "Not linked" with a hint). Reads from the local +// binding cache written by `../workspace.tsx` (post-scan dialog, on-demand +// picker, browser handoff). Cache is polled every 3s so a fresh bind +// surfaces without the user having to reload the TUI. +// +// Deliberately read-only. All bind mutations live in workspace.tsx / link.ts; +// this tile just reflects state. +import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" +import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" +import { createSignal, onCleanup, onMount, Show } from "solid-js" +import { readLocalBinding, type CachedBinding } from "@/altimate/workspace/state" +import { resolveWorkspaceWebUrl } from "@/altimate/workspace/browser-handoff" +import { AltimateApi } from "@/altimate/api/client" + +const id = "altimate:sidebar-workspace" + +const POLL_MS = 3000 + +function View(props: { api: TuiPluginApi }) { + const theme = () => props.api.theme.current + const [binding, setBinding] = createSignal(null) + const [manageUrl, setManageUrl] = createSignal(null) + + const refresh = async () => { + const dir = props.api.state.path.directory + const b = await readLocalBinding(dir).catch(() => null) + setBinding(b) + if (!b) { + setManageUrl(null) + return + } + try { + const creds = await AltimateApi.getCredentials() + const base = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) + setManageUrl(base ? `${base.toString().replace(/\/$/, "")}/w/${b.datamateId}` : null) + } catch { + setManageUrl(null) + } + } + + onMount(() => { + void refresh() + const timer = setInterval(() => void refresh(), POLL_MS) + onCleanup(() => clearInterval(timer)) + }) + + return ( + + + Workspace + + + Not linked — run /link + + } + > + {(b) => ( + <> + {b().datamateName} + + {(u) => {u()}} + + + )} + + + ) +} + +const tui: TuiPlugin = async (api) => { + api.slots.register({ + // Below MCP (200) and above LSP (300) — workspace identity is high-signal + // when present, but not more useful than the connection status above. + order: 250, + slots: { + sidebar_content() { + return + }, + }, + }) +} + +const plugin: BuiltinTuiPlugin = { + id, + tui, +} + +export default plugin diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index c9e08bdcab..2211a5ccd0 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -222,6 +222,100 @@ function OfferDialog(props: OfferProps) { ) } +/** Build the SaaS manage-workspace URL for a bound workspace. Deterministic + * from tenant + id, so any caller can construct it without an extra round-trip. + * Returns null when the current deployment isn't the freemium web (BYOK or + * unresolvable) — the confirmation dialog degrades to id-only in that case. */ +async function buildManageUrl(workspaceId: number): Promise { + try { + const creds = await AltimateApi.getCredentials() + const base = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) + if (!base) return null + return `${base.toString().replace(/\/$/, "")}/w/${workspaceId}` + } catch { + return null + } +} + +interface LinkedProps { + api: TuiPluginApi + workspaceName: string + manageUrl: string | null + /** Verb for the title — "Linked" / "Re-linked" / "Created". Keeps the + * three success paths visually consistent while still labelling what + * just happened. */ + verb: "Linked" | "Re-linked" | "Created" +} + +/** Persistent confirmation card shown after a successful bind. Replaces the + * transient success toast so the user has an unmissable "yes, it worked" and + * a stable CTA back to the browser. Dismissable via Done or Esc. */ +function WorkspaceLinkedDialog(props: LinkedProps) { + const title = () => { + const suffix = props.manageUrl ? ` — ${props.manageUrl}` : "" + return `${props.verb} workspace "${props.workspaceName}"${suffix}` + } + const options = () => { + if (props.manageUrl) { + return [ + { + title: "Continue editing in browser", + value: "open", + description: "Open the workspace in your browser.", + }, + { title: "Done", value: "done", description: "Close this dialog." }, + ] + } + return [{ title: "Done", value: "done", description: "Close this dialog." }] + } + return ( + { + if (option.value === "open" && props.manageUrl) { + const url = props.manageUrl + // Guard before delegating to open() — a rogue manage_url with a + // non-http protocol would otherwise dispatch to an unrelated OS + // scheme handler. buildManageUrl only ever emits http(s) URLs from + // resolveWorkspaceWebUrl, but the guard survives future changes. + if (!isSafeHttpUrl(url)) { + props.api.ui.toast({ + variant: "warning", + message: `Refused to open a non-http URL: ${url}`, + duration: 15_000, + }) + } else { + open(url).catch(() => { + props.api.ui.toast({ + variant: "warning", + message: `Could not open browser. Copy this URL: ${url}`, + duration: 15_000, + }) + }) + } + } + props.api.ui.dialog.clear() + }} + /> + ) +} + +/** Show the persistent linked-confirmation dialog. Builds the manage URL + * best-effort; degrades gracefully on BYOK/unresolvable. */ +async function showLinkedConfirmation( + api: TuiPluginApi, + verb: LinkedProps["verb"], + workspaceId: number, + workspaceName: string, +): Promise { + const manageUrl = await buildManageUrl(workspaceId) + api.ui.dialog.replace(() => ( + + )) +} + /** Post-scan / on-demand browser-handoff runner. Opens the SaaS approval * modal, waits for the callback, and binds the current project to the * returned workspace via the existing ``POST /bind`` endpoint. Every failure @@ -235,7 +329,7 @@ async function runBrowserHandoff( api.ui.dialog.clear() api.ui.toast({ variant: "info", - message: "Opening browser to set up your workspace — return here once you approve.", + message: "Opening browser to set up your workspace — approve there, then check back here for the confirmation.", }) const result: HandoffResult = await openWorkspaceBrowserHandoff({ identifier, projectName }) if (!result.ok) { @@ -253,10 +347,7 @@ async function runBrowserHandoff( projectPath: res.binding.project_path, linkedAt: Date.now(), }) - api.ui.toast({ - variant: "success", - message: `Linked to workspace "${res.binding.datamate_name}".`, - }) + await showLinkedConfirmation(api, "Linked", res.binding.datamate_id, res.binding.datamate_name) } catch (err) { if (err instanceof ConflictError) { api.ui.toast({ @@ -391,10 +482,11 @@ async function createAndBindInline( // 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 - // fallout inside the function itself: ``recordApprovedBinding`` already - // swallows its own errors (state.ts is best-effort), but ``open()`` and - // the toast APIs can reject unexpectedly. Fall back to a plain info - // toast so the user still sees the URL. (Kilo cycle 5.) + // fallout inside the function itself. ``recordApprovedBinding`` already + // swallows its own errors (state.ts is best-effort), but + // ``showLinkedConfirmation`` can reject on dialog-teardown races — the + // toast fallback keeps the user informed without taking the process down. + // (Kilo cycle 5.) try { await recordApprovedBinding(api.state.path.directory, { datamateId: res.datamate.id, @@ -403,27 +495,7 @@ async function createAndBindInline( projectPath: res.binding.project_path, linkedAt: Date.now(), }) - // Guard against a non-http(s) manage_url — ``open`` dispatches to whatever - // OS handler matches the protocol, so a rogue value could launch an - // unrelated app. Fall through to the info toast (with the URL for manual - // copy) if the URL isn't a safe http/https link. - if (isSafeHttpUrl(res.manage_url)) { - try { - await open(res.manage_url) - api.ui.toast({ - variant: "success", - message: `Workspace "${res.datamate.name}" created. Opened ${res.manage_url} in your browser.`, - }) - return - } catch { - /* fall through to the "open manually" toast below */ - } - } - api.ui.toast({ - variant: "info", - message: `Workspace "${res.datamate.name}" created. Open ${res.manage_url} to configure it.`, - duration: 10_000, - }) + await showLinkedConfirmation(api, "Created", res.datamate.id, res.datamate.name) } catch (err) { api.ui.toast({ variant: "info", @@ -435,7 +507,9 @@ async function createAndBindInline( /** True when the URL parses and its protocol is exactly ``http:`` or ``https:``. * Used before handing a server-supplied URL to ``open()`` (which would otherwise - * dispatch to whatever OS scheme handler matches the protocol). */ + * dispatch to whatever OS scheme handler matches the protocol). Kept exported + * as a top-level helper because both ``showLinkedConfirmation`` (below) and + * the on-demand link paths need the same guard. */ function isSafeHttpUrl(url: string): boolean { try { const u = new URL(url) @@ -581,10 +655,13 @@ function PickerDialog(props: PickerProps) { projectPath: res.binding.project_path, linkedAt: Date.now(), }) - props.api.ui.toast({ - variant: "success", - message: `Linked to workspace "${res.binding.datamate_name}".`, - }) + await showLinkedConfirmation( + props.api, + "Linked", + res.binding.datamate_id, + res.binding.datamate_name, + ) + return } else { // Rebind: pick the endpoint that matches which identifier the // pre-check RESOLVED the binding on — not what the current identifier @@ -607,12 +684,14 @@ function PickerDialog(props: PickerProps) { projectPath: res.binding.project_path, linkedAt: Date.now(), }) - props.api.ui.toast({ - variant: "success", - message: `Re-linked to workspace "${res.binding.datamate_name}".`, - }) + await showLinkedConfirmation( + props.api, + "Re-linked", + res.binding.datamate_id, + res.binding.datamate_name, + ) + return } - props.api.ui.dialog.clear() } catch (err) { // Surface as a toast so the user sees the specific failure. Dialog closes // either way — the user can re-invoke via /altimate.workspace.link. @@ -802,12 +881,12 @@ async function bindOrRebindInline( projectPath: res.binding.project_path, linkedAt: Date.now(), }) - api.ui.toast({ - variant: "success", - message: isRebind - ? `Re-linked to workspace "${res.binding.datamate_name}".` - : `Linked to workspace "${res.binding.datamate_name}".`, - }) + await showLinkedConfirmation( + api, + isRebind ? "Re-linked" : "Linked", + res.binding.datamate_id, + res.binding.datamate_name, + ) } catch (err) { let msg: string if (err instanceof ConflictError) { From 432753050a93dd6ec46c4c0ac29e177506bd9bb8 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 04:01:50 +0530 Subject: [PATCH 12/14] =?UTF-8?q?fix(workspace):=20consensus=20review=20?= =?UTF-8?q?=E2=80=94=20handoff=20error=20contract,=20credential=20re-verif?= =?UTF-8?q?y,=20sidebar=20polish,=20cache=20canonicalization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review findings introduced by this PR's commits (browser handoff + top-level nav / sidebar tile). PR #1099 fixes landed separately. - `runHandoffWithOpener` now wraps preflight (`getCredentials`) AND the post-listener async IIFE in one try/catch that converts every error to a `HandoffResult`. Previously a malformed credentials file rejected the returned Promise with no toast, and a throw inside the lazy `import("../plugin/altimate")` left the caller waiting the full 15 minutes with no reason surfaced. The port is captured into a local immediately after `startListener` resolves so a timeout-cleared handle can't be dereferenced later. (M4) - `HandoffSuccess` now carries a `credentials` fingerprint (apiUrl + tenant) that the handoff was validated against. `runBrowserHandoff` in both entry points re-reads `AltimateApi.getCredentials()` immediately before `bindExisting` and refuses if either field drifted — workspace ids are tenant-schema-local so a mid-flow account switch would otherwise bind under the wrong tenant. (M6) - `resolveWorkspaceWebUrl` guards the tenant with a DNS-label regex and reconstructs the origin from the parsed URL, so a credential row carrying `evil.example/path?x=` cannot open the handoff at `https://evil.example`. Override still available for local dev; both paths reject non-http(s) protocols. (m3) - Optional `AbortSignal` on `OpenBrowserHandoffInput` — a caller-fired abort tears down the listener immediately with `reason: "aborted"` instead of holding the port for 15 minutes; timeout is `.unref()`'d so it doesn't keep the CLI process alive on its own. (m2) - `port_exhausted` is now only returned when the errno is `EADDRINUSE` — other codes (EACCES, EBADF) map to `reason: "error"` so the user isn't told "ports all in use" for a permissions problem. (m5) - `project_path` + `project_remote` moved to the URL fragment, matching the `cli_context` rationale — those two values carry usernames / customer names / internal paths that shouldn't land in SaaS access logs, WAF logs, or browser history. `project_name` stays in the query because the SaaS approval modal renders it. Test updated. (m6) - `workspace_id` uses `Number.isInteger` instead of `Number.isFinite`, so `42.5` no longer reaches a backend expecting an integer. (m9) - Inline `` +` } const log = Log.create({ service: "altimate-workspace-handoff" }) @@ -67,6 +76,15 @@ function escapeHtml(s: string): string { ) } +/** JSON-encode + escape any ```` cannot + * close the surrounding inline ` +` } export type HandoffFailureReason = @@ -94,14 +112,29 @@ export type HandoffFailureReason = | "timeout" // 15-min window expired | "cancelled" // user hit Cancel in the browser | "tenant_mismatch" // callback tenant != credentials tenant - | "port_exhausted" // 7317..7325 all in use + | "port_exhausted" // 7317..7325 all EADDRINUSE | "browser_open_failed" + | "aborted" // caller-provided AbortSignal fired | "error" +/** Snapshot of the credentials the handoff started against, returned to the + * caller so it can re-verify against fresh creds immediately before binding + * (M6 in the consensus review). Workspace ids are tenant-schema-local, so + * binding a callback validated for tenant A under tenant B (after an account + * switch mid-flow) would 404 or, worse, hit an unrelated workspace. */ +export interface CredentialFingerprint { + apiUrl: string + tenant: string +} + export interface HandoffSuccess { ok: true workspaceId: number tenant: string + /** Credentials the handoff resolved and validated the callback against. + * Callers must compare against ``AltimateApi.getCredentials()`` at bind + * time and refuse the bind if either field drifted. */ + credentials: CredentialFingerprint } export interface HandoffFailure { ok: false @@ -114,14 +147,19 @@ export type HandoffResult = HandoffSuccess | HandoffFailure /** Compute the workspace-stack URL for a given API host + tenant, or null if * this deployment isn't supported (localhost, enterprise, custom domain). * - * Dev escape hatch: ``ALTIMATE_WORKSPACE_WEB_URL`` overrides the map lookup - * when set (must be a well-formed URL). Used for local integration testing - * against a non-freemium SaaS instance. Not something production users touch. */ + * Dev escape hatch: ``ALTIMATE_WORKSPACE_WEB_URL`` overrides the tenant map + * lookup when set. The override is DEV-ONLY — it returns the URL as-is + * without tenant scoping (which is what a local ``altimate2.localhost:3003`` + * dev server needs). Production callers must not set it; if it is somehow + * present and points off-tenant, the CSRF ``state`` still gates the callback + * so no cross-workspace bind is possible. */ export function resolveWorkspaceWebUrl(altimateUrl: string, tenant: string): URL | null { const override = process.env["ALTIMATE_WORKSPACE_WEB_URL"] if (override) { try { - return new URL(override) + const u = new URL(override) + if (u.protocol !== "http:" && u.protocol !== "https:") return null + return u } catch { return null } @@ -129,7 +167,15 @@ export function resolveWorkspaceWebUrl(altimateUrl: string, tenant: string): URL try { const apiHost = new URL(altimateUrl).host if (apiHost !== FREEMIUM_API_HOST) return null - return new URL(`https://${tenant}.${FREEMIUM_WORKSPACE_HOST}`) + // DNS-label guard — see TENANT_LABEL_RE for rationale. Double-check by + // reconstructing the origin from the parsed URL: if the parser resolved + // to a different host (embedded slashes, port, path in the "tenant"), + // refuse rather than emit a URL that points off-domain. + if (!TENANT_LABEL_RE.test(tenant)) return null + const lower = tenant.toLowerCase() + const u = new URL(`https://${lower}.${FREEMIUM_WORKSPACE_HOST}`) + if (u.hostname !== `${lower}.${FREEMIUM_WORKSPACE_HOST}`) return null + return u } catch { return null } @@ -211,7 +257,9 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; } const workspaceId = Number(workspaceIdRaw) - if (!Number.isFinite(workspaceId) || workspaceId <= 0) { + // Integer-only: floats like ``42.5`` are rejected server-side but produce + // a confusing failure the caller can't recover from. (m9 in the review.) + if (!Number.isInteger(workspaceId) || workspaceId <= 0) { const msg = `Invalid workspace_id: ${workspaceIdRaw}` respond(400, htmlError(msg)) pending.reject(markReason(new Error(msg), "error")) @@ -223,7 +271,15 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; // deterministic from the tenant we already validated above. const manageUrl = `${pending.workspaceWebBase.toString().replace(/\/$/, "")}/w/${workspaceId}` respond(200, deliverySuccessHtml(manageUrl)) - pending.resolve({ ok: true, workspaceId, tenant }) + // Callback validated — but the SUCCESS payload carries the credentials + // snapshot the handoff was started against; the caller re-verifies + // against fresh creds before binding (M6). This module never binds. + pending.resolve({ + ok: true, + workspaceId, + tenant, + credentials: { apiUrl: "", tenant: pending.expectedTenant }, // apiUrl filled in by caller + }) }) // Walk 7317..7325 — each server instance is independent, so a squatting @@ -246,6 +302,9 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; lastErr = err as NodeJS.ErrnoException // Defensive cleanup in case any listeners linger after a rejected bind. server.removeAllListeners("error") + // Only keep walking on EADDRINUSE — any other errno (EACCES, EBADF, …) + // is a real problem, not port squatting, so break out and report it + // faithfully rather than falsely claiming "all ports in use". (m5) if (lastErr.code !== "EADDRINUSE") break } } @@ -258,13 +317,18 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; ? `Every port in ${CALLBACK_PORT_MIN}-${CALLBACK_PORT_MAX} is in use (tried ${tried.join(", ")}). Close what's using them (e.g. \`lsof -i :${CALLBACK_PORT_MIN}\`) and try again.` : `Could not start the workspace-handoff server: ${lastErr instanceof Error ? lastErr.message : String(lastErr)}`, ), - "port_exhausted", + code === "EADDRINUSE" ? "port_exhausted" : "error", ) } export interface OpenBrowserHandoffInput { identifier: ProjectIdentifier projectName: string + /** Optional AbortSignal — if it fires the flow settles with + * ``{ok: false, reason: "aborted"}`` and tears down the listener. Lets a + * TUI supersede a stale handoff without leaking a port for the full + * 15-minute window. (m2) */ + signal?: AbortSignal } /** Full browser-handoff flow. Returns the created/picked workspace ID on @@ -283,12 +347,28 @@ export async function runHandoffWithOpener( input: OpenBrowserHandoffInput, openBrowser: (url: string) => Promise, ): Promise { - if (!(await AltimateApi.isConfigured().catch(() => false))) { - return { ok: false, reason: "not_configured" } + // Preflight is inside the same try/catch that owns the startup IIFE — a + // rejection from ``getCredentials()`` (malformed JSON, unresolved ${env:…} + // placeholder, schema mismatch) or from any other setup step converts to + // a HandoffResult instead of propagating as an unhandled rejection into + // the TUI's ``void runBrowserHandoff(...)`` call sites. (M4) + let creds: Awaited> + let webUrl: URL + try { + if (!(await AltimateApi.isConfigured().catch(() => false))) { + return { ok: false, reason: "not_configured" } + } + creds = await AltimateApi.getCredentials() + const resolved = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) + if (!resolved) return { ok: false, reason: "unavailable" } + webUrl = resolved + } catch (err) { + return { + ok: false, + reason: "error", + message: err instanceof Error ? err.message : String(err), + } } - const creds = await AltimateApi.getCredentials() - const webUrl = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) - if (!webUrl) return { ok: false, reason: "unavailable" } const state = randomBytes(16).toString("hex") @@ -306,7 +386,8 @@ export async function runHandoffWithOpener( } } - const settled = new Promise((resolve) => { + return new Promise((resolve) => { + let onAbort: (() => void) | null = null const pending: HandoffPending = { state, expectedTenant: creds.altimateInstanceName, @@ -314,11 +395,16 @@ export async function runHandoffWithOpener( resolve: (v) => { closeListener() clearTimeout(timeoutHandle) - resolve(v) + if (onAbort && input.signal) input.signal.removeEventListener("abort", onAbort) + // Fill in the apiUrl snapshot the listener couldn't set (it doesn't + // hold ``creds``); the tenant already went through the expectedTenant + // check inside the listener. + resolve({ ...v, credentials: { apiUrl: creds.altimateUrl, tenant: v.tenant } }) }, reject: (err) => { closeListener() clearTimeout(timeoutHandle) + if (onAbort && input.signal) input.signal.removeEventListener("abort", onAbort) const reason = (err as { handoffReason?: HandoffFailureReason }).handoffReason ?? "error" const authorizeUrl = (err as { authorizeUrl?: string }).authorizeUrl resolve({ @@ -332,49 +418,75 @@ export async function runHandoffWithOpener( const timeoutHandle = setTimeout(() => { pending.reject(markReason(new Error("Timed out waiting for browser workspace handoff"), "timeout")) }, DEFAULT_TIMEOUT_MS) + // ``.unref()`` so the timer alone doesn't keep the CLI process alive + // once every other handle has exited. (m2) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(timeoutHandle as any)?.unref?.() + + // Wire the AbortSignal — if it fires the flow settles with + // ``reason: "aborted"`` and the listener is torn down immediately. + if (input.signal) { + if (input.signal.aborted) { + pending.reject(markReason(new Error("Handoff aborted"), "aborted")) + return + } + onAbort = () => pending.reject(markReason(new Error("Handoff aborted"), "aborted")) + input.signal.addEventListener("abort", onAbort, { once: true }) + } ;(async () => { try { listenerHandle = await startListener(pending) + // Capture the port to a local IMMEDIATELY — ``listenerHandle`` is + // cleared by ``closeListener`` on timeout, and a lazy ``import()`` + // below can straddle that clear. (M4 sub-case) + const boundPort = listenerHandle.port + + // Import buildCliContext lazily so this module doesn't pull altimate.ts + // into every consumer's import graph at load time. + const { buildCliContext } = await import("../plugin/altimate") + const cliContext = await buildCliContext().catch((err) => { + log.warn("buildCliContext failed; proceeding without", { err: String(err) }) + return "" + }) + + const redirect = `http://127.0.0.1:${boundPort}/workspace-bound` + const target = new URL("/create-and-link", webUrl) + target.searchParams.set("client", "altimate-code") + target.searchParams.set("redirect", redirect) + target.searchParams.set("state", state) + target.searchParams.set("project_name", input.projectName) + // Project path + remote go in the URL FRAGMENT, not the query, so + // they don't land in SaaS access logs, WAF logs, or browser history + // as query params. Same rationale as ``cli_context`` in altimate.ts + // (see altimate.ts:135-137). (m6) + const fragment = new URLSearchParams() + if (input.identifier.repoRemote) fragment.set("project_remote", input.identifier.repoRemote) + if (input.identifier.projectPath) fragment.set("project_path", input.identifier.projectPath) + if (cliContext) fragment.set("cli_context", cliContext) + const authorizeUrl = fragment.toString() + ? `${target.toString()}#${fragment.toString()}` + : target.toString() + + try { + await openBrowser(authorizeUrl) + } catch (err) { + // Browser open failed. Preserve the URL so the caller can copy-paste. + pending.reject( + Object.assign( + markReason(new Error(`Could not open browser: ${err instanceof Error ? err.message : String(err)}`), "browser_open_failed"), + { authorizeUrl }, + ), + ) + } } catch (err) { + // ANY throw in this async IIFE — startListener rejection, the lazy + // ``import()``, ``buildCliContext()`` panic — funnels through + // pending.reject so ``settled`` resolves and the caller sees a + // ``HandoffResult`` instead of a 15-minute silent hang. (M4) const reason = (err as { handoffReason?: HandoffFailureReason }).handoffReason ?? "error" pending.reject(markReason(err as Error, reason)) - return - } - - // Import buildCliContext lazily so this module doesn't pull altimate.ts - // into every consumer's import graph at load time. - const { buildCliContext } = await import("../plugin/altimate") - const cliContext = await buildCliContext().catch((err) => { - log.warn("buildCliContext failed; proceeding without", { err: String(err) }) - return "" - }) - - const redirect = `http://127.0.0.1:${listenerHandle.port}/workspace-bound` - const target = new URL("/create-and-link", webUrl) - target.searchParams.set("client", "altimate-code") - target.searchParams.set("redirect", redirect) - target.searchParams.set("state", state) - if (input.identifier.repoRemote) target.searchParams.set("project_remote", input.identifier.repoRemote) - if (input.identifier.projectPath) target.searchParams.set("project_path", input.identifier.projectPath) - target.searchParams.set("project_name", input.projectName) - const authorizeUrl = cliContext - ? `${target.toString()}#cli_context=${encodeURIComponent(cliContext)}` - : target.toString() - - try { - await openBrowser(authorizeUrl) - } catch (err) { - // Browser open failed. Preserve the URL so the caller can copy-paste. - pending.reject( - Object.assign( - markReason(new Error(`Could not open browser: ${err instanceof Error ? err.message : String(err)}`), "browser_open_failed"), - { authorizeUrl }, - ), - ) } })() }) - - return settled } diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 5366d51696..075f861c79 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -91,6 +91,32 @@ function readCache(): CacheFile | null { } } +/** True when every key in the cache is already the canonical form of itself + * (i.e. no earlier-CLI-build unresolved keys remain). Cheap side condition + * so we can skip the per-read migration once the cache has been rewritten. */ +function isCanonicalized(cache: CacheFile): boolean { + for (const k of Object.keys(cache.bindings)) { + if (canonicalizeKey(k) !== k) return false + } + return true +} + +/** One-shot migration: rewrite the cache with canonical keys, collapsing any + * pair that resolves to the same target (last-writer-wins by ``linkedAt``). + * After this runs the O(n) lookup-time rescan in ``readLocalBinding`` is + * dead code — every subsequent read hits the direct key lookup. */ +function migrateToCanonicalKeys(cache: CacheFile): CacheFile { + const migrated: Record = {} + for (const [k, v] of Object.entries(cache.bindings)) { + const canon = canonicalizeKey(k) + const existing = migrated[canon] + if (!existing || existing.linkedAt <= v.linkedAt) migrated[canon] = v + } + const next: CacheFile = { ...cache, bindings: migrated } + writeCache(next) + return next +} + function writeCache(cache: CacheFile): void { const p = cachePath() Filesystem.writeJsonAtomic(p, cache) @@ -139,22 +165,25 @@ async function tenantKey(): Promise<{ tenant: string; apiUrl: string } | null> { } /** Read the local binding for ``directory`` — only returns a hit when the - * cache's stored (tenant, apiUrl) matches the current credentials. */ + * cache's stored (tenant, apiUrl) matches the current credentials. Runs a + * one-shot migration to canonical keys on the first read that finds an + * unresolved key (macOS ``/tmp`` → ``/private/tmp``), then relies on direct + * lookup for the process's remaining lifetime. */ export async function readLocalBinding(directory: string): Promise { const key = await tenantKey() if (!key) return null - const cache = readCache() + let cache = readCache() if (!cache) return null if (cache.tenant !== key.tenant || cache.apiUrl !== key.apiUrl) return null const canon = canonicalizeKey(directory) - // First try the direct canonical + raw key lookups (cheap). If neither hits, - // scan every stored key and re-canonicalize it — catches entries written by - // earlier CLI builds under an unresolved key (e.g. ``/tmp/foo``) even though - // the caller looks them back up under the resolved key (``/private/tmp/foo``). - const direct = cache.bindings[canon] ?? cache.bindings[directory] + const direct = cache.bindings[canon] if (direct) return direct - for (const [k, v] of Object.entries(cache.bindings)) { - if (canonicalizeKey(k) === canon) return v + // Cache miss: check if the cache still has any non-canonical keys and + // migrate the whole file once. After migration the lookup is a plain + // property access on every future read. + if (!isCanonicalized(cache)) { + cache = migrateToCanonicalKeys(cache) + return cache.bindings[canon] ?? null } return null } diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 21171b81ac..196ad8cb3d 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -192,6 +192,28 @@ async function runBrowserHandoff( process.exitCode = 1 return } + // M6 in the consensus review: re-verify credentials before binding. The + // browser window can stay open for up to 15 minutes; an account switch in + // that window would otherwise bind a callback validated for tenant A + // under tenant B (workspace ids are tenant-schema-local). + try { + const fresh = await AltimateApi.getCredentials() + if ( + fresh.altimateInstanceName !== result.credentials.tenant || + fresh.altimateUrl !== result.credentials.apiUrl + ) { + spin.stop( + `Credentials changed while the browser was open (was ${result.credentials.tenant}, now ${fresh.altimateInstanceName}). Re-run to link this project.`, + 1, + ) + process.exitCode = 1 + return + } + } catch { + spin.stop("Lost Altimate credentials while the browser was open — sign in and re-run.", 1) + process.exitCode = 1 + return + } spin.stop(`Workspace approved. Binding to project...`) const bindSpin = prompts.spinner() bindSpin.start("Linking workspace...") diff --git a/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx b/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx index 712b8d54d2..da6fdddb3b 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx @@ -2,8 +2,7 @@ // Right-pane sidebar tile that shows the workspace the current project // directory is bound to (or "Not linked" with a hint). Reads from the local // binding cache written by `../workspace.tsx` (post-scan dialog, on-demand -// picker, browser handoff). Cache is polled every 3s so a fresh bind -// surfaces without the user having to reload the TUI. +// picker, browser handoff). // // Deliberately read-only. All bind mutations live in workspace.tsx / link.ts; // this tile just reflects state. @@ -16,33 +15,69 @@ import { AltimateApi } from "@/altimate/api/client" const id = "altimate:sidebar-workspace" -const POLL_MS = 3000 +/** Cache-file poll cadence. Longer than a "reactive" ideal but the cheapest + * option that does not require plumbing an event bus through the binding + * writers. Trade-off documented (m1 in the consensus review): a fresh bind + * surfaces within one interval instead of instantly; a mostly-idle CLI reads + * the small cache file twice per minute. In-flight guard below prevents + * overlap when the file grows / the disk is slow. */ +const POLL_MS = 30_000 + +/** Cached credential lookup — the API is a network round-trip candidate in + * the general case, but the credentials source here (local file) rarely + * changes within a single CLI process. We memoize the resolved manage-URL + * base per (apiUrl, tenant) pair for the life of the process; if the file + * changes mid-session, the binding cache invalidation (in state.ts) still + * catches it via its own (tenant, apiUrl) top-level scoping. */ +let cachedManageBase: { apiUrl: string; tenant: string; base: string | null } | null = null +async function resolveManageBase(): Promise { + try { + const creds = await AltimateApi.getCredentials() + if ( + cachedManageBase && + cachedManageBase.apiUrl === creds.altimateUrl && + cachedManageBase.tenant === creds.altimateInstanceName + ) { + return cachedManageBase.base + } + const url = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) + const base = url ? url.toString().replace(/\/$/, "") : null + cachedManageBase = { apiUrl: creds.altimateUrl, tenant: creds.altimateInstanceName, base } + return base + } catch { + return null + } +} function View(props: { api: TuiPluginApi }) { const theme = () => props.api.theme.current const [binding, setBinding] = createSignal(null) const [manageUrl, setManageUrl] = createSignal(null) + let refreshInFlight = false const refresh = async () => { - const dir = props.api.state.path.directory - const b = await readLocalBinding(dir).catch(() => null) - setBinding(b) - if (!b) { - setManageUrl(null) - return - } + if (refreshInFlight) return + refreshInFlight = true try { - const creds = await AltimateApi.getCredentials() - const base = resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) - setManageUrl(base ? `${base.toString().replace(/\/$/, "")}/w/${b.datamateId}` : null) - } catch { - setManageUrl(null) + const dir = props.api.state.path.directory + const b = await readLocalBinding(dir).catch(() => null) + setBinding(b) + if (!b) { + setManageUrl(null) + return + } + const base = await resolveManageBase() + setManageUrl(base ? `${base}/w/${b.datamateId}` : null) + } finally { + refreshInFlight = false } } onMount(() => { void refresh() const timer = setInterval(() => void refresh(), POLL_MS) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(timer as any)?.unref?.() onCleanup(() => clearInterval(timer)) }) @@ -55,7 +90,7 @@ function View(props: { api: TuiPluginApi }) { when={binding()} fallback={ - Not linked — run /link + Not linked — run altimate-code link } > diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 2211a5ccd0..1e7154058c 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -336,8 +336,36 @@ async function runBrowserHandoff( toastHandoffFailure(api, result) return } - // Handoff succeeded — bind the project to the returned workspace via the - // existing bind endpoint. Same code path as PickerDialog's attach mode. + // M6 in the consensus review: the browser window can stay open for up to + // 15 minutes. If the user signs out or switches tenant mid-flow, the + // WorkspaceApi client re-reads credentials on every call — so a callback + // validated for tenant A would then bind under tenant B, and workspace + // ids are tenant-schema-local (same integer, different workspace). Compare + // the credential fingerprint the handoff was validated against with the + // credentials we're about to bind under, and refuse if either drifted. + try { + const fresh = await AltimateApi.getCredentials() + if ( + fresh.altimateInstanceName !== result.credentials.tenant || + fresh.altimateUrl !== result.credentials.apiUrl + ) { + api.ui.toast({ + variant: "error", + message: `Your Altimate credentials changed while the browser was open (was ${result.credentials.tenant}, now ${fresh.altimateInstanceName}). Re-run to link this project.`, + duration: 15_000, + }) + return + } + } catch { + api.ui.toast({ + variant: "error", + message: "Lost Altimate credentials while the browser was open — sign in and re-run.", + }) + return + } + // Handoff succeeded and credentials are still consistent — bind the project + // to the returned workspace via the existing bind endpoint. Same code path + // as PickerDialog's attach mode. try { const res = await WorkspaceApi.bindExisting(result.workspaceId, identifier) await recordApprovedBinding(api.state.path.directory, { diff --git a/packages/opencode/test/altimate/workspace/browser-handoff.test.ts b/packages/opencode/test/altimate/workspace/browser-handoff.test.ts index 386e89b589..a889d57443 100644 --- a/packages/opencode/test/altimate/workspace/browser-handoff.test.ts +++ b/packages/opencode/test/altimate/workspace/browser-handoff.test.ts @@ -204,7 +204,11 @@ describe("runHandoffWithOpener end-to-end", () => { } }) - test("URL includes project_remote + project_path + project_name from input", async () => { + test("URL includes project_name in query and project_remote/project_path in fragment", async () => { + // Per m6 in the consensus review: project_path + project_remote MUST NOT + // be sent as query params (they'd land in browser history, SaaS/CDN/WAF + // access logs, and REST-log aggregators). Move them to the URL fragment + // instead — same reason cli_context lives in the fragment. let observed = "" await runHandoffWithOpener( { @@ -219,9 +223,16 @@ describe("runHandoffWithOpener end-to-end", () => { }, ) const u = new URL(observed) - expect(u.searchParams.get("project_remote")).toBe("git@github.com:acme/foo.git") - expect(u.searchParams.get("project_path")).toBe("/w/foo") + // project_name is a display-safe label — the SaaS approval screen + // renders it in the modal — so it stays in the query. expect(u.searchParams.get("project_name")).toBe("foo") + // project_remote + project_path MUST NOT be in the query. + expect(u.searchParams.get("project_remote")).toBeNull() + expect(u.searchParams.get("project_path")).toBeNull() + // They live in the fragment instead. + const frag = new URLSearchParams(u.hash.replace(/^#/, "")) + expect(frag.get("project_remote")).toBe("git@github.com:acme/foo.git") + expect(frag.get("project_path")).toBe("/w/foo") expect(u.pathname).toBe("/create-and-link") }) }) From b26574072936e072ef1d97e80ce345b65df42fa8 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 10:45:23 +0530 Subject: [PATCH 13/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=205?= =?UTF-8?q?=20=E2=80=94=20no=20browser-handoff=20for=20already-linked=20pr?= =?UTF-8?q?oject,=20tighter=20workspace=5Fid=20spelling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two #1100-only cycle-5 findings: - **link.ts SET_UP_IN_BROWSER_SENTINEL** (CodeRabbit Major) — the browser handoff option was offered even when the project was already linked; ``runBrowserHandoff`` then created a fresh workspace and 409'd on ``bindExisting``, stranding the workspace. Gate the option on ``!existing`` alongside ``browserAvailable``. - **browser-handoff.ts workspace_id** (cubic P3) — ``Number()`` coerces ``"1e2"``, ``"0x2a"``, and ``" 42 "`` into finite integers, slipping past the ``isInteger`` guard. Require a plain decimal-digit spelling first. Test suite green (4072 pass across the altimate suite). Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- .../src/altimate/workspace/browser-handoff.ts | 13 ++++++++++++- packages/opencode/src/cli/cmd/link.ts | 8 +++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/browser-handoff.ts b/packages/opencode/src/altimate/workspace/browser-handoff.ts index a6cadfb0b7..b87ef133bd 100644 --- a/packages/opencode/src/altimate/workspace/browser-handoff.ts +++ b/packages/opencode/src/altimate/workspace/browser-handoff.ts @@ -256,9 +256,20 @@ async function startListener(pending: HandoffPending): Promise<{ server: Server; return } - const workspaceId = Number(workspaceIdRaw) // Integer-only: floats like ``42.5`` are rejected server-side but produce // a confusing failure the caller can't recover from. (m9 in the review.) + // Also reject non-canonical spellings — ``Number()`` happily coerces + // ``"1e2"``, ``"0x2a"``, and ``" 42 "`` into finite integers, so a + // callback URL carrying those forms would slip past ``isInteger`` and + // reach the bind payload. Requiring a plain decimal-digit string first + // is the tight gate. (cubic cycle 4/5.) + if (!/^[1-9][0-9]*$/.test(workspaceIdRaw)) { + const msg = `Invalid workspace_id: ${workspaceIdRaw}` + respond(400, htmlError(msg)) + pending.reject(markReason(new Error(msg), "error")) + return + } + const workspaceId = Number(workspaceIdRaw) if (!Number.isInteger(workspaceId) || workspaceId <= 0) { const msg = `Invalid workspace_id: ${workspaceIdRaw}` respond(400, htmlError(msg)) diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index 196ad8cb3d..d185025f16 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -115,7 +115,13 @@ export const LinkCommand = cmd({ resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) !== null const options: Array<{ value: string; label: string; hint?: string }> = [ - ...(browserAvailable + // Only offer browser handoff for UNLINKED projects (CodeRabbit cycle 5). + // ``runBrowserHandoff`` creates a fresh workspace and calls + // ``bindExisting``, which 409s when there's already an active binding — + // leaving the browser-created workspace stranded and no rebind actually + // happening. If the project is already linked, the caller wants a + // rebind path (offered elsewhere in this menu), not create-and-bind. + ...(browserAvailable && !existing ? [ { value: SET_UP_IN_BROWSER_SENTINEL, From 4f79ad3a9ff0b6d2d70dc8c8a36b2d0c0a114c6d Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 17 Aug 2026 12:08:29 +0530 Subject: [PATCH 14/14] =?UTF-8?q?fix(workspace):=20bot-review=20round=206?= =?UTF-8?q?=20=E2=80=94=20isBrowserHandoffAvailable=20guard,=20gate=20on?= =?UTF-8?q?=20preCheckOk,=20keep=20err=20diagnostic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three #1100-only cycle-6 findings: - **``isBrowserHandoffAvailable`` guarded** (CR Major) — line 63 wrapped ``isConfigured()`` with ``.catch(() => false)`` but ``getCredentials()`` on line 64 was unguarded. That call can throw on corrupt credentials JSON, Zod schema drift, or an unresolved ``${env:...}`` reference; an unhandled rejection there would take the TUI down. Wrap the whole body in try/catch and fail closed (treat as "handoff unavailable"). - **``link.ts`` browser option also gated on ``preCheckOk``** (Kilo suggestion) — was ``browserAvailable && !existing``. When the pre-check itself failed (network / 5xx), ``existing`` stays null while the project MAY be linked server-side. Offering the browser flow then reproduces the "workspace created + 409 on bindExisting" strand. Add ``&& preCheckOk``. - **``void err`` no-op replaced with log** (Kilo suggestion) — the previous ``catch (err) { ... void err }`` discarded the diagnostic. Log-warn so a regression in ``showLinkedConfirmation`` doesn't vanish silently. Test suite: green (4072 pass, 0 fail). Co-Authored-By: Claude Opus 4.7 Claude-Session: https://claude.ai/code/session_016H42Vt4pt5dcD7opRqckeM --- packages/opencode/src/cli/cmd/link.ts | 8 +++++++- .../src/plugin/tui/altimate/workspace.tsx | 19 +++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/cli/cmd/link.ts b/packages/opencode/src/cli/cmd/link.ts index d185025f16..68e740d1e9 100644 --- a/packages/opencode/src/cli/cmd/link.ts +++ b/packages/opencode/src/cli/cmd/link.ts @@ -121,7 +121,13 @@ export const LinkCommand = cmd({ // leaving the browser-created workspace stranded and no rebind actually // happening. If the project is already linked, the caller wants a // rebind path (offered elsewhere in this menu), not create-and-bind. - ...(browserAvailable && !existing + // + // Also gate on ``preCheckOk`` (Kilo cycle 6): when the pre-check itself + // failed (network / 5xx), ``existing`` stays null but the project MAY + // be linked server-side. Offering the browser flow then would run the + // same 409 → stranded-workspace path. Better to hide the option until + // the caller can confirm the binding state. + ...(browserAvailable && !existing && preCheckOk ? [ { value: SET_UP_IN_BROWSER_SENTINEL, diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 1e7154058c..6b7380e85a 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -60,9 +60,17 @@ const log = Log.create({ service: "altimate-workspace" }) * flow and the on-demand `altimate-code link` picker can hide the option * consistently when the deployment isn't supported. */ async function isBrowserHandoffAvailable(): Promise { - if (!(await AltimateApi.isConfigured().catch(() => false))) return false - const creds = await AltimateApi.getCredentials() - return resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) !== null + // Both credential calls can throw (corrupt JSON, schema drift, unresolved + // ``${env:...}`` reference). Callers use this in the sync arm of dialog + // rendering, so an unhandled rejection would take the TUI down. Fail + // closed — treat any credential error as "handoff unavailable". (CR cycle 6.) + try { + if (!(await AltimateApi.isConfigured().catch(() => false))) return false + const creds = await AltimateApi.getCredentials() + return resolveWorkspaceWebUrl(creds.altimateUrl, creds.altimateInstanceName) !== null + } catch { + return false + } } // ───────────────────────────────────────────────────────────────────────────── @@ -525,11 +533,14 @@ async function createAndBindInline( }) await showLinkedConfirmation(api, "Created", res.datamate.id, res.datamate.name) } catch (err) { + // Log the failure so a regression in ``showLinkedConfirmation`` doesn't + // vanish silently, then fall back to a plain toast. Previously ``void err`` + // discarded the diagnostic — Kilo cycle 6 called it out. + log.warn("workspace post-create confirmation failed", { err: String(err) }) api.ui.toast({ variant: "info", message: `Workspace "${res.datamate.name}" created and linked.`, }) - void err } }