From 51f4da48aeac512d951029543f42d2bf712eb402 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 03:34:26 +0800 Subject: [PATCH 01/21] feat(workspace): offer to install the engine a bound workspace needs A bound workspace whose declared tools need the local engine used to report a missing engine as a toast with a command in it. This replaces that with an offer: Install now / Copy command / Not now. The install only ever runs from an explicit choice; the turn boundary picks the installed engine up on the next message. The offer crosses to the TUI over the event bus and is re-derived there; headless prints one stderr line. --- .../src/altimate/workspace/engine-offer.ts | 199 +++++++++ .../src/altimate/workspace/engine-overlay.ts | 76 +++- .../src/altimate/workspace/engine-seams.ts | 7 + packages/opencode/src/cli/cmd/run.ts | 23 ++ .../src/plugin/tui/altimate/workspace.tsx | 378 +++++++++++++++++- .../test/altimate/plugin/workspace.test.ts | 72 +++- .../workspace/engine-install-offer.test.ts | 330 +++++++++++++++ .../altimate/workspace/engine-overlay.test.ts | 43 +- 8 files changed, 1093 insertions(+), 35 deletions(-) create mode 100644 packages/opencode/src/altimate/workspace/engine-offer.ts create mode 100644 packages/opencode/test/altimate/workspace/engine-install-offer.test.ts diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts new file mode 100644 index 000000000..955171898 --- /dev/null +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -0,0 +1,199 @@ +// altimate_change - new file +// +// The install offer for a workspace whose engine is missing or too old. +// +// State-free on purpose: the TUI plugin runs in its own module realm and +// receives the offer as a bare command over the event bus, so it re-derives +// the detail here from disk and PATH rather than from the overlay's memory. +// Offer, never install on the flow's own account — `installEngine` only ever +// runs from an explicit "Install now". +import { execFile } from "node:child_process" +import { Process } from "@/util/process" +import { AppRuntime } from "@/effect/app-runtime" +import { EventV2Bridge } from "@/event-v2-bridge" +import { TuiEvent } from "@/server/tui-event" +import { readLocalBinding } from "./state" +import { isHeadless, log, syncInternals } from "./engine-seams" +import { declaredBounded, notify, printLine, versionOf, which } from "./engine-probes" +import { ENGINE_BINARY, ENGINE_PACKAGE, MIN_ENGINE_VERSION, clearsFloor, type Toast } from "./engine-types" + +/** Node major the npm install path needs. The CLI itself is a self-contained + * binary and does not need Node — only this install route does. */ +export const MIN_NODE_MAJOR = 20 +/** How long "Install now" waits for npm before giving up. */ +export const INSTALL_TIMEOUT_MS = 300_000 +/** Command the TUI plugin registers to raise the install offer. The offer + * crosses to the TUI over the same event bus toasts use; it cannot cross + * in-process, because the plugin runtime loads plugins in a separate realm. + * `CommandExecute` carries no payload, so the plugin re-derives the offer + * with `describeOffer()`. */ +export const OFFER_COMMAND = "altimate.workspace.engineInstallOffer" + +/** A "no usable engine" state, described well enough for an interactive + * surface to act on it without re-deriving anything. */ +export type EngineOffer = { + reason: "engine-missing" | "engine-too-old" + /** Stable id — the 7-day "Not now" latch keys on this, not the name. */ + workspaceId: string + workspaceName: string + /** Declared, CLI-servable integration tools that are unavailable without it. */ + declared: number + /** Version found — only set for "engine-too-old". */ + found?: string + /** The exact install/update command. */ + command: string +} + +/** Interactive surface for the offer, in the same realm. Returns true when it + * took ownership. Deliberately synchronous: it claims the offer and renders + * out-of-band rather than making the turn boundary wait for a person. */ +export type OfferHandler = (offer: EngineOffer) => boolean + +export type InstallResult = { ok: true } | { ok: false; error: string } + +/** npm spec to install. ALTIMATE_ENGINE_INSTALL_SPEC overrides it so E2E can + * point the real install path at a local tarball instead of the registry. */ +export function installSpec(): string { + return process.env["ALTIMATE_ENGINE_INSTALL_SPEC"] || `${ENGINE_PACKAGE}@${MIN_ENGINE_VERSION}` +} + +/** The command shown, copied, printed, and run — always the same string, so + * "Copy command" hands over exactly what "Install now" would have executed. */ +export function installCommand(): string { + return `npm i -g ${installSpec()}` +} + +/** Re-derive the current "no usable engine" state for a directory, from the + * binding on disk and the engine on PATH. Null when there is nothing to offer: + * unbound, or an engine that clears the floor. */ +export async function describeOffer(directory: string): Promise { + const binding = syncInternals.resolveBinding + ? await syncInternals.resolveBinding(directory) + : await readLocalBinding(directory).catch(() => null) + if (!binding) return null + const workspaceId = String(binding.datamateId) + const bin = which(ENGINE_BINARY) + const found = bin ? await versionOf(bin) : null + if (bin && clearsFloor(found)) return null + const declared = (await declaredBounded(workspaceId))?.keys.length ?? 0 + return { + reason: bin ? "engine-too-old" : "engine-missing", + workspaceId, + workspaceName: binding.datamateName, + declared, + ...(bin ? { found: found ?? "unknown" } : {}), + command: installCommand(), + } +} + +/** Node major on PATH, or null when Node is absent. Gates "Install now": with + * no Node there is nothing to run npm with, so the offer shows the command. */ +export function nodeMajor(): Promise { + if (syncInternals.nodeMajor) return syncInternals.nodeMajor() + const bin = which("node") + if (!bin) return Promise.resolve(null) + return new Promise((resolve) => { + execFile(bin, ["--version"], { timeout: 5000 }, (err, stdout) => { + if (err) return resolve(null) + const major = Number.parseInt(stdout.trim().replace(/^v/, "").split(".")[0] ?? "", 10) + resolve(Number.isFinite(major) ? major : null) + }) + }) +} + +/** Whether npm can be invoked at all. Node and npm are separate packages on + * several Linux distributions, so Node 20+ does not imply `npm i -g` runs. */ +export function npmAvailable(): boolean { + if (syncInternals.npmAvailable) return syncInternals.npmAvailable() + return which(process.platform === "win32" ? "npm.cmd" : "npm") !== null +} + +/** Run the global install. Only ever reached from an explicit user choice. + * + * `npm.cmd` on Windows: a normal Node install exposes npm as a command shim, + * and nothing here spawns a shell. The deadline comes from an abort signal: + * `Process.spawn` consults `timeout` only inside its abort handler. A zero + * exit is not a usable engine — npm's global bin directory need not be on + * PATH — so the same discovery the turn boundary does runs before success. */ +export async function installEngine(): Promise { + const spec = installSpec() + if (syncInternals.install) return syncInternals.install(spec) + const npm = process.platform === "win32" ? "npm.cmd" : "npm" + const deadline = AbortSignal.timeout(INSTALL_TIMEOUT_MS) + try { + const result = await Process.run([npm, "i", "-g", spec], { abort: deadline, nothrow: true }) + if (result.code === 0) { + const installedBin = which(ENGINE_BINARY) + if (!installedBin) { + return { + ok: false, + error: `npm installed it, but ${ENGINE_BINARY} is not on PATH — add your npm global bin directory to PATH`, + } + } + const installedVersion = await versionOf(installedBin) + if (!clearsFloor(installedVersion)) { + return { + ok: false, + error: `npm installed it, but ${ENGINE_BINARY} on PATH reports ${installedVersion ?? "no version"}`, + } + } + return { ok: true } + } + if (deadline.aborted) { + return { ok: false, error: `npm did not finish within ${Math.round(INSTALL_TIMEOUT_MS / 60_000)} minutes` } + } + const detail = result.stderr.toString().trim().split(/\r?\n/).slice(-3).join(" ") + return { ok: false, error: detail || `npm exited with code ${result.code}` } + } catch (err) { + if (deadline.aborted) { + return { ok: false, error: `npm did not finish within ${Math.round(INSTALL_TIMEOUT_MS / 60_000)} minutes` } + } + return { ok: false, error: err instanceof Error ? err.message : String(err) } + } +} + +/** Ask the TUI to raise the offer. False when the bus is unavailable. */ +async function publishOffer(): Promise { + if (syncInternals.publishOffer) return syncInternals.publishOffer() + try { + await AppRuntime.runPromise( + EventV2Bridge.Service.use((events) => events.publish(TuiEvent.CommandExecute, { command: OFFER_COMMAND })), + ) + return true + } catch (err) { + log.warn("could not publish the engine install offer", { err: String(err) }) + return false + } +} + +/** Hand the offer to a same-realm surface. False when none is registered. */ +function offerInstall(offer: EngineOffer): boolean { + const handler = syncInternals.offer + if (!handler) return false + try { + return handler(offer) + } catch (err) { + log.warn("install offer surface failed; falling back to toast", { err: String(err) }) + return false + } +} + +/** One printed line for headless `run`. */ +export function describeOfferLine(offer: EngineOffer): string { + const tools = `${offer.declared} integration tool${offer.declared === 1 ? "" : "s"}` + return offer.reason === "engine-too-old" + ? `Workspace "${offer.workspaceName}": ${tools} need ${ENGINE_BINARY} ${MIN_ENGINE_VERSION}+ (found ${offer.found ?? "unknown"}). Update with: ${offer.command}` + : `Workspace "${offer.workspaceName}": ${tools} need the local engine, which is not installed. Install it with: ${offer.command}` +} + +/** Offer via the dialog surface when there is one; otherwise print (headless) + * or toast (bus unavailable). Exactly one of these happens. */ +export async function offerOrNotify(offer: EngineOffer, toast: Toast): Promise { + if (isHeadless()) { + printLine(describeOfferLine(offer)) + return + } + if (offerInstall(offer)) return + if (await publishOffer()) return + await notify(toast) +} diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index b88a85fba..15bc40d4e 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -36,9 +36,10 @@ import { type ScopedBinding, } from "./engine-seams" import { declaredBounded, notify, printLine, resolveBinding, versionOf, which } from "./engine-probes" +import { installCommand, offerOrNotify, type EngineOffer } from "./engine-offer" import { ENGINE_BINARY, - INSTALL_COMMAND, + INSTALL_HELPS, REPAIRABLE, TOOL_PREFIX, clearsFloor, @@ -56,6 +57,7 @@ import { } from "./engine-types" export * from "./engine-types" +export * from "./engine-offer" export { isEnabled, isHeadless, isServe, syncInternals } from "./engine-seams" /** Sessions remembered per process. It is a memo; an evicted session just re-settles. */ @@ -78,10 +80,16 @@ function now(): number { async function probeEngine(): Promise { const at = now() - if (probeMemo && (probeMemo.result.kind === "ok" || at - probeMemo.at < FAILED_PROBE_TTL_MS)) { + const bin = which(ENGINE_BINARY) + // A usable engine is remembered for the process. A missing one is asked + // about on every call — `which` is a PATH scan, no process spawn — so an + // install made from the offer dialog (which runs in another module realm and + // cannot reach this memo) is seen on the next turn. A too-old or broken one + // costs a spawn to re-check, so that is rate-limited by the TTL. + if (probeMemo && probeMemo.result.kind === "ok") return probeMemo.result + if (probeMemo && probeMemo.result.kind === "too-old" && bin && at - probeMemo.at < FAILED_PROBE_TTL_MS) { return probeMemo.result } - const bin = which(ENGINE_BINARY) let result: Probe if (!bin) { result = { kind: "missing" } @@ -545,19 +553,43 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS count === undefined ? `Workspace "${workspace.name}" has integration tools that run on the local engine, which is not installed.` : `Workspace "${workspace.name}" declares ${count} integration tool${count === 1 ? "" : "s"}. They run on the local engine, which is not installed.` - await announceRefusal(sessionID, outcome, { - title: `Workspace "${workspace.name}" needs the local engine`, - message: `${what} Install it with: ${INSTALL_COMMAND}`, - variant: "warning", - }) + await announceRefusal( + sessionID, + outcome, + { + title: `Workspace "${workspace.name}" needs the local engine`, + message: `${what} Install it with: ${installCommand()}`, + variant: "warning", + }, + { + reason: "engine-missing", + workspaceId: workspace.id, + workspaceName: workspace.name, + declared: count ?? 0, + command: installCommand(), + }, + ) return } record(sessionID, refusal) - await announceRefusal(sessionID, refusal, { - title: `Workspace "${workspace.name}": engine not usable`, - message: describeRefusal(refusal.found, workspace.name), - variant: "warning", - }) + const declared = (await declaredFor(workspace.id))?.keys.length ?? 0 + await announceRefusal( + sessionID, + refusal, + { + title: `Workspace "${workspace.name}": engine not usable`, + message: describeRefusal(refusal.found, workspace.name), + variant: "warning", + }, + { + reason: "engine-too-old", + workspaceId: workspace.id, + workspaceName: workspace.name, + declared, + found: refusal.found ?? "unknown", + command: installCommand(), + }, + ) return } @@ -625,16 +657,26 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS /** Tell the session about a refusal, once per unchanged verdict. * - * The substitution point for the install offer: when `installWouldHelp(outcome)` - * a dialog replaces the toast here; it never adds a second message. Headless - * `run` prints one stderr line instead. */ -export async function announceRefusal(sessionID: string, outcome: Outcome, toast: Toast): Promise { + * The substitution point for the install offer: when installing would help + * and an `offer` is supplied, the offer surface (dialog, headless line, or + * toast fallback) replaces the toast — never adds to it. Otherwise headless + * `run` prints one stderr line and the TUI gets the toast. */ +export async function announceRefusal( + sessionID: string, + outcome: Outcome, + toast: Toast, + offer?: EngineOffer, +): Promise { const rec = sessions.get(sessionID) ?? record(sessionID, outcome) const detail = "error" in outcome ? outcome.error : "found" in outcome ? String(outcome.found) : "" const declared = "declared" in outcome ? String(outcome.declared ?? "?") : "" const signature = `${outcome.kind}:${detail}:${declared}:${toast.title}` if (rec.announced === signature) return rec.announced = signature + if (offer && INSTALL_HELPS[outcome.kind]) { + await offerOrNotify(offer, toast) + return + } if (isHeadless()) { printLine(`${toast.title}: ${toast.message}`) return diff --git a/packages/opencode/src/altimate/workspace/engine-seams.ts b/packages/opencode/src/altimate/workspace/engine-seams.ts index 6b924f6b1..a823e25a5 100644 --- a/packages/opencode/src/altimate/workspace/engine-seams.ts +++ b/packages/opencode/src/altimate/workspace/engine-seams.ts @@ -7,6 +7,7 @@ import { Instance } from "@/project/instance" import { Log } from "@/altimate/util/log" import type { CachedBinding } from "./state" import type { Declared, LocalMcpConfig, McpEntry, McpStatus, Toast } from "./engine-types" +import type { EngineOffer, InstallResult } from "./engine-offer" export const log = Log.create({ service: "workspace-engine" }) @@ -27,6 +28,12 @@ export const syncInternals: { declared?: (workspaceId: string) => Promise notify?: (toast: Toast) => Promise printLine?: (line: string) => void + /** Install-offer seams (see engine-offer.ts). */ + offer?: (offer: EngineOffer) => boolean + publishOffer?: () => Promise + nodeMajor?: () => Promise + npmAvailable?: () => boolean + install?: (spec: string) => Promise instanceDirectory?: () => string | null headless?: () => boolean serve?: () => boolean diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 49debaad7..b3015d8a1 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -50,6 +50,8 @@ import { SessionTermination } from "../../session/termination" // altimate_change end // altimate_change start — upstream_fix: type-only import for the tracing-config cast (see tracer setup below) import type { ConfigV1 } from "@opencode-ai/core/v1/config/config" +// altimate_change - render the workspace engine offer in an attached run +import { OFFER_COMMAND, installCommand } from "@/altimate/workspace/engine-offer" // altimate_change end // When a tool's parameters can't be statically inferred (legacy fork tools whose @@ -925,6 +927,27 @@ You are speaking to a non-technical business executive. Follow these rules stric sawBusy = true } // altimate_change end + + // altimate_change start — an attached run is the only surface that can + // show the workspace engine offer. With --attach the turn boundary and + // isHeadless() run in the server process, which publishes the offer + // command and treats the publish as delivery — while this event loop, + // which has no handler for it, is the only thing the user is looking at. + // Placed before the idle break: the loop stops on idle, so a handler + // after it never runs for an offer that arrives in the same batch. + if ( + event.type === "tui.command.execute" && + (event.properties as { command?: string }).command === OFFER_COMMAND + ) { + // stderr: stdout is raw JSON events under --format json. + process.stderr.write( + `This workspace's integration tools need the local engine on the server. Install it there with: ${installCommand()}` + + EOL, + ) + continue + } + // altimate_change end + if ( event.type === "session.status" && event.properties.sessionID === sessionID && diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 2a75f93c4..201f9b34e 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -24,8 +24,9 @@ import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import { createHash } from "node:crypto" +import { existsSync } from "node:fs" import open from "open" -import { createSignal, onMount } from "solid-js" +import { createSignal, onCleanup, onMount } from "solid-js" import { ConflictError, ForbiddenError, @@ -48,6 +49,17 @@ import { resolveProjectIdentifier, } from "@/altimate/workspace/detect" import { readLocalBinding, recordApprovedBinding } from "@/altimate/workspace/state" +import { + describeOffer, + installCommand, + installEngine, + nodeMajor as detectNodeMajor, + npmAvailable, + MIN_NODE_MAJOR, + OFFER_COMMAND, + type EngineOffer, +} from "@/altimate/workspace/engine-offer" +import { useClipboard } from "@opencode-ai/tui/context/clipboard" import { AltimateApi } from "@/altimate/api/client" import { Log } from "@/altimate/util/log" @@ -1143,6 +1155,357 @@ async function runFlow(api: TuiPluginApi, directory: string): Promise { )) } +// ───────────────────────────────────────────────────────────────────────────── +// Engine install offer. The workspace engine overlay decides there is no usable engine and hands +// the offer here; this file owns the interaction. Offer, never silently +// install — the install only ever runs from an explicit "Install now". +// ───────────────────────────────────────────────────────────────────────────── + +const KV_ENGINE_SKIP_PREFIX = "altimate.workspace.engineInstall.skip." + +/** Latch key from (tenant, apiUrl, workspace id). Keyed on the workspace so a + * "Not now" for one workspace doesn't silence the offer for another, and on + * the id rather than the name so a rename doesn't reset it. */ +function engineSkipKey(workspaceId: string, scope: LatchScope | null): string { + const scopeString = scope ? `${scope.tenant}|${scope.apiUrl}|` : "" + return ( + KV_ENGINE_SKIP_PREFIX + + createHash("sha1") + .update(scopeString + workspaceId) + .digest("hex") + ) +} + +/** Same 7-day TTL and clock-rewind handling as the post-scan latch. */ +function isEngineSkipActive( + api: TuiPluginApi, + workspaceId: string, + scope: LatchScope | null, + nowMs: number, +): boolean { + const rec = api.kv.get<{ skippedAt: number }>(engineSkipKey(workspaceId, scope)) + if (!rec || typeof rec.skippedAt !== "number") return false + const delta = nowMs - rec.skippedAt + if (delta < 0) return false + return delta < SKIP_TTL_MS +} + +function recordEngineSkip( + api: TuiPluginApi, + workspaceId: string, + scope: LatchScope | null, + nowMs: number, +): void { + api.kv.set(engineSkipKey(workspaceId, scope), { skippedAt: nowMs }) +} + +interface EngineOfferProps { + api: TuiPluginApi + offer: EngineOffer + /** Node major on PATH, or null when Node is absent. Resolved by the caller + * so the dialog itself stays sync (same shape as ``browserAvailable``). */ + nodeMajor: number | null + /** Whether npm itself can be invoked. Node 20+ is not enough: several Linux + * distributions package node and npm separately. */ + hasNpm: boolean + latchScope: LatchScope | null + /** Which raise owns the single-offer latch; only the owner releases it. */ + generation: number +} + +/** One DialogSelect for every phase — the row set changes, the component never + * does. Swapping the top-level dialog component between states tears down and + * remounts the dialog, which drops focus and loses the phase signal. Sentinel + * rows carry the non-idle phases, and never use ``disabled: true`` ( + * DialogSelect's ``filtered()`` drops those, leaving an empty list). */ +function EngineInstallOfferDialog(props: EngineOfferProps) { + const clipboard = useClipboard() + const [phase, setPhase] = createSignal<"idle" | "installing" | "installed" | "failed">("idle") + const [failure, setFailure] = createSignal(null) + // The install outlives this component: Escape or a click outside dismisses + // the dialog while npm keeps running. Signals set after that update nothing + // anyone can see — a failed install or the five-minute timeout would be + // completely silent — and clearing the dialog stack would close whatever + // opened in our place. So completion reports through a toast when we are + // gone, and only touches the dialog while we still own it. + let mounted = true + onCleanup(() => { + mounted = false + // Release the single-offer latch however this dialog goes away — chosen, + // dismissed, or replaced — but only if this dialog still owns it. A + // superseded dialog tearing down must not free a slot the newer one holds. + if (engineOfferGeneration === props.generation) engineOfferVisible = false + }) + // ``onSelect`` is delivered synchronously per Enter keypress; the install is + // a multi-minute await. Without this latch a second Enter starts a second + // ``npm i -g`` against the same global prefix. + let installing = false + + const command = () => props.offer.command + const canInstall = () => props.nodeMajor !== null && props.nodeMajor >= MIN_NODE_MAJOR && props.hasNpm + + const title = () => { + const tools = `${props.offer.declared} integration tool${props.offer.declared === 1 ? "" : "s"}` + const head = + props.offer.reason === "engine-too-old" + ? `Workspace "${props.offer.workspaceName}" needs a newer local engine (found ${props.offer.found ?? "unknown"}) — ${tools} unavailable` + : `Workspace "${props.offer.workspaceName}" declares ${tools}, which need the local engine` + const parts = [head, command()] + if (!canInstall()) { + parts.push( + props.nodeMajor === null + ? `(needs Node ${MIN_NODE_MAJOR}+ to install — Node was not found on PATH)` + : props.nodeMajor < MIN_NODE_MAJOR + ? `(needs Node ${MIN_NODE_MAJOR}+ to install — found Node ${props.nodeMajor})` + : `(needs npm to install — npm was not found on PATH)`, + ) + } + const err = failure() + if (err) parts.push(`(install failed: ${err})`) + return parts.join(" · ") + } + + const options = () => { + switch (phase()) { + case "installing": + return [{ title: "Installing… this can take a minute.", value: "busy" }] + case "installed": + return [{ title: "Installed — attaching integrations.", value: "close" }] + case "failed": + return [ + { title: "Copy command", value: "copy", description: "Run it yourself, then start a new session." }, + { title: "Close", value: "close" }, + ] + default: + return [ + ...(canInstall() + ? [ + { + title: "Install now", + value: "install", + description: `Runs ${command()} and attaches this session when it finishes.`, + }, + ] + : []), + { + title: "Copy command", + value: "copy", + description: "Copy the install command to your clipboard.", + }, + { + title: "Not now", + value: "skip", + description: "Won't ask again for this workspace for 7 days.", + }, + ] + } + } + + const runInstall = async () => { + setPhase("installing") + engineInstallInFlight = true + try { + await performInstall() + } finally { + engineInstallInFlight = false + } + } + + const performInstall = async () => { + const result = await installEngine() + if (!result.ok) { + installing = false + if (!mounted) { + // Dismissed mid-install: the failed-phase rows have nowhere to render, + // so the error reaches the user as a toast or not at all. + props.api.ui.toast({ + variant: "error", + message: `Workspace engine install failed: ${result.error}. Run: ${command()}`, + duration: 30_000, + }) + return + } + setFailure(result.error) + setPhase("failed") + return + } + setPhase("installed") + // Only clear a dialog we still own — by now the user may have opened + // another, and clearing the stack would take theirs down instead. + if (mounted) props.api.ui.dialog.clear() + // Deliberately NOT reconciling this session from here. The plugin runtime + // loads this file in its own realm, so the overlay module here is not the + // one the server consults. Nothing needs to: the turn boundary looks for a + // missing engine on PATH again every turn, so the engine just installed is + // picked up on the next message without a restart. + props.api.ui.toast({ + variant: "success", + message: + `Workspace engine installed. Integration tools for "${props.offer.workspaceName}" ` + + `attach on your next message.`, + duration: 15_000, + }) + } + + const copyCommand = () => { + const cmd = command() + void (async () => { + try { + await clipboard.write?.(cmd) + // A resolved write is NOT proof of a copy. The host's writer picks + // xclip/xsel on Linux and otherwise falls back to clipboardy, and it + // swallows backend failures (`.catch(() => undefined)`), so on the many + // Linux/WSL boxes with neither tool installed the write silently does + // nothing. Read back and compare before claiming success. (Caught by + // E2E: the toast said "Copied:" while the clipboard was untouched.) + const back = await clipboard.read?.() + if (back?.data.trim() === cmd) { + props.api.ui.toast({ variant: "info", message: `Copied: ${cmd}` }) + return + } + } catch { + // Unreadable or unwritable clipboard — fall through and show it. + } + props.api.ui.toast({ + variant: "warning", + message: `Could not confirm the clipboard. Run: ${cmd}`, + duration: 30_000, + }) + })() + } + + return ( + { + if (option.value === "busy") return + if (option.value === "install") { + if (installing) return + installing = true + void runInstall().catch((err) => { + setFailure(err instanceof Error ? err.message : String(err)) + setPhase("failed") + installing = false + }) + return + } + if (option.value === "copy") { + copyCommand() + props.api.ui.dialog.clear() + return + } + if (option.value === "skip") { + recordEngineSkip(props.api, props.offer.workspaceId, props.latchScope, Date.now()) + } + props.api.ui.dialog.clear() + }} + /> + ) +} + +/** Show the offer unless the 7-day latch suppresses it. + * + * The overlay raises this as a bare command over the event bus — it cannot hand + * us the offer object, because the plugin runtime loads this file in a separate + * realm from the attach flow. So the detail is re-derived here, the same way + * the post-scan prompt re-derives its own state from the directory. Node + * availability and latch scope are resolved before rendering so the dialog + * itself stays sync. */ +let engineOfferVisible = false +/** Identifies which raise owns the latch, so a superseded dialog's teardown + * cannot free a slot that a newer dialog is still holding. */ +let engineOfferGeneration = 0 +/** Held for the lifetime of an `npm i -g`, independently of the dialog. + * + * The install outlives the dialog that started it: dismissing mid-install + * tears the component down and frees the offer latch, but npm keeps running. + * Without this, the next turn's repair retry raises a fresh offer whose + * "Install now" starts a SECOND `npm i -g` against the same global prefix. + * The dialog latch answers "is an offer on screen"; this one answers "is an + * install still running", and only the second survives dismissal. */ +let engineInstallInFlight = false + +async function showEngineInstallOffer(api: TuiPluginApi): Promise { + // The attach re-probes a repairable failure on every turn, so the offer can + // be raised again while an earlier one is still up — including mid-install, + // where a fresh idle dialog replaces the "Installing…" one and then swallows + // the user's keystrokes into its own filter. Observed end-to-end: after a + // successful install the pane showed a second offer in its idle phase and + // typing went to the dialog rather than the prompt. One offer at a time. + // + // The slot is reserved BEFORE the first await. Discovery below awaits three + // times, and a check-then-act guard placed after them lets two dispatches + // that arrive close together both pass — which is worse than the bug it + // fixes, because the second dialog can replace an installing one and start a + // concurrent global npm install. + // `attach ` runs this plugin on the CLIENT while the binding, the PATH + // that matters and the MCP session all live on the SERVER. Probing PATH here + // would describe the wrong machine, and "Install now" would install npm on + // the client, leaving the server exactly as it was behind a success toast. + // attach.ts recognises that case the same way — the server's directory does + // not exist locally — so use it and refuse to act, saying where the fix goes. + // + // Not a complete answer: a client that happens to have the same path, with a + // binding, is still misread. Closing that needs server-side discovery and + // install behind an API, which this PR does not add. + if (!existsSync(api.state.path.directory)) { + log.info("engine install offer suppressed: not the host that owns this workspace") + api.ui.toast({ + variant: "warning", + message: `This workspace's engine is missing on the server, not on this machine. Run there: ${installCommand()}`, + duration: 30_000, + }) + return + } + if (engineOfferVisible) return + if (engineInstallInFlight) { + // An install started from an earlier dialog is still running; offering + // again would invite a second concurrent global install. + log.info("engine install offer suppressed while an install is in flight") + return + } + engineOfferVisible = true + const generation = ++engineOfferGeneration + const release = () => { + // Only the current owner may free the slot. + if (engineOfferGeneration === generation) engineOfferVisible = false + } + try { + const offer = await describeOffer(api.state.path.directory) + // Null means the situation resolved between the attach and this dialog — an + // engine appeared, or the project is no longer bound. Say nothing. + if (!offer) return release() + const latchScope = await currentLatchScope() + if (isEngineSkipActive(api, offer.workspaceId, latchScope, Date.now())) { + log.info("engine install offer suppressed by 7-day latch", { workspaceId: offer.workspaceId }) + return release() + } + const major = await detectNodeMajor() + const hasNpm = npmAvailable() + api.ui.dialog.replace(() => ( + + )) + } catch (err) { + release() + throw err + } +} + // ───────────────────────────────────────────────────────────────────────────── // Plugin registration // ───────────────────────────────────────────────────────────────────────────── @@ -1173,6 +1536,17 @@ const tui: TuiPlugin = async (api) => { runFlow(api, api.state.path.directory).catch((err) => reportFlowFailure(api, err)) }, }, + { + // Raised by the workspace engine overlay over the event bus when a bound workspace has + // no usable engine. Internal: dispatched, never shown in the palette. + name: OFFER_COMMAND, + title: "Workspace engine install offer", + category: "Altimate", + namespace: "internal", + run() { + showEngineInstallOffer(api).catch((err) => reportFlowFailure(api, err)) + }, + }, { name: "altimate.workspace.link", title: "Link this project to a workspace", @@ -1195,5 +1569,5 @@ 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 } +export { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip } // altimate_change end diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index f545c3e0d..08716c177 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -28,7 +28,7 @@ afterAll(() => { } }) -const { isSkipActive, recordSkip } = await import( +const { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip } = await import( "../../../src/plugin/tui/altimate/workspace" ) const { projectNameFromRemote, detectProjectRemote } = await import( @@ -533,3 +533,73 @@ describe("Skip latch", () => { ).toBe(false) }) }) + +// ───────────────────────────────────────────────────────────────────────────── +// Engine install-offer latch — "Not now" silences the offer for 7 days, per +// workspace. Keyed on the workspace id so a rename doesn't reset it, and +// scoped by (tenant, apiUrl) like the post-scan latch. +// ───────────────────────────────────────────────────────────────────────────── + +describe("Engine install-offer latch", () => { + const scope = { tenant: "acme", apiUrl: "https://api.acme.example.com" } + const workspaceId = "42" + const DAY = 24 * 60 * 60 * 1000 + + test("no record → not active", () => { + const api = { kv: makeKv() } as any + expect(isEngineSkipActive(api, workspaceId, scope, Date.now())).toBe(false) + }) + + test("recorded within 7 days → active", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordEngineSkip(api, workspaceId, scope, now) + expect(isEngineSkipActive(api, workspaceId, scope, now + 6 * DAY)).toBe(true) + }) + + test("recorded past 7 days → not active", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordEngineSkip(api, workspaceId, scope, now) + expect(isEngineSkipActive(api, workspaceId, scope, now + 8 * DAY)).toBe(false) + }) + + test("boundary at exactly 7 days → not active", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordEngineSkip(api, workspaceId, scope, now) + expect(isEngineSkipActive(api, workspaceId, scope, now + 7 * DAY)).toBe(false) + }) + + test("latching one workspace does not silence another", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordEngineSkip(api, "42", scope, now) + expect(isEngineSkipActive(api, "42", scope, now + DAY)).toBe(true) + expect(isEngineSkipActive(api, "43", scope, now + DAY)).toBe(false) + }) + + test("a latch in one account does not apply to another", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordEngineSkip(api, workspaceId, scope, now) + const other = { tenant: "globex", apiUrl: "https://api.globex.example.com" } + expect(isEngineSkipActive(api, workspaceId, other, now + DAY)).toBe(false) + }) + + test("a future timestamp (clock rewind) re-offers instead of latching forever", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + recordEngineSkip(api, workspaceId, scope, now + 5 * DAY) + expect(isEngineSkipActive(api, workspaceId, scope, now)).toBe(false) + }) + + test("the post-scan latch and the engine latch are independent", () => { + const api = { kv: makeKv() } as any + const now = 1_700_000_000_000 + const ident = { repoRemote: "git@github.com:acme/proj-a.git", projectPath: "/work/proj-a" } + recordSkip(api, ident, scope, now) + expect(isSkipActive(api, ident, scope, now + DAY)).toBe(true) + expect(isEngineSkipActive(api, workspaceId, scope, now + DAY)).toBe(false) + }) +}) diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts new file mode 100644 index 000000000..33e2e3fb2 --- /dev/null +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -0,0 +1,330 @@ +// altimate_change - new file +// +// The "no usable engine" offer: which surface gets it, what the fallback +// emits when there is no surface, what the TUI re-derives, and the install +// path's gates and verification. Everything routes through `syncInternals`. +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import { + ENGINE_BINARY, + ENGINE_PACKAGE, + MIN_ENGINE_VERSION, + beforeTurn, + describeOffer, + installCommand, + installEngine, + installSpec, + nodeMajor, + resetForTests, + settledOutcome, + syncInternals, + type EngineOffer, + type Toast, +} from "../../../src/altimate/workspace/engine-overlay" +import { Process } from "../../../src/util/process" +import type { CachedBinding } from "../../../src/altimate/workspace/state" + +const DIR = "/tmp/analytics" +const ORIGINAL_FLAG = process.env.ALTIMATE_WORKSPACE +const ORIGINAL_SPEC = process.env.ALTIMATE_ENGINE_INSTALL_SPEC + +const binding: CachedBinding = { + datamateId: 42, + datamateName: "analytics", + repoRemote: null, + projectPath: DIR, + linkedAt: 0, +} as CachedBinding + +type Harness = { offers: EngineOffer[]; toasts: Toast[]; printed: string[]; published: number } + +/** No engine on PATH (or an old one) plus captured surfaces. */ +function install(opts: { + which?: string | null + version?: string | null + declaredKeys?: string[] + headless?: boolean + bus?: boolean + surface?: boolean + bound?: boolean +}): Harness { + const h: Harness = { offers: [], toasts: [], printed: [], published: 0 } + process.env.ALTIMATE_WORKSPACE = "1" + syncInternals.serve = () => false + syncInternals.headless = () => opts.headless === true + syncInternals.instanceDirectory = () => DIR + syncInternals.resolveBinding = async () => (opts.bound === false ? null : binding) + syncInternals.which = () => (opts.which === undefined ? null : opts.which) + syncInternals.versionOf = async () => (opts.version === undefined ? null : opts.version) + syncInternals.declared = async () => ({ + keys: opts.declaredKeys ?? ["dbt_build_model", "dbt_compile_model"], + extensionKeys: [], + }) + syncInternals.notify = async (toast) => { + h.toasts.push(toast) + } + syncInternals.printLine = (line) => { + h.printed.push(line) + } + syncInternals.publishOffer = async () => { + if (opts.bus === false) return false + h.published += 1 + return true + } + if (opts.surface) { + syncInternals.offer = (offer) => { + h.offers.push(offer) + return true + } + } + const config: { mcp?: Record } = { mcp: {} } + let loaded = false + syncInternals.config = { + invalidate: async () => { + loaded = false + }, + get: async () => { + if (!loaded) { + const { overlay } = await import("../../../src/altimate/workspace/engine-overlay") + config.mcp = {} + await overlay(DIR, config) + loaded = true + } + return config + }, + } + syncInternals.mcp = { + status: async () => ({ datamate: { status: "connected" } }), + add: async () => {}, + remove: async () => {}, + tools: async () => ({}), + } + return h +} + +beforeEach(() => resetForTests()) +afterEach(() => { + resetForTests() + for (const key of Object.keys(syncInternals)) delete (syncInternals as Record)[key] + if (ORIGINAL_FLAG === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_FLAG + if (ORIGINAL_SPEC === undefined) delete process.env.ALTIMATE_ENGINE_INSTALL_SPEC + else process.env.ALTIMATE_ENGINE_INSTALL_SPEC = ORIGINAL_SPEC +}) + +describe("install command", () => { + test("pins the minimum engine version by default", () => { + delete process.env.ALTIMATE_ENGINE_INSTALL_SPEC + expect(installSpec()).toBe(`${ENGINE_PACKAGE}@${MIN_ENGINE_VERSION}`) + expect(installCommand()).toBe(`npm i -g ${ENGINE_PACKAGE}@${MIN_ENGINE_VERSION}`) + }) + test("honours ALTIMATE_ENGINE_INSTALL_SPEC so E2E can point at a tarball", () => { + process.env.ALTIMATE_ENGINE_INSTALL_SPEC = "/tmp/datamate.tgz" + expect(installCommand()).toBe("npm i -g /tmp/datamate.tgz") + }) +}) + +describe("nodeMajor", () => { + test("null when node is not on PATH", async () => { + syncInternals.which = () => null + expect(await nodeMajor()).toBeNull() + }) +}) + +describe("offer routing — engine missing", () => { + test("a same-realm surface takes the offer and nothing else is emitted", async () => { + const h = install({ surface: true }) + await beforeTurn("s1") + expect(h.offers).toEqual([ + { + reason: "engine-missing", + workspaceId: "42", + workspaceName: "analytics", + declared: 2, + command: installCommand(), + }, + ]) + expect(h.published).toBe(0) + expect(h.toasts).toEqual([]) + expect(h.printed).toEqual([]) + expect(settledOutcome("s1")).toEqual({ kind: "engine-missing", declared: 2 }) + }) + test("in a TUI the offer is published over the bus and nothing is printed or toasted", async () => { + const h = install({}) + await beforeTurn("s1") + expect(h.published).toBe(1) + expect(h.toasts).toEqual([]) + expect(h.printed).toEqual([]) + }) + test("falls back to the toast only when the bus is unavailable", async () => { + const h = install({ bus: false }) + await beforeTurn("s1") + expect(h.published).toBe(0) + expect(h.toasts).toHaveLength(1) + expect(h.toasts[0].message).toContain(installCommand()) + }) + test("headless prints exactly one line naming workspace and command, and no toast", async () => { + const h = install({ headless: true }) + await beforeTurn("s1") + await beforeTurn("s1") + expect(h.printed).toEqual([ + `Workspace "analytics": 2 integration tools need the local engine, which is not installed. Install it with: ${installCommand()}`, + ]) + expect(h.toasts).toEqual([]) + expect(h.published).toBe(0) + }) + test("singularises the tool count", async () => { + const h = install({ headless: true, declaredKeys: ["dbt_build_model"] }) + await beforeTurn("s1") + expect(h.printed[0]).toContain("1 integration tool need") + }) + test("the offer is raised once per session per verdict", async () => { + const h = install({}) + await beforeTurn("s1") + await beforeTurn("s1") + expect(h.published).toBe(1) + await beforeTurn("s2") + expect(h.published).toBe(2) + }) +}) + +describe("offer routing — engine too old", () => { + test("carries the found version and the update command", async () => { + const h = install({ surface: true, which: "/usr/local/bin/datamate", version: "0.6.3" }) + await beforeTurn("s1") + expect(h.offers).toEqual([ + { + reason: "engine-too-old", + workspaceId: "42", + workspaceName: "analytics", + declared: 2, + found: "0.6.3", + command: installCommand(), + }, + ]) + expect(settledOutcome("s1")).toEqual({ kind: "engine-too-old", found: "0.6.3" }) + }) + test("headless, the printed line names the found version", async () => { + const h = install({ headless: true, which: "/usr/local/bin/datamate", version: "0.6.3" }) + await beforeTurn("s1") + expect(h.printed).toEqual([ + `Workspace "analytics": 2 integration tools need ${ENGINE_BINARY} ${MIN_ENGINE_VERSION}+ (found 0.6.3). Update with: ${installCommand()}`, + ]) + }) + test("a broken engine reports 'unknown' rather than a version", async () => { + const h = install({ surface: true, which: "/usr/local/bin/datamate", version: null }) + await beforeTurn("s1") + expect(h.offers[0]).toMatchObject({ reason: "engine-too-old", found: "unknown" }) + }) +}) + +describe("offer is not raised when an engine is usable", () => { + test("a healthy engine never reaches the offer path", async () => { + const h = install({ surface: true, which: "/usr/local/bin/datamate", version: "0.7.0" }) + await beforeTurn("s1") + expect(h.offers).toEqual([]) + expect(h.published).toBe(0) + expect(h.printed).toEqual([]) + expect(settledOutcome("s1")?.kind).toBe("attached") + }) +}) + +describe("describeOffer — the TUI re-derives its own detail", () => { + test("describes a missing engine", async () => { + install({}) + expect(await describeOffer(DIR)).toEqual({ + reason: "engine-missing", + workspaceId: "42", + workspaceName: "analytics", + declared: 2, + command: installCommand(), + }) + }) + test("describes an engine below the floor, naming the version found", async () => { + install({ which: "/usr/local/bin/datamate", version: "0.6.3" }) + expect(await describeOffer(DIR)).toMatchObject({ reason: "engine-too-old", found: "0.6.3" }) + }) + test("returns null when an engine already clears the floor", async () => { + install({ which: "/usr/local/bin/datamate", version: "0.7.0" }) + expect(await describeOffer(DIR)).toBeNull() + }) + test("returns null when the project is not bound", async () => { + install({ bound: false }) + expect(await describeOffer(DIR)).toBeNull() + }) +}) + +describe("headless notice stream", () => { + test("the default printer writes to stderr, never stdout", async () => { + const h = install({ headless: true }) + delete syncInternals.printLine + const err = spyOn(process.stderr, "write").mockImplementation(() => true) + const out = spyOn(process.stdout, "write").mockImplementation(() => true) + try { + await beforeTurn("s1") + expect(err).toHaveBeenCalledTimes(1) + expect(String(err.mock.calls[0]?.[0])).toContain(installCommand()) + expect(out).not.toHaveBeenCalled() + } finally { + err.mockRestore() + out.mockRestore() + } + expect(h.printed).toEqual([]) + }) +}) + +describe("install deadline", () => { + test("passes an abort signal to the spawn, not just a timeout", async () => { + syncInternals.which = () => "/usr/local/bin/datamate" + syncInternals.versionOf = async () => "0.7.0" + const run = spyOn(Process, "run").mockImplementation(async (_cmd, opts) => { + expect(opts?.abort).toBeInstanceOf(AbortSignal) + return { code: 0, stdout: Buffer.from(""), stderr: Buffer.from("") } as never + }) + try { + expect(await installEngine()).toEqual({ ok: true }) + expect(run).toHaveBeenCalledTimes(1) + } finally { + run.mockRestore() + } + }) +}) + +describe("install success is verified, not assumed", () => { + test("a zero exit with the engine still absent from PATH is a failure", async () => { + syncInternals.which = () => null + const run = spyOn(Process, "run").mockImplementation( + async () => ({ code: 0, stdout: Buffer.from(""), stderr: Buffer.from("") }) as never, + ) + try { + const result = await installEngine() + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error).toContain("not on PATH") + } finally { + run.mockRestore() + } + }) + test("a zero exit with a below-floor engine on PATH is a failure", async () => { + syncInternals.which = () => "/usr/local/bin/datamate" + syncInternals.versionOf = async () => "0.6.3" + const run = spyOn(Process, "run").mockImplementation( + async () => ({ code: 0, stdout: Buffer.from(""), stderr: Buffer.from("") }) as never, + ) + try { + const result = await installEngine() + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error).toContain("0.6.3") + } finally { + run.mockRestore() + } + }) + test("a non-zero exit reports npm's last lines", async () => { + const run = spyOn(Process, "run").mockImplementation( + async () => ({ code: 1, stdout: Buffer.from(""), stderr: Buffer.from("boom\nEACCES denied") }) as never, + ) + try { + expect(await installEngine()).toEqual({ ok: false, error: "boom EACCES denied" }) + } finally { + run.mockRestore() + } + }) +}) diff --git a/packages/opencode/test/altimate/workspace/engine-overlay.test.ts b/packages/opencode/test/altimate/workspace/engine-overlay.test.ts index 8556c67e0..0e91d1098 100644 --- a/packages/opencode/test/altimate/workspace/engine-overlay.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-overlay.test.ts @@ -112,6 +112,9 @@ function install(opts: { h.lines.push(line) } syncInternals.now = () => h.clock + // No TUI bus in this harness, so a refusal that would raise the install + // offer falls back to the toast, which is what these tests observe. + syncInternals.publishOffer = async () => false syncInternals.mcp = { status: async () => h.live ? { datamate: { status: h.status, ...(h.statusError ? { error: h.statusError } : {}) } } : {}, @@ -247,13 +250,13 @@ describe("overlay — what the config loader gets", () => { expect(h.probes).toBe(1) resetForTests() - const missing = install({ version: "0.6.3" }) - await overlay(DIR, missing.config) - await overlay(DIR, missing.config) - expect(missing.probes).toBe(1) - missing.clock += FAILED_PROBE_TTL_MS - await overlay(DIR, missing.config) - expect(missing.probes).toBe(2) + const old = install({ version: "0.6.3" }) + await overlay(DIR, old.config) + await overlay(DIR, old.config) + expect(old.probes).toBe(1) + old.clock += FAILED_PROBE_TTL_MS + await overlay(DIR, old.config) + expect(old.probes).toBe(2) }) test("a binding read that throws leaves the config as loaded", async () => { @@ -532,7 +535,7 @@ describe("beforeTurn — what a turn boundary does", () => { await beforeTurn("s1") expect(h.invalidates).toBe(1) expect(h.added).toHaveLength(1) - expect(pinnedWorkspace(h.added[0])).toBe("7") + expect(pinnedWorkspace(h.added[0] as LocalMcpConfig)).toBe("7") expect(pinnedWorkspace(h.config.mcp!.datamate as LocalMcpConfig)).toBe("7") expect(managedWorkspace()).toEqual({ id: "7", name: "growth" }) expect(settledOutcome("s1")?.kind).toBe("attached") @@ -720,20 +723,30 @@ describe("beforeTurn — what a turn boundary does", () => { expect(h.added).toEqual([]) }) - test("an engine installed after a refusal is picked up once the probe is asked again", async () => { + test("an engine installed after a refusal is picked up at the next turn boundary", async () => { + // The install dialog runs in another module realm and cannot reach the + // probe memo, so a missing engine is looked for on PATH every turn. const h = install({ which: null }) await beforeTurn("s1") expect(settledOutcome("s1")?.kind).toBe("engine-missing") h.which = "/usr/local/bin/datamate" - // Within the TTL the failed probe is not repeated... await beforeTurn("s1") - expect(settledOutcome("s1")?.kind).toBe("engine-missing") - expect(h.added).toEqual([]) - // ...the install offer invalidates it explicitly; a later turn re-probes on its own. + expect(h.added).toHaveLength(1) + expect(pinnedWorkspace(h.added[0] as LocalMcpConfig)).toBe("42") + expect(settledOutcome("s1")?.kind).toBe("attached") + }) + + test("a too-old engine is re-probed only after the TTL, or when the probe is invalidated", async () => { + const h = install({ version: "0.6.3" }) + await beforeTurn("s1") + expect(h.probes).toBe(1) + h.version = "0.7.0" + await beforeTurn("s1") + expect(h.probes).toBe(1) + expect(settledOutcome("s1")?.kind).toBe("engine-too-old") invalidateProbe() await beforeTurn("s1") - expect(h.added).toHaveLength(1) - expect(pinnedWorkspace(h.added[0])).toBe("42") + expect(h.probes).toBe(2) expect(settledOutcome("s1")?.kind).toBe("attached") }) From 7d7046bbd3b8850c50f31c2a9ea8ff783d3786b5 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 05:32:58 +0800 Subject: [PATCH 02/21] fix: let the install offer return in a session that outlives the Not-now latch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `announceRefusal` deduped the offer once per session per verdict, so a session open for longer than the 7-day "Not now" latch never saw the offer again — the TUI's latch check was unreachable. The offer route's dedupe now expires on the same TTL, `OFFER_SKIP_TTL_MS`, defined once in `engine-offer` and shared with the plugin's latch. Toast-only refusals keep "once per session". --- .../src/altimate/workspace/engine-offer.ts | 4 +++ .../src/altimate/workspace/engine-overlay.ts | 27 ++++++++++++++----- .../src/plugin/tui/altimate/workspace.tsx | 6 +++-- .../workspace/engine-install-offer.test.ts | 18 +++++++++++++ 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 955171898..159952fc6 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -28,6 +28,10 @@ export const INSTALL_TIMEOUT_MS = 300_000 * `CommandExecute` carries no payload, so the plugin re-derives the offer * with `describeOffer()`. */ export const OFFER_COMMAND = "altimate.workspace.engineInstallOffer" +/** How long "Not now" silences the offer for a workspace. The TUI latch and + * the per-session announce dedupe both key on this, so a session that + * outlives the latch sees the offer again instead of waiting for a new one. */ +export const OFFER_SKIP_TTL_MS = 7 * 24 * 60 * 60 * 1000 /** A "no usable engine" state, described well enough for an interactive * surface to act on it without re-deriving anything. */ diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index 15bc40d4e..1e70245ab 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -36,7 +36,7 @@ import { type ScopedBinding, } from "./engine-seams" import { declaredBounded, notify, printLine, resolveBinding, versionOf, which } from "./engine-probes" -import { installCommand, offerOrNotify, type EngineOffer } from "./engine-offer" +import { OFFER_SKIP_TTL_MS, installCommand, offerOrNotify, type EngineOffer } from "./engine-offer" import { ENGINE_BINARY, INSTALL_HELPS, @@ -285,14 +285,19 @@ export async function managedWorkspaceLoaded( /** `retried`: this session already spent its one re-add on a failed handshake. * Per session, so "start a new session to try again" is true. */ -type SessionRecord = { outcome: Outcome; announced?: string; retried?: boolean } +type SessionRecord = { outcome: Outcome; announced?: string; announcedAt?: number; retried?: boolean } const sessions = new Map() const declaredCache = new Map() function record(sessionID: string, outcome: Outcome): SessionRecord { const previous = sessions.get(sessionID) sessions.delete(sessionID) - const next: SessionRecord = { outcome, announced: previous?.announced, retried: previous?.retried } + const next: SessionRecord = { + outcome, + announced: previous?.announced, + announcedAt: previous?.announcedAt, + retried: previous?.retried, + } sessions.set(sessionID, next) while (sessions.size > MAX_TRACKED_SESSIONS) { const oldest = sessions.keys().next().value @@ -660,7 +665,11 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS * The substitution point for the install offer: when installing would help * and an `offer` is supplied, the offer surface (dialog, headless line, or * toast fallback) replaces the toast — never adds to it. Otherwise headless - * `run` prints one stderr line and the TUI gets the toast. */ + * `run` prints one stderr line and the TUI gets the toast. + * + * The offer route's "once" expires with the "Not now" latch: a session that + * stays open past `OFFER_SKIP_TTL_MS` is offered again, so the latch (which + * the TUI checks on every offer) decides, not the age of the session. */ export async function announceRefusal( sessionID: string, outcome: Outcome, @@ -671,9 +680,15 @@ export async function announceRefusal( const detail = "error" in outcome ? outcome.error : "found" in outcome ? String(outcome.found) : "" const declared = "declared" in outcome ? String(outcome.declared ?? "?") : "" const signature = `${outcome.kind}:${detail}:${declared}:${toast.title}` - if (rec.announced === signature) return + const offering = !!offer && INSTALL_HELPS[outcome.kind] + const at = now() + if (rec.announced === signature) { + const expired = offering && rec.announcedAt !== undefined && at - rec.announcedAt >= OFFER_SKIP_TTL_MS + if (!expired) return + } rec.announced = signature - if (offer && INSTALL_HELPS[outcome.kind]) { + rec.announcedAt = at + if (offering) { await offerOrNotify(offer, toast) return } diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 201f9b34e..969f905b2 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -57,6 +57,7 @@ import { npmAvailable, MIN_NODE_MAJOR, OFFER_COMMAND, + OFFER_SKIP_TTL_MS, type EngineOffer, } from "@/altimate/workspace/engine-offer" import { useClipboard } from "@opencode-ai/tui/context/clipboard" @@ -1176,7 +1177,8 @@ function engineSkipKey(workspaceId: string, scope: LatchScope | null): string { ) } -/** Same 7-day TTL and clock-rewind handling as the post-scan latch. */ +/** Same clock-rewind handling as the post-scan latch; the TTL is the one the + * attach side's announce dedupe expires on, so both agree on "7 days". */ function isEngineSkipActive( api: TuiPluginApi, workspaceId: string, @@ -1187,7 +1189,7 @@ function isEngineSkipActive( if (!rec || typeof rec.skippedAt !== "number") return false const delta = nowMs - rec.skippedAt if (delta < 0) return false - return delta < SKIP_TTL_MS + return delta < OFFER_SKIP_TTL_MS } function recordEngineSkip( diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 33e2e3fb2..2328c15bb 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -20,6 +20,7 @@ import { type EngineOffer, type Toast, } from "../../../src/altimate/workspace/engine-overlay" +import { OFFER_SKIP_TTL_MS } from "../../../src/altimate/workspace/engine-offer" import { Process } from "../../../src/util/process" import type { CachedBinding } from "../../../src/altimate/workspace/state" @@ -185,6 +186,23 @@ describe("offer routing — engine missing", () => { await beforeTurn("s2") expect(h.published).toBe(2) }) + test("a session that outlives the Not-now latch is offered again", async () => { + // The TUI re-checks its 7-day latch on every offer; the dedupe here must + // not outlast that latch, or a long-lived session never sees the offer + // return after "Not now" expires. + let clock = 1_000_000 + syncInternals.now = () => clock + const h = install({}) + await beforeTurn("s1") + clock += OFFER_SKIP_TTL_MS - 1 + await beforeTurn("s1") + expect(h.published).toBe(1) + clock += 1 + await beforeTurn("s1") + expect(h.published).toBe(2) + await beforeTurn("s1") + expect(h.published).toBe(2) + }) }) describe("offer routing — engine too old", () => { From 4328acd516e235fa78aec4014397dd1ea7fef00d Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 05:45:45 +0800 Subject: [PATCH 03/21] fix: re-probe a too-old engine as soon as the file on PATH changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The too-old probe memo was honoured for 30s by path alone, so an engine updated in place by the offer's `npm i -g` stayed refused on the next message — and nothing could invalidate the memo, since the offer runs in another module realm. The memo now also carries the binary's fingerprint (size + mtime, symlinks followed) and ends when it changes; an un-stat-able binary falls back to the TTL. --- .../src/altimate/workspace/engine-overlay.ts | 28 +++++++++++----- .../src/altimate/workspace/engine-probes.ts | 15 +++++++++ .../src/altimate/workspace/engine-seams.ts | 1 + .../altimate/workspace/engine-overlay.test.ts | 33 +++++++++++++++++++ 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index 1e70245ab..f535e2e0e 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -35,7 +35,7 @@ import { syncInternals, type ScopedBinding, } from "./engine-seams" -import { declaredBounded, notify, printLine, resolveBinding, versionOf, which } from "./engine-probes" +import { declaredBounded, fingerprint, notify, printLine, resolveBinding, versionOf, which } from "./engine-probes" import { OFFER_SKIP_TTL_MS, installCommand, offerOrNotify, type EngineOffer } from "./engine-offer" import { ENGINE_BINARY, @@ -72,7 +72,7 @@ const DECLARED_RETRY_MS = 60_000 type Probe = { kind: "ok"; version: string } | { kind: "missing" } | { kind: "too-old"; found: string | null } -let probeMemo: { result: Probe; at: number } | null = null +let probeMemo: { result: Probe; at: number; fingerprint: string | null } | null = null function now(): number { return syncInternals.now ? syncInternals.now() : Date.now() @@ -85,9 +85,19 @@ async function probeEngine(): Promise { // about on every call — `which` is a PATH scan, no process spawn — so an // install made from the offer dialog (which runs in another module realm and // cannot reach this memo) is seen on the next turn. A too-old or broken one - // costs a spawn to re-check, so that is rate-limited by the TTL. + // costs a spawn to re-check, so that is rate-limited by the TTL — but only + // while the file on PATH is the same one: an update written over it (the + // offer's `npm i -g` on an old engine) changes the fingerprint and is + // re-probed on the next turn, just as an install is. + const seen = bin ? fingerprint(bin) : null if (probeMemo && probeMemo.result.kind === "ok") return probeMemo.result - if (probeMemo && probeMemo.result.kind === "too-old" && bin && at - probeMemo.at < FAILED_PROBE_TTL_MS) { + if ( + probeMemo && + probeMemo.result.kind === "too-old" && + bin && + probeMemo.fingerprint === seen && + at - probeMemo.at < FAILED_PROBE_TTL_MS + ) { return probeMemo.result } let result: Probe @@ -97,12 +107,14 @@ async function probeEngine(): Promise { const version = await versionOf(bin) result = clearsFloor(version) ? { kind: "ok", version: version! } : { kind: "too-old", found: version } } - probeMemo = { result, at } + probeMemo = { result, at, fingerprint: seen } return result } /** Forget the last probe, so the next turn boundary looks for the engine - * again immediately. The install offer calls this after an install. */ + * again immediately. Nothing in production calls this — the offer dialog runs + * in another module realm — which is why the probe itself notices an engine + * that appeared or changed on PATH. Kept for tests and diagnostics. */ export function invalidateProbe(): void { probeMemo = null } @@ -700,8 +712,8 @@ export async function announceRefusal( } /** Is a re-probe worth asking for on the next turn? Exposed for the install - * offer, which schedules nothing itself: it installs, invalidates the probe, - * and the next turn boundary attaches. */ + * offer, which schedules nothing itself: it installs, and the next turn + * boundary sees the new or changed binary on PATH and attaches. */ export function isRepairable(outcome: Outcome | undefined): boolean { return !!outcome && REPAIRABLE[outcome.kind] } diff --git a/packages/opencode/src/altimate/workspace/engine-probes.ts b/packages/opencode/src/altimate/workspace/engine-probes.ts index 55861d7a1..878337bd0 100644 --- a/packages/opencode/src/altimate/workspace/engine-probes.ts +++ b/packages/opencode/src/altimate/workspace/engine-probes.ts @@ -2,6 +2,7 @@ // // Everything that asks the outside world a question: the binary, its // version, the workspace allowlist, and the user-facing surfaces. +import { statSync } from "fs" import launch from "cross-spawn" import { which as whichBinary } from "@opencode-ai/core/util/which" import { AltimateApi } from "@/altimate/api/client" @@ -45,6 +46,20 @@ export function which(cmd: string): string | null { return syncInternals.which ? syncInternals.which(cmd) : whichBinary(cmd) } +/** Identity of the file behind a PATH hit, cheap enough to ask every turn: + * size and mtime of the target (symlinks followed, so an npm bin shim whose + * package was reinstalled reads as changed). Null when it cannot be stat'ed; + * with nothing to compare, the caller's memo falls back to its TTL. */ +export function fingerprint(bin: string): string | null { + if (syncInternals.fingerprint) return syncInternals.fingerprint(bin) + try { + const stat = statSync(bin) + return `${stat.size}:${stat.mtimeMs}` + } catch { + return null + } +} + /** `datamate --version`, stdout only. The engine prints its real package * version here; its MCP `serverInfo` was a hard-coded placeholder on the very * engines the floor excludes, so the handshake cannot be asked instead. diff --git a/packages/opencode/src/altimate/workspace/engine-seams.ts b/packages/opencode/src/altimate/workspace/engine-seams.ts index a823e25a5..ce6304e1f 100644 --- a/packages/opencode/src/altimate/workspace/engine-seams.ts +++ b/packages/opencode/src/altimate/workspace/engine-seams.ts @@ -25,6 +25,7 @@ export const syncInternals: { resolveBinding?: (directory: string) => Promise which?: (cmd: string) => string | null versionOf?: (bin: string) => Promise + fingerprint?: (bin: string) => string | null declared?: (workspaceId: string) => Promise notify?: (toast: Toast) => Promise printLine?: (line: string) => void diff --git a/packages/opencode/test/altimate/workspace/engine-overlay.test.ts b/packages/opencode/test/altimate/workspace/engine-overlay.test.ts index 0e91d1098..a52fdcc86 100644 --- a/packages/opencode/test/altimate/workspace/engine-overlay.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-overlay.test.ts @@ -55,6 +55,7 @@ type Harness = { /** Whether MCP holds a client under the key — set when MCP "bootstraps" from * the first config load, then tracked through add/remove, as in the runtime. */ live?: boolean + fingerprint: string | null } function install(opts: { @@ -90,6 +91,7 @@ function install(opts: { toasts: [], lines: [], clock: 1_000_000, + fingerprint: "bin-1", } process.env.ALTIMATE_WORKSPACE = opts.flag === false ? "" : "1" syncInternals.serve = () => opts.serve === true @@ -112,6 +114,7 @@ function install(opts: { h.lines.push(line) } syncInternals.now = () => h.clock + syncInternals.fingerprint = () => h.fingerprint // No TUI bus in this harness, so a refusal that would raise the install // offer falls back to the toast, which is what these tests observe. syncInternals.publishOffer = async () => false @@ -750,6 +753,36 @@ describe("beforeTurn — what a turn boundary does", () => { expect(settledOutcome("s1")?.kind).toBe("attached") }) + test("a too-old engine updated in place is re-probed on the next turn, inside the TTL", async () => { + // The offer's install writes the new engine over the old one at the same + // PATH entry, from another module realm that cannot invalidate this memo. + // The file's fingerprint changing is what ends the memo, not the clock. + const h = install({ version: "0.6.3" }) + await beforeTurn("s1") + expect(h.probes).toBe(1) + expect(settledOutcome("s1")?.kind).toBe("engine-too-old") + h.version = "0.7.0" + h.fingerprint = "bin-2" + h.clock += 1_000 + await beforeTurn("s1") + expect(h.probes).toBe(2) + expect(settledOutcome("s1")?.kind).toBe("attached") + // A binary that cannot be stat'ed has no fingerprint to compare, so the + // memo falls back to its TTL alone — never a spawn on every turn. + resetForTests() + const u = install({ version: "0.6.3" }) + u.fingerprint = null + await beforeTurn("s1") + u.version = "0.7.0" + u.clock += 1_000 + await beforeTurn("s1") + expect(u.probes).toBe(1) + u.clock += FAILED_PROBE_TTL_MS + await beforeTurn("s1") + expect(u.probes).toBe(2) + expect(settledOutcome("s1")?.kind).toBe("attached") + }) + test("a failed probe is repeated on its own after the TTL", async () => { const h = install({ version: "0.6.3" }) await beforeTurn("s1") From f3e1764c758d58db55cc8d4da578251a114ae07d Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 06:06:31 +0800 Subject: [PATCH 04/21] fix: re-raise the offer hourly past the latch window; scope the attached-run line to its session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The attach-side dedupe expired seven days after the offer was raised, but the TUI latch runs from "Not now", which can come later — a long-lived session could wait a second full window. Past the window the offer is now re-raised every `OFFER_RECHECK_MS` and the TUI suppresses it until its latch ends. The offer command now carries the session it was raised for, so an attached headless run prints only its own offer, not another session's in the directory. --- .../src/altimate/workspace/engine-offer.ts | 21 ++++++++++++----- .../src/altimate/workspace/engine-overlay.ts | 14 +++++++---- .../src/altimate/workspace/engine-seams.ts | 2 +- packages/opencode/src/cli/cmd/run.ts | 5 +++- packages/opencode/src/server/tui-event.ts | 6 +++++ .../workspace/engine-install-offer.test.ts | 23 +++++++++++++++---- 6 files changed, 54 insertions(+), 17 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 159952fc6..500a75691 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -32,6 +32,11 @@ export const OFFER_COMMAND = "altimate.workspace.engineInstallOffer" * the per-session announce dedupe both key on this, so a session that * outlives the latch sees the offer again instead of waiting for a new one. */ export const OFFER_SKIP_TTL_MS = 7 * 24 * 60 * 60 * 1000 +/** Once a session's offer is older than the latch, how often it is raised + * again while the verdict stands. The TUI's latch starts when "Not now" is + * chosen, not when the offer was raised, so the attach side cannot know when + * it ends: it re-raises at this cadence and the TUI suppresses until then. */ +export const OFFER_RECHECK_MS = 60 * 60 * 1000 /** A "no usable engine" state, described well enough for an interactive * surface to act on it without re-deriving anything. */ @@ -156,12 +161,16 @@ export async function installEngine(): Promise { } } -/** Ask the TUI to raise the offer. False when the bus is unavailable. */ -async function publishOffer(): Promise { - if (syncInternals.publishOffer) return syncInternals.publishOffer() +/** Ask the TUI to raise the offer. False when the bus is unavailable. The + * session is carried so an attached headless run, which reads the same event + * stream, prints the offer raised for its own session only. */ +async function publishOffer(sessionID: string): Promise { + if (syncInternals.publishOffer) return syncInternals.publishOffer(sessionID) try { await AppRuntime.runPromise( - EventV2Bridge.Service.use((events) => events.publish(TuiEvent.CommandExecute, { command: OFFER_COMMAND })), + EventV2Bridge.Service.use((events) => + events.publish(TuiEvent.CommandExecute, { command: OFFER_COMMAND, sessionID }), + ), ) return true } catch (err) { @@ -192,12 +201,12 @@ export function describeOfferLine(offer: EngineOffer): string { /** Offer via the dialog surface when there is one; otherwise print (headless) * or toast (bus unavailable). Exactly one of these happens. */ -export async function offerOrNotify(offer: EngineOffer, toast: Toast): Promise { +export async function offerOrNotify(offer: EngineOffer, toast: Toast, sessionID: string): Promise { if (isHeadless()) { printLine(describeOfferLine(offer)) return } if (offerInstall(offer)) return - if (await publishOffer()) return + if (await publishOffer(sessionID)) return await notify(toast) } diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index f535e2e0e..e0d84d11b 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -36,7 +36,7 @@ import { type ScopedBinding, } from "./engine-seams" import { declaredBounded, fingerprint, notify, printLine, resolveBinding, versionOf, which } from "./engine-probes" -import { OFFER_SKIP_TTL_MS, installCommand, offerOrNotify, type EngineOffer } from "./engine-offer" +import { OFFER_RECHECK_MS, OFFER_SKIP_TTL_MS, installCommand, offerOrNotify, type EngineOffer } from "./engine-offer" import { ENGINE_BINARY, INSTALL_HELPS, @@ -681,7 +681,11 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS * * The offer route's "once" expires with the "Not now" latch: a session that * stays open past `OFFER_SKIP_TTL_MS` is offered again, so the latch (which - * the TUI checks on every offer) decides, not the age of the session. */ + * the TUI checks on every offer) decides, not the age of the session. The + * latch is measured from the user's "Not now", which can come well after the + * offer was raised, so after the first expiry the offer is re-raised every + * `OFFER_RECHECK_MS` rather than once per further window — the TUI keeps + * suppressing it until its latch really ends. */ export async function announceRefusal( sessionID: string, outcome: Outcome, @@ -694,14 +698,16 @@ export async function announceRefusal( const signature = `${outcome.kind}:${detail}:${declared}:${toast.title}` const offering = !!offer && INSTALL_HELPS[outcome.kind] const at = now() + let repeat = false if (rec.announced === signature) { const expired = offering && rec.announcedAt !== undefined && at - rec.announcedAt >= OFFER_SKIP_TTL_MS if (!expired) return + repeat = true } rec.announced = signature - rec.announcedAt = at + rec.announcedAt = repeat ? at - OFFER_SKIP_TTL_MS + OFFER_RECHECK_MS : at if (offering) { - await offerOrNotify(offer, toast) + await offerOrNotify(offer, toast, sessionID) return } if (isHeadless()) { diff --git a/packages/opencode/src/altimate/workspace/engine-seams.ts b/packages/opencode/src/altimate/workspace/engine-seams.ts index ce6304e1f..92f1e5c28 100644 --- a/packages/opencode/src/altimate/workspace/engine-seams.ts +++ b/packages/opencode/src/altimate/workspace/engine-seams.ts @@ -31,7 +31,7 @@ export const syncInternals: { printLine?: (line: string) => void /** Install-offer seams (see engine-offer.ts). */ offer?: (offer: EngineOffer) => boolean - publishOffer?: () => Promise + publishOffer?: (sessionID: string) => Promise nodeMajor?: () => Promise npmAvailable?: () => boolean install?: (spec: string) => Promise diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index b3015d8a1..28f447c78 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -935,9 +935,12 @@ You are speaking to a non-technical business executive. Follow these rules stric // which has no handler for it, is the only thing the user is looking at. // Placed before the idle break: the loop stops on idle, so a handler // after it never runs for an offer that arrives in the same batch. + // The stream carries every session's events for this directory; only + // the offer raised for this run's session is this run's to print. if ( event.type === "tui.command.execute" && - (event.properties as { command?: string }).command === OFFER_COMMAND + (event.properties as { command?: string }).command === OFFER_COMMAND && + (event.properties as { sessionID?: string }).sessionID === sessionID ) { // stderr: stdout is raw JSON events under --format json. process.stderr.write( diff --git a/packages/opencode/src/server/tui-event.ts b/packages/opencode/src/server/tui-event.ts index 73412b877..93d3f7e6d 100644 --- a/packages/opencode/src/server/tui-event.ts +++ b/packages/opencode/src/server/tui-event.ts @@ -31,6 +31,12 @@ export const TuiEvent = { ]), Schema.String, ]), + // altimate_change start — the workspace engine install offer is published + // as a command for the TUI plugin, and an attached headless run reads the + // same stream: the session it was raised for lets that run print only its + // own offer, not another session's in the same directory. + sessionID: Schema.optional(Schema.String), + // altimate_change end }, }), ToastShow: EventV2.define({ diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 2328c15bb..439d9ea56 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -20,7 +20,7 @@ import { type EngineOffer, type Toast, } from "../../../src/altimate/workspace/engine-overlay" -import { OFFER_SKIP_TTL_MS } from "../../../src/altimate/workspace/engine-offer" +import { OFFER_RECHECK_MS, OFFER_SKIP_TTL_MS } from "../../../src/altimate/workspace/engine-offer" import { Process } from "../../../src/util/process" import type { CachedBinding } from "../../../src/altimate/workspace/state" @@ -36,7 +36,7 @@ const binding: CachedBinding = { linkedAt: 0, } as CachedBinding -type Harness = { offers: EngineOffer[]; toasts: Toast[]; printed: string[]; published: number } +type Harness = { offers: EngineOffer[]; toasts: Toast[]; printed: string[]; published: number; publishedFor: string[] } /** No engine on PATH (or an old one) plus captured surfaces. */ function install(opts: { @@ -48,7 +48,7 @@ function install(opts: { surface?: boolean bound?: boolean }): Harness { - const h: Harness = { offers: [], toasts: [], printed: [], published: 0 } + const h: Harness = { offers: [], toasts: [], printed: [], published: 0, publishedFor: [] } process.env.ALTIMATE_WORKSPACE = "1" syncInternals.serve = () => false syncInternals.headless = () => opts.headless === true @@ -66,9 +66,10 @@ function install(opts: { syncInternals.printLine = (line) => { h.printed.push(line) } - syncInternals.publishOffer = async () => { + syncInternals.publishOffer = async (sessionID) => { if (opts.bus === false) return false h.published += 1 + h.publishedFor.push(sessionID) return true } if (opts.surface) { @@ -178,13 +179,16 @@ describe("offer routing — engine missing", () => { await beforeTurn("s1") expect(h.printed[0]).toContain("1 integration tool need") }) - test("the offer is raised once per session per verdict", async () => { + test("the offer is raised once per session per verdict, naming the session it is for", async () => { const h = install({}) await beforeTurn("s1") await beforeTurn("s1") expect(h.published).toBe(1) await beforeTurn("s2") expect(h.published).toBe(2) + // An attached headless run reads every session's events for the directory + // and prints only the offer raised for its own session. + expect(h.publishedFor).toEqual(["s1", "s2"]) }) test("a session that outlives the Not-now latch is offered again", async () => { // The TUI re-checks its 7-day latch on every offer; the dedupe here must @@ -202,6 +206,15 @@ describe("offer routing — engine missing", () => { expect(h.published).toBe(2) await beforeTurn("s1") expect(h.published).toBe(2) + // The latch runs from "Not now", which may come long after the offer was + // raised, so once the window has passed the offer is re-raised hourly — + // never held for another full window. + clock += OFFER_RECHECK_MS - 1 + await beforeTurn("s1") + expect(h.published).toBe(2) + clock += 1 + await beforeTurn("s1") + expect(h.published).toBe(3) }) }) From c6acbcbb083986755b68545000ebb6535f96cafe Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 06:25:51 +0800 Subject: [PATCH 05/21] fix: print the headless refusal line once per process, not once per session A headless `run` whose agent uses the task tool creates a child session, and that session's first catalog settled the same verdict and printed the same stderr line again. The headless line is now deduplicated per verdict across the process; sessions still track their own announcement for the toast and offer surfaces. --- .../opencode/src/altimate/workspace/engine-overlay.ts | 10 ++++++++++ .../altimate/workspace/engine-install-offer.test.ts | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index e0d84d11b..8b8395f93 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -300,6 +300,8 @@ export async function managedWorkspaceLoaded( type SessionRecord = { outcome: Outcome; announced?: string; announcedAt?: number; retried?: boolean } const sessions = new Map() const declaredCache = new Map() +/** Verdict signatures a headless process has already printed to stderr. */ +const headlessPrinted = new Set() function record(sessionID: string, outcome: Outcome): SessionRecord { const previous = sessions.get(sessionID) @@ -706,6 +708,13 @@ export async function announceRefusal( } rec.announced = signature rec.announcedAt = repeat ? at - OFFER_SKIP_TTL_MS + OFFER_RECHECK_MS : at + if (isHeadless()) { + // A headless `run` is one process with one stderr, whatever sessions it + // creates along the way (a sub-agent's session settles the same verdict + // and would print the same line). One line per verdict per process. + if (headlessPrinted.has(signature)) return + headlessPrinted.add(signature) + } if (offering) { await offerOrNotify(offer, toast, sessionID) return @@ -731,6 +740,7 @@ export function resetForTests(): void { sessions.clear() turnTools.clear() declaredCache.clear() + headlessPrinted.clear() } /** Test-only views. */ diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 439d9ea56..1797faaa2 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -241,6 +241,15 @@ describe("offer routing — engine too old", () => { `Workspace "analytics": 2 integration tools need ${ENGINE_BINARY} ${MIN_ENGINE_VERSION}+ (found 0.6.3). Update with: ${installCommand()}`, ]) }) + test("headless, a sub-agent's session in the same process prints nothing more", async () => { + // One `run` is one process with one stderr: the task tool's child session + // settles the same verdict and must not repeat the line. + const h = install({ headless: true, which: "/usr/local/bin/datamate", version: "0.6.3" }) + await beforeTurn("parent") + await beforeTurn("child") + await beforeTurn("parent") + expect(h.printed).toHaveLength(1) + }) test("a broken engine reports 'unknown' rather than a version", async () => { const h = install({ surface: true, which: "/usr/local/bin/datamate", version: null }) await beforeTurn("s1") From e0cd5c9237cf59c92ae264f59f7b4234eee48f70 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 06:34:46 +0800 Subject: [PATCH 06/21] fix: wait for the TUI kv store to hydrate before consulting the Not-now latch The store is empty until kv.json has been read, so an offer raised on the first message after a restart read a persisted "Not now" as absent and showed the dialog inside the seven days. `api.kv.ready` is a plain getter, so the offer polls it (25 ms, bounded at 3 s) and on timeout proceeds as before. --- .../src/plugin/tui/altimate/workspace.tsx | 26 +++++++++++++- .../test/altimate/plugin/workspace.test.ts | 36 ++++++++++++++++++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 969f905b2..40214bfd1 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1177,6 +1177,27 @@ function engineSkipKey(workspaceId: string, scope: LatchScope | null): string { ) } +/** The KV store starts empty and fills in once the persisted file is read + * (`api.kv.ready`). A latch checked before that reads as absent, so an offer + * raised on the first message after a restart would ignore a "Not now" that + * is still in force. `ready` is a plain getter with nothing to await, so poll + * it — bounded, and on timeout proceed as if hydrated rather than never + * answer. Resolves to whether the store was ready. */ +const KV_READY_TIMEOUT_MS = 3_000 +const KV_READY_POLL_MS = 25 +async function awaitKvReady( + kv: { readonly ready: boolean }, + timeoutMs = KV_READY_TIMEOUT_MS, + pollMs = KV_READY_POLL_MS, +): Promise { + const deadline = Date.now() + timeoutMs + while (!kv.ready) { + if (Date.now() >= deadline) return false + await new Promise((resolve) => setTimeout(resolve, pollMs)) + } + return true +} + /** Same clock-rewind handling as the post-scan latch; the TTL is the one the * attach side's announce dedupe expires on, so both agree on "7 days". */ function isEngineSkipActive( @@ -1486,6 +1507,9 @@ async function showEngineInstallOffer(api: TuiPluginApi): Promise { // engine appeared, or the project is no longer bound. Say nothing. if (!offer) return release() const latchScope = await currentLatchScope() + if (!(await awaitKvReady(api.kv))) { + log.warn("kv store not hydrated in time; checking the engine install latch against what is loaded") + } if (isEngineSkipActive(api, offer.workspaceId, latchScope, Date.now())) { log.info("engine install offer suppressed by 7-day latch", { workspaceId: offer.workspaceId }) return release() @@ -1571,5 +1595,5 @@ 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, isEngineSkipActive, recordEngineSkip } +export { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip, awaitKvReady } // altimate_change end diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index 08716c177..0be8207bd 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -28,7 +28,7 @@ afterAll(() => { } }) -const { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip } = await import( +const { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip, awaitKvReady } = await import( "../../../src/plugin/tui/altimate/workspace" ) const { projectNameFromRemote, detectProjectRemote } = await import( @@ -603,3 +603,37 @@ describe("Engine install-offer latch", () => { expect(isEngineSkipActive(api, workspaceId, scope, now + DAY)).toBe(false) }) }) + +describe("engine install offer — kv hydration", () => { + // The store starts empty until kv.json has been read; a "Not now" latch + // checked before that reads as absent. The offer waits for `ready`. + test("waits for the store to hydrate before the latch is consulted", async () => { + let ready = false + setTimeout(() => { + ready = true + }, 60) + const t0 = Date.now() + expect( + await awaitKvReady( + { + get ready() { + return ready + }, + }, + 1_000, + 5, + ), + ).toBe(true) + expect(Date.now() - t0).toBeGreaterThanOrEqual(50) + }) + test("returns at once when the store is already hydrated", async () => { + const t0 = Date.now() + expect(await awaitKvReady({ ready: true }, 1_000, 5)).toBe(true) + expect(Date.now() - t0).toBeLessThan(50) + }) + test("gives up after the timeout so a stuck read never blocks the offer", async () => { + const t0 = Date.now() + expect(await awaitKvReady({ ready: false }, 40, 5)).toBe(false) + expect(Date.now() - t0).toBeGreaterThanOrEqual(35) + }) +}) From cb056bfa93bb4fcdcc5f999b9fa4f4f3a1dfc26b Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 06:47:22 +0800 Subject: [PATCH 07/21] fix: give the engine install a real deadline over npm's whole process tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Process.run` settles on `close`, and the abort only signalled npm's own pid, so a descendant that kept the stderr pipe open held `installEngine` past the five minutes and the dialog stayed on "Installing…". The install now runs in its own process group, settles on the child's `exit`, and at the deadline signals the group — SIGTERM, then SIGKILL after a grace — before reporting the timeout. --- .../src/altimate/workspace/engine-offer.ts | 140 +++++++++++++----- .../src/altimate/workspace/engine-seams.ts | 4 + .../workspace/engine-install-offer.test.ts | 73 ++++----- 3 files changed, 140 insertions(+), 77 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 500a75691..8c4cc698b 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -7,7 +7,8 @@ // the detail here from disk and PATH rather than from the overlay's memory. // Offer, never install on the flow's own account — `installEngine` only ever // runs from an explicit "Install now". -import { execFile } from "node:child_process" +import { execFile, type ChildProcess } from "node:child_process" +import launch from "cross-spawn" import { Process } from "@/util/process" import { AppRuntime } from "@/effect/app-runtime" import { EventV2Bridge } from "@/event-v2-bridge" @@ -117,48 +118,117 @@ export function npmAvailable(): boolean { return which(process.platform === "win32" ? "npm.cmd" : "npm") !== null } -/** Run the global install. Only ever reached from an explicit user choice. - * - * `npm.cmd` on Windows: a normal Node install exposes npm as a command shim, - * and nothing here spawns a shell. The deadline comes from an abort signal: - * `Process.spawn` consults `timeout` only inside its abort handler. A zero - * exit is not a usable engine — npm's global bin directory need not be on - * PATH — so the same discovery the turn boundary does runs before success. */ +/** Options are the process-group and deadline handling for the one command + * the offer runs. Nothing here spawns a shell. */ +export type InstallRun = { code: number | null; timedOut: boolean; stderr: string } +/** After the deadline's SIGTERM, how long the tree gets before SIGKILL and the + * run is reported as timed out regardless of what is still alive. */ +export const INSTALL_KILL_GRACE_MS = 5_000 + +/** Run the install command with a real deadline. npm forks a tree (scripts, + * node), and a descendant that outlives npm can keep the stderr pipe open, so + * the run settles on the child's `exit`, never on `close`, and the deadline + * signals the whole process group (POSIX: the child is its own group leader; + * Windows: taskkill /T) — SIGTERM first, SIGKILL after the grace, then the run + * reports the timeout whether or not anything is still holding a pipe. */ +export function runInstall( + argv: string[], + timeoutMs = INSTALL_TIMEOUT_MS, + graceMs = INSTALL_KILL_GRACE_MS, +): Promise { + if (syncInternals.runInstall) return syncInternals.runInstall(argv, timeoutMs) + return new Promise((resolve) => { + const grouped = process.platform !== "win32" + let child: ChildProcess + try { + child = launch(argv[0], argv.slice(1), { + stdio: ["ignore", "ignore", "pipe"], + detached: grouped, + windowsHide: process.platform === "win32", + }) + } catch (err) { + resolve({ code: null, timedOut: false, stderr: err instanceof Error ? err.message : String(err) }) + return + } + let stderr = "" + child.stderr?.on("data", (chunk) => { + stderr = (stderr + String(chunk)).slice(-4096) + }) + let timedOut = false + let settled = false + let hard: ReturnType | undefined + const finish = (code: number | null) => { + if (settled) return + settled = true + clearTimeout(timer) + if (hard) clearTimeout(hard) + resolve({ code, timedOut, stderr }) + } + const killTree = (signal: NodeJS.Signals) => { + if (grouped && child.pid) { + try { + process.kill(-child.pid, signal) + return + } catch { + // The group is already gone; fall through to the child itself. + } + } + if (process.platform === "win32") { + void Process.stop(child) + return + } + try { + child.kill(signal) + } catch { + // Already exited. + } + } + const timer = setTimeout(() => { + timedOut = true + killTree("SIGTERM") + hard = setTimeout(() => { + killTree("SIGKILL") + finish(null) + }, graceMs) + }, timeoutMs) + child.once("exit", (code) => finish(code)) + child.once("error", (err) => { + stderr = stderr || err.message + finish(null) + }) + }) +} + +/** `npm i -g ` with a deadline (`runInstall`). A zero exit is not a + * usable engine — npm's global bin directory need not be on PATH — so the same + * discovery the turn boundary does runs before success. */ export async function installEngine(): Promise { const spec = installSpec() if (syncInternals.install) return syncInternals.install(spec) const npm = process.platform === "win32" ? "npm.cmd" : "npm" - const deadline = AbortSignal.timeout(INSTALL_TIMEOUT_MS) - try { - const result = await Process.run([npm, "i", "-g", spec], { abort: deadline, nothrow: true }) - if (result.code === 0) { - const installedBin = which(ENGINE_BINARY) - if (!installedBin) { - return { - ok: false, - error: `npm installed it, but ${ENGINE_BINARY} is not on PATH — add your npm global bin directory to PATH`, - } - } - const installedVersion = await versionOf(installedBin) - if (!clearsFloor(installedVersion)) { - return { - ok: false, - error: `npm installed it, but ${ENGINE_BINARY} on PATH reports ${installedVersion ?? "no version"}`, - } + const run = await runInstall([npm, "i", "-g", spec]) + if (run.timedOut) { + return { ok: false, error: `npm did not finish within ${Math.round(INSTALL_TIMEOUT_MS / 60_000)} minutes` } + } + if (run.code === 0) { + const installedBin = which(ENGINE_BINARY) + if (!installedBin) { + return { + ok: false, + error: `npm installed it, but ${ENGINE_BINARY} is not on PATH — add your npm global bin directory to PATH`, } - return { ok: true } - } - if (deadline.aborted) { - return { ok: false, error: `npm did not finish within ${Math.round(INSTALL_TIMEOUT_MS / 60_000)} minutes` } } - const detail = result.stderr.toString().trim().split(/\r?\n/).slice(-3).join(" ") - return { ok: false, error: detail || `npm exited with code ${result.code}` } - } catch (err) { - if (deadline.aborted) { - return { ok: false, error: `npm did not finish within ${Math.round(INSTALL_TIMEOUT_MS / 60_000)} minutes` } + const installedVersion = await versionOf(installedBin) + if (!clearsFloor(installedVersion)) { + return { + ok: false, + error: `npm installed it, but ${ENGINE_BINARY} on PATH reports ${installedVersion ?? "no version"}`, + } } - return { ok: false, error: err instanceof Error ? err.message : String(err) } + return { ok: true } } + const detail = run.stderr.trim().split(/\r?\n/).slice(-3).join(" ") + return { ok: false, error: detail || `npm exited with code ${run.code ?? "unknown"}` } } /** Ask the TUI to raise the offer. False when the bus is unavailable. The diff --git a/packages/opencode/src/altimate/workspace/engine-seams.ts b/packages/opencode/src/altimate/workspace/engine-seams.ts index 92f1e5c28..93f69f35c 100644 --- a/packages/opencode/src/altimate/workspace/engine-seams.ts +++ b/packages/opencode/src/altimate/workspace/engine-seams.ts @@ -32,6 +32,10 @@ export const syncInternals: { /** Install-offer seams (see engine-offer.ts). */ offer?: (offer: EngineOffer) => boolean publishOffer?: (sessionID: string) => Promise + runInstall?: ( + argv: string[], + timeoutMs: number, + ) => Promise<{ code: number | null; timedOut: boolean; stderr: string }> nodeMajor?: () => Promise npmAvailable?: () => boolean install?: (spec: string) => Promise diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 1797faaa2..cdf14d0d2 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -12,6 +12,7 @@ import { describeOffer, installCommand, installEngine, + runInstall, installSpec, nodeMajor, resetForTests, @@ -21,7 +22,6 @@ import { type Toast, } from "../../../src/altimate/workspace/engine-overlay" import { OFFER_RECHECK_MS, OFFER_SKIP_TTL_MS } from "../../../src/altimate/workspace/engine-offer" -import { Process } from "../../../src/util/process" import type { CachedBinding } from "../../../src/altimate/workspace/state" const DIR = "/tmp/analytics" @@ -313,58 +313,47 @@ describe("headless notice stream", () => { }) describe("install deadline", () => { - test("passes an abort signal to the spawn, not just a timeout", async () => { - syncInternals.which = () => "/usr/local/bin/datamate" - syncInternals.versionOf = async () => "0.7.0" - const run = spyOn(Process, "run").mockImplementation(async (_cmd, opts) => { - expect(opts?.abort).toBeInstanceOf(AbortSignal) - return { code: 0, stdout: Buffer.from(""), stderr: Buffer.from("") } as never - }) - try { - expect(await installEngine()).toEqual({ ok: true }) - expect(run).toHaveBeenCalledTimes(1) - } finally { - run.mockRestore() - } + const posix = process.platform !== "win32" + test.skipIf(!posix)("settles on the child's exit even when a descendant keeps stderr open", async () => { + // npm forks a tree; a straggler holding the pipe must not hold the run. + const t0 = Date.now() + const run = await runInstall(["sh", "-c", "sleep 5 >&2 2>/dev/null & exit 0"], 4_000, 200) + expect(run.code).toBe(0) + expect(run.timedOut).toBe(false) + expect(Date.now() - t0).toBeLessThan(2_000) + }) + test.skipIf(!posix)("the deadline terminates a tree that ignores SIGTERM and reports the timeout", async () => { + const t0 = Date.now() + const run = await runInstall(["sh", "-c", "trap '' TERM; sleep 30"], 200, 200) + expect(run.timedOut).toBe(true) + expect(Date.now() - t0).toBeLessThan(3_000) + }) + test("a timed-out run is reported as such, not as an npm failure", async () => { + syncInternals.runInstall = async () => ({ code: null, timedOut: true, stderr: "" }) + const result = await installEngine() + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error).toContain("did not finish within") }) }) describe("install success is verified, not assumed", () => { test("a zero exit with the engine still absent from PATH is a failure", async () => { syncInternals.which = () => null - const run = spyOn(Process, "run").mockImplementation( - async () => ({ code: 0, stdout: Buffer.from(""), stderr: Buffer.from("") }) as never, - ) - try { - const result = await installEngine() - expect(result.ok).toBe(false) - if (!result.ok) expect(result.error).toContain("not on PATH") - } finally { - run.mockRestore() - } + syncInternals.runInstall = async () => ({ code: 0, timedOut: false, stderr: "" }) + const result = await installEngine() + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error).toContain("not on PATH") }) test("a zero exit with a below-floor engine on PATH is a failure", async () => { syncInternals.which = () => "/usr/local/bin/datamate" syncInternals.versionOf = async () => "0.6.3" - const run = spyOn(Process, "run").mockImplementation( - async () => ({ code: 0, stdout: Buffer.from(""), stderr: Buffer.from("") }) as never, - ) - try { - const result = await installEngine() - expect(result.ok).toBe(false) - if (!result.ok) expect(result.error).toContain("0.6.3") - } finally { - run.mockRestore() - } + syncInternals.runInstall = async () => ({ code: 0, timedOut: false, stderr: "" }) + const result = await installEngine() + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error).toContain("0.6.3") }) test("a non-zero exit reports npm's last lines", async () => { - const run = spyOn(Process, "run").mockImplementation( - async () => ({ code: 1, stdout: Buffer.from(""), stderr: Buffer.from("boom\nEACCES denied") }) as never, - ) - try { - expect(await installEngine()).toEqual({ ok: false, error: "boom EACCES denied" }) - } finally { - run.mockRestore() - } + syncInternals.runInstall = async () => ({ code: 1, timedOut: false, stderr: "boom\nEACCES denied" }) + expect(await installEngine()).toEqual({ ok: false, error: "boom EACCES denied" }) }) }) From 4ab82aececd0ae7dcb30a32196f885cce4183ae5 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 06:54:26 +0800 Subject: [PATCH 08/21] fix: keep the install deadline's SIGKILL armed after npm itself exits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit npm, the group leader, normally dies on the deadline's SIGTERM, and its exit cleared the escalation timer — so a descendant that ignored SIGTERM outlived the reported timeout. Once the deadline has fired the SIGKILL to the process group stays scheduled; the group outlives its leader while any member is alive. --- .../opencode/src/altimate/workspace/engine-offer.ts | 7 ++++++- .../altimate/workspace/engine-install-offer.test.ts | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 8c4cc698b..ff1f0db90 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -161,7 +161,10 @@ export function runInstall( if (settled) return settled = true clearTimeout(timer) - if (hard) clearTimeout(hard) + // Past the deadline the escalation stays armed: npm (the group leader) + // usually dies on SIGTERM, but a descendant that ignores it must still + // get the SIGKILL, so the leader's exit does not cancel it. + if (hard && !timedOut) clearTimeout(hard) resolve({ code, timedOut, stderr }) } const killTree = (signal: NodeJS.Signals) => { @@ -187,6 +190,8 @@ export function runInstall( timedOut = true killTree("SIGTERM") hard = setTimeout(() => { + // The group outlives its leader while any member is alive, so this + // reaches survivors even after npm itself has exited. killTree("SIGKILL") finish(null) }, graceMs) diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index cdf14d0d2..7665f649c 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -328,6 +328,18 @@ describe("install deadline", () => { expect(run.timedOut).toBe(true) expect(Date.now() - t0).toBeLessThan(3_000) }) + test.skipIf(!posix)("a descendant that ignores SIGTERM is still killed after the leader exits", async () => { + // npm (the leader) dies on the deadline's SIGTERM; the escalation must + // survive its exit and reach the straggler. + const marker = `31.${process.pid}` + const run = await runInstall(["sh", "-c", `(trap '' TERM; exec sleep ${marker}) & sleep 30`], 200, 300) + expect(run.timedOut).toBe(true) + await new Promise((resolve) => setTimeout(resolve, 600)) + const survivors = Bun.spawnSync(["pgrep", "-f", `sleep ${marker}`]) + .stdout.toString() + .trim() + expect(survivors).toBe("") + }) test("a timed-out run is reported as such, not as an npm failure", async () => { syncInternals.runInstall = async () => ({ code: null, timedOut: true, stderr: "" }) const result = await installEngine() From 62b9be6a3024da629ef27049bfd308a9bf1194a9 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 08:51:47 +0800 Subject: [PATCH 09/21] chore: rebase onto the tenant-scoped overlay (declaredFor takes the workspace) --- packages/opencode/src/altimate/workspace/engine-overlay.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index 8b8395f93..ba454a03f 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -591,7 +591,7 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS return } record(sessionID, refusal) - const declared = (await declaredFor(workspace.id))?.keys.length ?? 0 + const declared = (await declaredFor(workspace))?.keys.length ?? 0 await announceRefusal( sessionID, refusal, From 0176a5573ff08ff31fa474a2f3e2011b73dfaf22 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 28 Aug 2026 09:20:54 +0800 Subject: [PATCH 10/21] fix: drop the filter box from the engine install offer dialog The dialog already skipped filtering (`skipFilter`); the box itself only collected stray keystrokes while the install ran. `renderFilter` is passed through the plugin dialog API to the component that already supports it. --- .../opencode/src/plugin/tui/altimate/workspace.tsx | 12 +++++++----- packages/plugin/src/tui.ts | 3 +++ packages/tui/src/plugin/adapters.tsx | 3 +++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 40214bfd1..0c40fbf13 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1401,12 +1401,14 @@ function EngineInstallOfferDialog(props: EngineOfferProps) { { if (option.value === "busy") return diff --git a/packages/plugin/src/tui.ts b/packages/plugin/src/tui.ts index b96a5d7b7..70c15b8f4 100644 --- a/packages/plugin/src/tui.ts +++ b/packages/plugin/src/tui.ts @@ -180,6 +180,9 @@ export type TuiDialogSelectProps = { onFilter?: (query: string) => void onSelect?: (option: TuiDialogSelectOption) => void skipFilter?: boolean + // altimate_change start — a fixed-option dialog can hide the filter box entirely + renderFilter?: boolean + // altimate_change end current?: Value } diff --git a/packages/tui/src/plugin/adapters.tsx b/packages/tui/src/plugin/adapters.tsx index fb2b104b6..5fb7b7dd0 100644 --- a/packages/tui/src/plugin/adapters.tsx +++ b/packages/tui/src/plugin/adapters.tsx @@ -245,6 +245,9 @@ export function createTuiApiAdapters(input: Input): Omit ) From cb69795c5d73fa57929aea7b1716a8e8d80fb42c Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 01:44:40 +0800 Subject: [PATCH 11/21] test(workspace): pin the "clears the floor" versions to the floor constant The engine floor moved to 0.7.1 in the attach PR; these tests still spelled the old floor as the version that clears it. --- .../test/altimate/workspace/engine-install-offer.test.ts | 4 ++-- .../opencode/test/altimate/workspace/engine-overlay.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 7665f649c..5d2dda4a4 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -259,7 +259,7 @@ describe("offer routing — engine too old", () => { describe("offer is not raised when an engine is usable", () => { test("a healthy engine never reaches the offer path", async () => { - const h = install({ surface: true, which: "/usr/local/bin/datamate", version: "0.7.0" }) + const h = install({ surface: true, which: "/usr/local/bin/datamate", version: MIN_ENGINE_VERSION }) await beforeTurn("s1") expect(h.offers).toEqual([]) expect(h.published).toBe(0) @@ -284,7 +284,7 @@ describe("describeOffer — the TUI re-derives its own detail", () => { expect(await describeOffer(DIR)).toMatchObject({ reason: "engine-too-old", found: "0.6.3" }) }) test("returns null when an engine already clears the floor", async () => { - install({ which: "/usr/local/bin/datamate", version: "0.7.0" }) + install({ which: "/usr/local/bin/datamate", version: MIN_ENGINE_VERSION }) expect(await describeOffer(DIR)).toBeNull() }) test("returns null when the project is not bound", async () => { diff --git a/packages/opencode/test/altimate/workspace/engine-overlay.test.ts b/packages/opencode/test/altimate/workspace/engine-overlay.test.ts index a52fdcc86..af96b1f6a 100644 --- a/packages/opencode/test/altimate/workspace/engine-overlay.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-overlay.test.ts @@ -743,7 +743,7 @@ describe("beforeTurn — what a turn boundary does", () => { const h = install({ version: "0.6.3" }) await beforeTurn("s1") expect(h.probes).toBe(1) - h.version = "0.7.0" + h.version = MIN_ENGINE_VERSION await beforeTurn("s1") expect(h.probes).toBe(1) expect(settledOutcome("s1")?.kind).toBe("engine-too-old") @@ -761,7 +761,7 @@ describe("beforeTurn — what a turn boundary does", () => { await beforeTurn("s1") expect(h.probes).toBe(1) expect(settledOutcome("s1")?.kind).toBe("engine-too-old") - h.version = "0.7.0" + h.version = MIN_ENGINE_VERSION h.fingerprint = "bin-2" h.clock += 1_000 await beforeTurn("s1") @@ -773,7 +773,7 @@ describe("beforeTurn — what a turn boundary does", () => { const u = install({ version: "0.6.3" }) u.fingerprint = null await beforeTurn("s1") - u.version = "0.7.0" + u.version = MIN_ENGINE_VERSION u.clock += 1_000 await beforeTurn("s1") expect(u.probes).toBe(1) From c79dfe3904455f9a027e03902f88cf15a510ce11 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 18:55:33 +0800 Subject: [PATCH 12/21] fix(workspace): hold the engine install offer until the kv store hydrates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TUI's "Not now" latch lives in kv.json, which the store reads after startup. When that read outlasted the 3 s wait, the offer consulted the still-empty store, read the latch as absent and opened the dialog inside the seven days the user had asked for. Dropping the offer instead is no better: the attach announces a verdict once per session and only re-raises after the latch window, so the session would go without an offer it may be owed. Hold instead. Past the warning the offer waits with no deadline — `ready` flips once the read settles, on failure as well as success — and the offer is derived only afterwards, so a long wait cannot leave it stale. The single-offer slot stays reserved throughout. --- .../src/plugin/tui/altimate/workspace.tsx | 19 +++++++++++---- .../test/altimate/plugin/workspace.test.ts | 23 ++++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 0c40fbf13..419c5ccd5 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1181,8 +1181,9 @@ function engineSkipKey(workspaceId: string, scope: LatchScope | null): string { * (`api.kv.ready`). A latch checked before that reads as absent, so an offer * raised on the first message after a restart would ignore a "Not now" that * is still in force. `ready` is a plain getter with nothing to await, so poll - * it — bounded, and on timeout proceed as if hydrated rather than never - * answer. Resolves to whether the store was ready. */ + * it, bounded by `timeoutMs` (pass `Infinity` to hold until it flips — it + * does, on a failed read as well as a successful one). Resolves to whether + * the store was ready; the caller decides what a timeout means. */ const KV_READY_TIMEOUT_MS = 3_000 const KV_READY_POLL_MS = 25 async function awaitKvReady( @@ -1504,14 +1505,22 @@ async function showEngineInstallOffer(api: TuiPluginApi): Promise { if (engineOfferGeneration === generation) engineOfferVisible = false } try { + // An unhydrated store is not an absent latch: a "Not now" chosen before + // this restart is in the file still being read. Hold the offer until the + // read settles rather than open a dialog the latch may forbid — and rather + // than drop it, which would cost the session its offer (the attach only + // re-raises once the latch window has passed). The slot stays reserved + // meanwhile, and the offer is derived only afterwards so a long wait + // cannot leave it stale. + if (!(await awaitKvReady(api.kv))) { + log.warn("kv store not hydrated in time; holding the engine install offer until it is") + await awaitKvReady(api.kv, Number.POSITIVE_INFINITY) + } const offer = await describeOffer(api.state.path.directory) // Null means the situation resolved between the attach and this dialog — an // engine appeared, or the project is no longer bound. Say nothing. if (!offer) return release() const latchScope = await currentLatchScope() - if (!(await awaitKvReady(api.kv))) { - log.warn("kv store not hydrated in time; checking the engine install latch against what is loaded") - } if (isEngineSkipActive(api, offer.workspaceId, latchScope, Date.now())) { log.info("engine install offer suppressed by 7-day latch", { workspaceId: offer.workspaceId }) return release() diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index 0be8207bd..8fbfc056a 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -631,9 +631,30 @@ describe("engine install offer — kv hydration", () => { expect(await awaitKvReady({ ready: true }, 1_000, 5)).toBe(true) expect(Date.now() - t0).toBeLessThan(50) }) - test("gives up after the timeout so a stuck read never blocks the offer", async () => { + test("reports a read that outlasts the wait, so the caller can hold the offer", async () => { const t0 = Date.now() expect(await awaitKvReady({ ready: false }, 40, 5)).toBe(false) expect(Date.now() - t0).toBeGreaterThanOrEqual(35) }) + // The offer holds past the warning with no deadline: an unhydrated store is + // not an absent latch, and the attach would not re-raise a dropped offer. + test("holds without a deadline until the store hydrates", async () => { + let ready = false + setTimeout(() => { + ready = true + }, 60) + const t0 = Date.now() + expect( + await awaitKvReady( + { + get ready() { + return ready + }, + }, + Number.POSITIVE_INFINITY, + 5, + ), + ).toBe(true) + expect(Date.now() - t0).toBeGreaterThanOrEqual(50) + }) }) From a29d36c3c09f5b4a9e6ff6d50f94aa058ed3b9a9 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 19:05:45 +0800 Subject: [PATCH 13/21] fix(workspace): key the headless refusal line on the verdict, not the tool count The per-process dedupe for the headless `run` line used the same signature as the per-session announce, which carries the declared tool count. That count is not part of the verdict: when the declared lookup fails for the parent session and recovers for a sub-agent's session after the retry window, the verdict is unchanged but the count is not, and the process printed the line twice. Dedupe the printed line on kind, detail and title alone. --- .../src/altimate/workspace/engine-overlay.ts | 12 ++++++++---- .../workspace/engine-install-offer.test.ts | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index ba454a03f..0f7081240 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -66,7 +66,7 @@ export const MAX_TRACKED_SESSIONS = 256 * cost a process spawn on every turn while still being noticed once installed. */ export const FAILED_PROBE_TTL_MS = 30_000 /** A failed allowlist lookup is retried at most this often. */ -const DECLARED_RETRY_MS = 60_000 +export const DECLARED_RETRY_MS = 60_000 // ── the engine on PATH ────────────────────────────────────────────────────── @@ -711,9 +711,13 @@ export async function announceRefusal( if (isHeadless()) { // A headless `run` is one process with one stderr, whatever sessions it // creates along the way (a sub-agent's session settles the same verdict - // and would print the same line). One line per verdict per process. - if (headlessPrinted.has(signature)) return - headlessPrinted.add(signature) + // and would print the same line). One line per verdict per process — and + // the declared count is not part of the verdict: a lookup that fails for + // one session and recovers for the next changes the number in the line, + // not what the line has to say. + const line = `${outcome.kind}:${detail}:${toast.title}` + if (headlessPrinted.has(line)) return + headlessPrinted.add(line) } if (offering) { await offerOrNotify(offer, toast, sessionID) diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 5d2dda4a4..5447a9dc0 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -15,6 +15,7 @@ import { runInstall, installSpec, nodeMajor, + DECLARED_RETRY_MS, resetForTests, settledOutcome, syncInternals, @@ -250,6 +251,22 @@ describe("offer routing — engine too old", () => { await beforeTurn("parent") expect(h.printed).toHaveLength(1) }) + test("headless, a count that arrives with a later session's catalog prints nothing more", async () => { + // The declared lookup can fail for the parent session and recover for the + // task tool's child session once the retry window has passed. The verdict + // is unchanged — only the number in the line — so the process still + // prints once. + const h = install({ headless: true }) + let clock = 1_000_000 + syncInternals.now = () => clock + syncInternals.declared = async () => null + await beforeTurn("parent") + expect(h.printed).toHaveLength(1) + clock += DECLARED_RETRY_MS + 1 + syncInternals.declared = async () => ({ keys: ["dbt_build_model", "dbt_compile_model"], extensionKeys: [] }) + await beforeTurn("child") + expect(h.printed).toHaveLength(1) + }) test("a broken engine reports 'unknown' rather than a version", async () => { const h = install({ surface: true, which: "/usr/local/bin/datamate", version: null }) await beforeTurn("s1") From 168dba8e337fc47c0b56d46a825dd5be4ee3e77c Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 20:01:50 +0800 Subject: [PATCH 14/21] fix(workspace): leave the count out of the offer when it is unknown When the allowlist lookup fails or the API is not configured, the offer payload collapsed the missing count to 0, so the headless line read "0 integration tools need the local engine" and the dialog declared zero tools next to an Install button. The count is now optional on the offer and the text drops the number instead. While there: "1 integration tool needs", not "need". --- .../src/altimate/workspace/engine-offer.ts | 22 ++++++++++++++----- .../src/altimate/workspace/engine-overlay.ts | 6 ++--- .../src/plugin/tui/altimate/workspace.tsx | 7 ++++-- .../workspace/engine-install-offer.test.ts | 10 ++++++++- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index ff1f0db90..2bd37dc66 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -47,7 +47,10 @@ export type EngineOffer = { workspaceId: string workspaceName: string /** Declared, CLI-servable integration tools that are unavailable without it. */ - declared: number + /** Declared integration tools — absent when the allowlist lookup failed or + * the API is not configured, so the text can drop the number rather than + * print 0. */ + declared?: number /** Version found — only set for "engine-too-old". */ found?: string /** The exact install/update command. */ @@ -85,12 +88,12 @@ export async function describeOffer(directory: string): Promise props.nodeMajor !== null && props.nodeMajor >= MIN_NODE_MAJOR && props.hasNpm const title = () => { - const tools = `${props.offer.declared} integration tool${props.offer.declared === 1 ? "" : "s"}` + const n = props.offer.declared + const tools = n === undefined ? "its integration tools" : `${n} integration tool${n === 1 ? "" : "s"}` const head = props.offer.reason === "engine-too-old" ? `Workspace "${props.offer.workspaceName}" needs a newer local engine (found ${props.offer.found ?? "unknown"}) — ${tools} unavailable` - : `Workspace "${props.offer.workspaceName}" declares ${tools}, which need the local engine` + : n === undefined + ? `Workspace "${props.offer.workspaceName}" has integration tools that need the local engine` + : `Workspace "${props.offer.workspaceName}" declares ${tools}, which need${n === 1 ? "s" : ""} the local engine` const parts = [head, command()] if (!canInstall()) { parts.push( diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 5447a9dc0..a185ff825 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -178,7 +178,7 @@ describe("offer routing — engine missing", () => { test("singularises the tool count", async () => { const h = install({ headless: true, declaredKeys: ["dbt_build_model"] }) await beforeTurn("s1") - expect(h.printed[0]).toContain("1 integration tool need") + expect(h.printed[0]).toContain("1 integration tool needs") }) test("the offer is raised once per session per verdict, naming the session it is for", async () => { const h = install({}) @@ -251,6 +251,14 @@ describe("offer routing — engine too old", () => { await beforeTurn("parent") expect(h.printed).toHaveLength(1) }) + test("headless, an unknown declared count is not printed as 0", async () => { + const h = install({ headless: true }) + syncInternals.declared = async () => null + await beforeTurn("s1") + expect(h.printed).toEqual([ + `Workspace "analytics": its integration tools need the local engine, which is not installed. Install it with: ${installCommand()}`, + ]) + }) test("headless, a count that arrives with a later session's catalog prints nothing more", async () => { // The declared lookup can fail for the parent session and recover for the // task tool's child session once the retry window has passed. The verdict From 46711a59fee21ba8f0c2d7b9cffe6f84b5bc8b8a Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 20:05:01 +0800 Subject: [PATCH 15/21] fix(workspace): announce a refusal once per verdict, not once per count The per-session announce signature carried the declared tool count, so a lookup that failed on one turn and recovered on the next re-raised the offer for the same missing-engine verdict. Key the signature on kind, detail and title; the headless line reuses it. Also from review: a thrown install rejection after the dialog was dismissed now reaches the user as a toast, as a reported failure already did; the too-old toast advertises the same command the offer copies and runs (`describeRefusal` takes it); and the run closes its end of npm's stderr pipe once settled so an inheriting descendant cannot hold it open. --- .../src/altimate/workspace/engine-offer.ts | 3 +++ .../src/altimate/workspace/engine-overlay.ts | 18 ++++++++---------- .../src/altimate/workspace/engine-types.ts | 10 +++++++--- .../src/plugin/tui/altimate/workspace.tsx | 15 +++++++++++++-- .../workspace/engine-install-offer.test.ts | 14 ++++++++++++++ .../altimate/workspace/engine-types.test.ts | 3 +++ 6 files changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 2bd37dc66..1c12b730f 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -168,6 +168,9 @@ export function runInstall( // usually dies on SIGTERM, but a descendant that ignores it must still // get the SIGKILL, so the leader's exit does not cancel it. if (hard && !timedOut) clearTimeout(hard) + // A descendant that inherited stderr can hold the pipe open after npm + // exits; nothing more is read from it once the run has settled. + child.stderr?.destroy() resolve({ code, timedOut, stderr }) } const killTree = (signal: NodeJS.Signals) => { diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index 4cffec1ae..9cac9be16 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -597,7 +597,7 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS refusal, { title: `Workspace "${workspace.name}": engine not usable`, - message: describeRefusal(refusal.found, workspace.name), + message: describeRefusal(refusal.found, workspace.name, installCommand()), variant: "warning", }, { @@ -696,8 +696,10 @@ export async function announceRefusal( ): Promise { const rec = sessions.get(sessionID) ?? record(sessionID, outcome) const detail = "error" in outcome ? outcome.error : "found" in outcome ? String(outcome.found) : "" - const declared = "declared" in outcome ? String(outcome.declared ?? "?") : "" - const signature = `${outcome.kind}:${detail}:${declared}:${toast.title}` + // The declared count is not part of the verdict: a lookup that fails on one + // turn and recovers on the next changes the number in the text, not what + // the text has to say, so it must not re-announce (nor re-print). + const signature = `${outcome.kind}:${detail}:${toast.title}` const offering = !!offer && INSTALL_HELPS[outcome.kind] const at = now() let repeat = false @@ -711,13 +713,9 @@ export async function announceRefusal( if (isHeadless()) { // A headless `run` is one process with one stderr, whatever sessions it // creates along the way (a sub-agent's session settles the same verdict - // and would print the same line). One line per verdict per process — and - // the declared count is not part of the verdict: a lookup that fails for - // one session and recovers for the next changes the number in the line, - // not what the line has to say. - const line = `${outcome.kind}:${detail}:${toast.title}` - if (headlessPrinted.has(line)) return - headlessPrinted.add(line) + // and would print the same line). One line per verdict per process. + if (headlessPrinted.has(signature)) return + headlessPrinted.add(signature) } if (offering) { await offerOrNotify(offer, toast, sessionID) diff --git a/packages/opencode/src/altimate/workspace/engine-types.ts b/packages/opencode/src/altimate/workspace/engine-types.ts index 62e559bbe..e88d8c3b5 100644 --- a/packages/opencode/src/altimate/workspace/engine-types.ts +++ b/packages/opencode/src/altimate/workspace/engine-types.ts @@ -189,17 +189,21 @@ export function pinnedWorkspace(entry: EntryLike | null): string | null { * run" are one code path and very different problems; conflating them sent * more than one debugging session hunting a version mismatch that did not * exist. */ -export function describeRefusal(found: string | null, workspaceName: string): string { +export function describeRefusal( + found: string | null, + workspaceName: string, + command: string = INSTALL_COMMAND, +): string { if (!found) { return ( `The ${ENGINE_BINARY} on PATH did not report a usable version, so it cannot serve workspace ` + `"${workspaceName}". It is more likely broken than out of date — try \`${ENGINE_BINARY} --version\` ` + - `directly. Reinstall with: ${INSTALL_COMMAND}` + `directly. Reinstall with: ${command}` ) } return ( `Found ${ENGINE_BINARY} ${found}; workspace "${workspaceName}" needs ${MIN_ENGINE_VERSION} or newer. ` + - `Update with: ${INSTALL_COMMAND}` + `Update with: ${command}` ) } diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 26d10ce60..5ccf8566d 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1420,9 +1420,20 @@ function EngineInstallOfferDialog(props: EngineOfferProps) { if (installing) return installing = true void runInstall().catch((err) => { - setFailure(err instanceof Error ? err.message : String(err)) - setPhase("failed") + const message = err instanceof Error ? err.message : String(err) installing = false + if (!mounted) { + // Same as a reported failure after dismissal: the rows are gone, + // so the error reaches the user as a toast or not at all. + props.api.ui.toast({ + variant: "error", + message: `Workspace engine install failed: ${message}. Run: ${command()}`, + duration: 30_000, + }) + return + } + setFailure(message) + setPhase("failed") }) return } diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index a185ff825..8e69310c0 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -251,6 +251,20 @@ describe("offer routing — engine too old", () => { await beforeTurn("parent") expect(h.printed).toHaveLength(1) }) + test("a count that arrives on a later turn does not re-raise the offer in the session", async () => { + // Same verdict, better number: the dialog was already raised for it. + const h = install({ surface: true }) + let clock = 1_000_000 + syncInternals.now = () => clock + syncInternals.declared = async () => null + await beforeTurn("s1") + expect(h.offers).toHaveLength(1) + expect(h.offers[0]).not.toHaveProperty("declared") + clock += DECLARED_RETRY_MS + 1 + syncInternals.declared = async () => ({ keys: ["dbt_build_model", "dbt_compile_model"], extensionKeys: [] }) + await beforeTurn("s1") + expect(h.offers).toHaveLength(1) + }) test("headless, an unknown declared count is not printed as 0", async () => { const h = install({ headless: true }) syncInternals.declared = async () => null diff --git a/packages/opencode/test/altimate/workspace/engine-types.test.ts b/packages/opencode/test/altimate/workspace/engine-types.test.ts index 316ee380f..091f39b03 100644 --- a/packages/opencode/test/altimate/workspace/engine-types.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-types.test.ts @@ -149,6 +149,9 @@ describe("messages", () => { expect(describeRefusal(null, "analytics")).toContain(INSTALL_COMMAND) expect(describeRefusal("0.6.3", "analytics")).toContain(`needs ${MIN_ENGINE_VERSION} or newer`) expect(describeRefusal("0.6.3", "analytics")).toContain("Found datamate 0.6.3") + expect(describeRefusal("0.6.3", "analytics", "npm i -g @altimateai/datamate@next")).toContain( + "Update with: npm i -g @altimateai/datamate@next", + ) }) test("the missing list is truncated after five", () => { expect(describeMissing([])).toBe("") From afae5b80a35f97e94d35eeb4019264948e539647 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 20:14:32 +0800 Subject: [PATCH 16/21] fix(workspace): fingerprint the engine binary by identity, not size and mtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The too-old probe memo ended when the file on PATH changed, judged by size and mtime — an identity a replacement can collide with if it keeps the byte length and restores the timestamp, leaving a just-installed engine refused until the memo's TTL ran out. Fingerprint inode, size, mtime and ctime: a replacement has a new inode, and a rewrite in place moves the ctime, which nothing in userland can set back. Also merges the two doc comments on the offer's optional `declared`. --- .../src/altimate/workspace/engine-offer.ts | 7 ++--- .../src/altimate/workspace/engine-probes.ts | 9 ++++-- .../altimate/workspace/engine-probes.test.ts | 30 +++++++++++++++++-- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 1c12b730f..2a75f6ae3 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -46,10 +46,9 @@ export type EngineOffer = { /** Stable id — the 7-day "Not now" latch keys on this, not the name. */ workspaceId: string workspaceName: string - /** Declared, CLI-servable integration tools that are unavailable without it. */ - /** Declared integration tools — absent when the allowlist lookup failed or - * the API is not configured, so the text can drop the number rather than - * print 0. */ + /** Declared, CLI-servable integration tools that are unavailable without + * it. Absent when the allowlist lookup failed or the API is not configured, + * so the text can drop the number rather than print 0. */ declared?: number /** Version found — only set for "engine-too-old". */ found?: string diff --git a/packages/opencode/src/altimate/workspace/engine-probes.ts b/packages/opencode/src/altimate/workspace/engine-probes.ts index 878337bd0..132e571fa 100644 --- a/packages/opencode/src/altimate/workspace/engine-probes.ts +++ b/packages/opencode/src/altimate/workspace/engine-probes.ts @@ -47,14 +47,17 @@ export function which(cmd: string): string | null { } /** Identity of the file behind a PATH hit, cheap enough to ask every turn: - * size and mtime of the target (symlinks followed, so an npm bin shim whose - * package was reinstalled reads as changed). Null when it cannot be stat'ed; + * inode, size, mtime and ctime of the target (symlinks followed, so an npm + * bin shim whose package was reinstalled reads as changed). A replacement + * file has a new inode; a rewrite in place that keeps the length and restores + * the mtime still moves the ctime, which nothing in userland can set back — + * so an update cannot read as the same file. Null when it cannot be stat'ed; * with nothing to compare, the caller's memo falls back to its TTL. */ export function fingerprint(bin: string): string | null { if (syncInternals.fingerprint) return syncInternals.fingerprint(bin) try { const stat = statSync(bin) - return `${stat.size}:${stat.mtimeMs}` + return `${stat.dev}:${stat.ino}:${stat.size}:${stat.mtimeMs}:${stat.ctimeMs}` } catch { return null } diff --git a/packages/opencode/test/altimate/workspace/engine-probes.test.ts b/packages/opencode/test/altimate/workspace/engine-probes.test.ts index c5871ca89..a5edccfd8 100644 --- a/packages/opencode/test/altimate/workspace/engine-probes.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-probes.test.ts @@ -3,10 +3,10 @@ // The engine probes against real processes: `versionOf` must settle on the // engine's own exit, never wait on a descendant that inherited its stdout. import { describe, expect, test } from "bun:test" -import { chmodSync, mkdtempSync, writeFileSync } from "node:fs" +import { chmodSync, mkdtempSync, statSync, utimesSync, writeFileSync } from "node:fs" import os from "node:os" import path from "node:path" -import { versionOf } from "../../../src/altimate/workspace/engine-probes" +import { fingerprint, versionOf } from "../../../src/altimate/workspace/engine-probes" const posix = process.platform !== "win32" @@ -18,6 +18,32 @@ function fakeEngine(script: string): string { return bin } +describe("fingerprint", () => { + test("an in-place rewrite that keeps the length and restores the mtime still reads as a new file", async () => { + const dir = mkdtempSync(path.join(os.tmpdir(), "engine-fp-")) + const bin = path.join(dir, "datamate") + writeFileSync(bin, "#!/bin/sh\necho 0.6.3\n") + // A whole-second mtime, as files extracted from a tarball carry, so a + // later `utimes` can restore it exactly. + const stamp = Math.floor(Date.now() / 1000) - 60 + utimesSync(bin, stamp, stamp) + const before = statSync(bin) + const first = fingerprint(bin) + // ctime has whole-millisecond resolution on some filesystems; make sure + // the rewrite lands in a later tick. + await new Promise((resolve) => setTimeout(resolve, 20)) + writeFileSync(bin, "#!/bin/sh\necho 0.7.1\n") // same byte length + utimesSync(bin, stamp, stamp) // "postinstall restores the timestamp" + const after = statSync(bin) + expect(after.size).toBe(before.size) + expect(after.mtimeMs).toBe(before.mtimeMs) + expect(fingerprint(bin)).not.toBe(first) + }) + test("null when the file cannot be stat'ed", () => { + expect(fingerprint(path.join(os.tmpdir(), "engine-fp-missing", "datamate"))).toBeNull() + }) +}) + describe("versionOf", () => { test.skipIf(!posix)("reads the version even when a descendant keeps stdout open", async () => { const bin = fakeEngine('echo "0.7.0"; sleep 5 & exit 0') From aadaf28e00dae7992e5ba403931c5387d4a67822 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 20:17:27 +0800 Subject: [PATCH 17/21] fix(workspace): tell same-named workspaces apart in the refusal signature The announce signature carries the workspace's name through the toast title, and two workspaces can share a name: a headless process serving a second directory bound to a namesake would have had its line suppressed. The workspace id is part of the signature now. The mid-session count test moves to the engine-missing block it actually exercises. --- .../src/altimate/workspace/engine-overlay.ts | 6 ++- .../workspace/engine-install-offer.test.ts | 38 ++++++++++++------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index 9cac9be16..1c10443db 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -698,8 +698,10 @@ export async function announceRefusal( const detail = "error" in outcome ? outcome.error : "found" in outcome ? String(outcome.found) : "" // The declared count is not part of the verdict: a lookup that fails on one // turn and recovers on the next changes the number in the text, not what - // the text has to say, so it must not re-announce (nor re-print). - const signature = `${outcome.kind}:${detail}:${toast.title}` + // the text has to say, so it must not re-announce (nor re-print). The + // workspace is: the title carries its name, and two workspaces can share + // one, so the id goes in as well. + const signature = `${outcome.kind}:${detail}:${toast.title}:${offer?.workspaceId ?? ""}` const offering = !!offer && INSTALL_HELPS[outcome.kind] const at = now() let repeat = false diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 8e69310c0..87691cd51 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -217,6 +217,30 @@ describe("offer routing — engine missing", () => { await beforeTurn("s1") expect(h.published).toBe(3) }) + test("a count that arrives on a later turn does not re-raise the offer in the session", async () => { + // Same verdict, better number: the dialog was already raised for it. + const h = install({ surface: true }) + let clock = 1_000_000 + syncInternals.now = () => clock + syncInternals.declared = async () => null + await beforeTurn("s1") + expect(h.offers).toHaveLength(1) + expect(h.offers[0]).not.toHaveProperty("declared") + clock += DECLARED_RETRY_MS + 1 + syncInternals.declared = async () => ({ keys: ["dbt_build_model", "dbt_compile_model"], extensionKeys: [] }) + await beforeTurn("s1") + expect(h.offers).toHaveLength(1) + }) + test("headless, two same-named workspaces in one process each print their line", async () => { + // The title names the workspace, not its id; a second directory bound to + // a different workspace with the same name is a different verdict. + const h = install({ headless: true }) + await beforeTurn("s1") + syncInternals.instanceDirectory = () => "/tmp/analytics-2" + syncInternals.resolveBinding = async () => ({ ...binding, datamateId: 43, projectPath: "/tmp/analytics-2" }) + await beforeTurn("s2") + expect(h.printed).toHaveLength(2) + }) }) describe("offer routing — engine too old", () => { @@ -251,20 +275,6 @@ describe("offer routing — engine too old", () => { await beforeTurn("parent") expect(h.printed).toHaveLength(1) }) - test("a count that arrives on a later turn does not re-raise the offer in the session", async () => { - // Same verdict, better number: the dialog was already raised for it. - const h = install({ surface: true }) - let clock = 1_000_000 - syncInternals.now = () => clock - syncInternals.declared = async () => null - await beforeTurn("s1") - expect(h.offers).toHaveLength(1) - expect(h.offers[0]).not.toHaveProperty("declared") - clock += DECLARED_RETRY_MS + 1 - syncInternals.declared = async () => ({ keys: ["dbt_build_model", "dbt_compile_model"], extensionKeys: [] }) - await beforeTurn("s1") - expect(h.offers).toHaveLength(1) - }) test("headless, an unknown declared count is not printed as 0", async () => { const h = install({ headless: true }) syncInternals.declared = async () => null From d448ff3a09e9a4d2c0a06637c3004dcc040e7dee Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Mon, 31 Aug 2026 20:21:53 +0800 Subject: [PATCH 18/21] test(workspace): derive the second directory's overlay before its turn --- .../altimate/workspace/engine-install-offer.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 87691cd51..ad3f8a937 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -236,9 +236,17 @@ describe("offer routing — engine missing", () => { // a different workspace with the same name is a different verdict. const h = install({ headless: true }) await beforeTurn("s1") - syncInternals.instanceDirectory = () => "/tmp/analytics-2" - syncInternals.resolveBinding = async () => ({ ...binding, datamateId: 43, projectPath: "/tmp/analytics-2" }) + // A second directory in the same process, bound to a namesake workspace: + // its overlay is derived at its own config load, as the first one's was. + const DIR2 = "/tmp/analytics-2" + syncInternals.instanceDirectory = () => DIR2 + syncInternals.resolveBinding = async () => ({ ...binding, datamateId: 43, projectPath: DIR2 }) + const { overlay } = await import("../../../src/altimate/workspace/engine-overlay") + const config2: { mcp?: Record } = { mcp: {} } + await overlay(DIR2, config2) + syncInternals.config = { invalidate: async () => {}, get: async () => config2 } await beforeTurn("s2") + expect(settledOutcome("s2")).toEqual({ kind: "engine-missing", declared: 2 }) expect(h.printed).toHaveLength(2) }) }) From e1cfbcf2d66197c8f2b3f1d4cb6ee4c947b485a6 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Tue, 1 Sep 2026 12:35:33 +0800 Subject: [PATCH 19/21] fix(workspace): reap what outlives a successful npm install When npm exited before the deadline, the run resolved and the deadline's cleanup was cancelled, so a descendant a lifecycle script had left in the process group kept running. On exit the group now gets SIGTERM, then SIGKILL after the grace, without the result waiting for either. Real-process test. Also fixes a mis-indented line in the offer flow. --- .../src/altimate/workspace/engine-offer.ts | 12 +++++++++++- .../src/plugin/tui/altimate/workspace.tsx | 2 +- .../workspace/engine-install-offer.test.ts | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 2a75f6ae3..ed9e75520 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -201,7 +201,17 @@ export function runInstall( finish(null) }, graceMs) }, timeoutMs) - child.once("exit", (code) => finish(code)) + child.once("exit", (code) => { + finish(code) + // npm is done, but a descendant it forked may still be alive in the + // group (a lifecycle script's daemon). It is the install's straggler, + // not the user's: reap it — SIGTERM now, SIGKILL after the grace — + // without holding the result or this process for it. + if (!timedOut && grouped) { + killTree("SIGTERM") + setTimeout(() => killTree("SIGKILL"), graceMs).unref() + } + }) child.once("error", (err) => { stderr = stderr || err.message finish(null) diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 5ccf8566d..0d03703bf 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1540,7 +1540,7 @@ async function showEngineInstallOffer(api: TuiPluginApi): Promise { return release() } const major = await detectNodeMajor() - const hasNpm = npmAvailable() + const hasNpm = npmAvailable() api.ui.dialog.replace(() => ( { .trim() expect(survivors).toBe("") }) + test.skipIf(!posix)("a descendant that outlives a successful npm is reaped", async () => { + // npm done, exit 0 — but a lifecycle script left a daemon in the group. It + // is the install's straggler, not the user's: gone within the grace, + // without the run waiting for it. + const marker = `32.${process.pid}` + const t0 = Date.now() + const run = await runInstall(["sh", "-c", `(exec sleep ${marker}) & exit 0`], 4_000, 200) + expect(run.code).toBe(0) + expect(run.timedOut).toBe(false) + expect(Date.now() - t0).toBeLessThan(2_000) + await new Promise((resolve) => setTimeout(resolve, 600)) + const survivors = Bun.spawnSync(["pgrep", "-f", `sleep ${marker}`]) + .stdout.toString() + .trim() + expect(survivors).toBe("") + }) test("a timed-out run is reported as such, not as an npm failure", async () => { syncInternals.runInstall = async () => ({ code: null, timedOut: true, stderr: "" }) const result = await installEngine() From 68dcce4313b6a7f57d17669acfeb85c3d44a5f0f Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Tue, 1 Sep 2026 15:29:22 +0800 Subject: [PATCH 20/21] fix(workspace): review follow-ups on the install offer's raise path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavioural tests for the raise path, driven through the plugin's stubbed api with no renderer: a missing engine reaches the dialog once and a second raise while it is up does not; a raise during an in-flight install never reaches the dialog; a null offer frees the slot; a directory absent on this host is refused with the server-side notice; the 7-day latch suppresses and frees; and `canInstallWith` — Node 20+ is not enough, npm must be on PATH. Also: the healthy probe short-circuits before the PATH scan; a clock that moves backwards re-raises the offer, as the TUI's latch already treats it (test); the second hourly re-raise is asserted; the install seam carries the grace argument; the deadline's SIGKILL timer is unref'd and its dead clear removed; a too-old engine still first on PATH after a zero exit is named as shadowing, with its path; an unreadable binding in `describeOffer` is logged; the kv hold has a local six-minute bound above the store's own lock timeout; the failed phase's `current` row exists; the never-painted "installed" row is gone; the unread `missing` memo write says so. --- .../src/altimate/workspace/engine-offer.ts | 22 +++- .../src/altimate/workspace/engine-overlay.ts | 15 ++- .../src/altimate/workspace/engine-seams.ts | 1 + .../src/plugin/tui/altimate/workspace.tsx | 46 +++++-- .../test/altimate/plugin/workspace.test.ts | 116 +++++++++++++++++- .../workspace/engine-install-offer.test.ts | 16 +++ 6 files changed, 197 insertions(+), 19 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index ed9e75520..2c6e187f3 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -81,7 +81,12 @@ export function installCommand(): string { export async function describeOffer(directory: string): Promise { const binding = syncInternals.resolveBinding ? await syncInternals.resolveBinding(directory) - : await readLocalBinding(directory).catch(() => null) + : await readLocalBinding(directory).catch((err) => { + // Transiently unreadable is not unbound; the overlay tells them apart + // and this dialog says nothing either way — leave a trace. + log.info("engine offer: binding unreadable, nothing offered this turn", { directory, err: String(err) }) + return null + }) if (!binding) return null const workspaceId = String(binding.datamateId) const bin = which(ENGINE_BINARY) @@ -138,7 +143,7 @@ export function runInstall( timeoutMs = INSTALL_TIMEOUT_MS, graceMs = INSTALL_KILL_GRACE_MS, ): Promise { - if (syncInternals.runInstall) return syncInternals.runInstall(argv, timeoutMs) + if (syncInternals.runInstall) return syncInternals.runInstall(argv, timeoutMs, graceMs) return new Promise((resolve) => { const grouped = process.platform !== "win32" let child: ChildProcess @@ -166,7 +171,6 @@ export function runInstall( // Past the deadline the escalation stays armed: npm (the group leader) // usually dies on SIGTERM, but a descendant that ignores it must still // get the SIGKILL, so the leader's exit does not cancel it. - if (hard && !timedOut) clearTimeout(hard) // A descendant that inherited stderr can hold the pipe open after npm // exits; nothing more is read from it once the run has settled. child.stderr?.destroy() @@ -196,10 +200,11 @@ export function runInstall( killTree("SIGTERM") hard = setTimeout(() => { // The group outlives its leader while any member is alive, so this - // reaches survivors even after npm itself has exited. + // reaches survivors even after npm itself has exited. Unref'd: it must + // not keep the process alive once the run has already resolved. killTree("SIGKILL") finish(null) - }, graceMs) + }, graceMs).unref() }, timeoutMs) child.once("exit", (code) => { finish(code) @@ -240,9 +245,14 @@ export async function installEngine(): Promise { } const installedVersion = await versionOf(installedBin) if (!clearsFloor(installedVersion)) { + // The likeliest cause is not a bad install: an older engine earlier on + // PATH shadows the one npm just wrote. return { ok: false, - error: `npm installed it, but ${ENGINE_BINARY} on PATH reports ${installedVersion ?? "no version"}`, + error: + `npm installed it, but the ${ENGINE_BINARY} first on PATH (${installedBin}) reports ` + + `${installedVersion ?? "no version"} — an older install earlier on PATH is shadowing the new one; ` + + `remove it, or put npm's global bin directory ahead of it`, } } return { ok: true } diff --git a/packages/opencode/src/altimate/workspace/engine-overlay.ts b/packages/opencode/src/altimate/workspace/engine-overlay.ts index 1c10443db..424c0f140 100644 --- a/packages/opencode/src/altimate/workspace/engine-overlay.ts +++ b/packages/opencode/src/altimate/workspace/engine-overlay.ts @@ -80,8 +80,8 @@ function now(): number { async function probeEngine(): Promise { const at = now() - const bin = which(ENGINE_BINARY) - // A usable engine is remembered for the process. A missing one is asked + // A usable engine is remembered for the process — before the PATH scan, so + // the healthy path costs nothing per turn. A missing one is asked // about on every call — `which` is a PATH scan, no process spawn — so an // install made from the offer dialog (which runs in another module realm and // cannot reach this memo) is seen on the next turn. A too-old or broken one @@ -89,8 +89,9 @@ async function probeEngine(): Promise { // while the file on PATH is the same one: an update written over it (the // offer's `npm i -g` on an old engine) changes the fingerprint and is // re-probed on the next turn, just as an install is. - const seen = bin ? fingerprint(bin) : null if (probeMemo && probeMemo.result.kind === "ok") return probeMemo.result + const bin = which(ENGINE_BINARY) + const seen = bin ? fingerprint(bin) : null if ( probeMemo && probeMemo.result.kind === "too-old" && @@ -107,6 +108,8 @@ async function probeEngine(): Promise { const version = await versionOf(bin) result = clearsFloor(version) ? { kind: "ok", version: version! } : { kind: "too-old", found: version } } + // A `missing` result is recorded too but never honoured: the guards above + // match only `ok` and `too-old`, so an install is noticed on the next call. probeMemo = { result, at, fingerprint: seen } return result } @@ -706,7 +709,11 @@ export async function announceRefusal( const at = now() let repeat = false if (rec.announced === signature) { - const expired = offering && rec.announcedAt !== undefined && at - rec.announcedAt >= OFFER_SKIP_TTL_MS + // A clock that moved backwards (NTP correction, VM resume) reads as + // expired, as the TUI's latch treats it — otherwise the overlay would stop + // raising the offer until real time caught up plus the whole window. + const elapsed = rec.announcedAt === undefined ? undefined : at - rec.announcedAt + const expired = offering && elapsed !== undefined && (elapsed < 0 || elapsed >= OFFER_SKIP_TTL_MS) if (!expired) return repeat = true } diff --git a/packages/opencode/src/altimate/workspace/engine-seams.ts b/packages/opencode/src/altimate/workspace/engine-seams.ts index 93f69f35c..9d2f0ad99 100644 --- a/packages/opencode/src/altimate/workspace/engine-seams.ts +++ b/packages/opencode/src/altimate/workspace/engine-seams.ts @@ -35,6 +35,7 @@ export const syncInternals: { runInstall?: ( argv: string[], timeoutMs: number, + graceMs: number, ) => Promise<{ code: number | null; timedOut: boolean; stderr: string }> nodeMajor?: () => Promise npmAvailable?: () => boolean diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 0d03703bf..f59f0275b 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1185,6 +1185,8 @@ function engineSkipKey(workspaceId: string, scope: LatchScope | null): string { * does, on a failed read as well as a successful one). Resolves to whether * the store was ready; the caller decides what a timeout means. */ const KV_READY_TIMEOUT_MS = 3_000 +/** Upper bound on holding an offer for hydration; see `showEngineInstallOffer`. */ +export const KV_READY_HOLD_MS = 6 * 60_000 const KV_READY_POLL_MS = 25 async function awaitKvReady( kv: { readonly ready: boolean }, @@ -1244,7 +1246,7 @@ interface EngineOfferProps { * DialogSelect's ``filtered()`` drops those, leaving an empty list). */ function EngineInstallOfferDialog(props: EngineOfferProps) { const clipboard = useClipboard() - const [phase, setPhase] = createSignal<"idle" | "installing" | "installed" | "failed">("idle") + const [phase, setPhase] = createSignal<"idle" | "installing" | "failed">("idle") const [failure, setFailure] = createSignal(null) // The install outlives this component: Escape or a click outside dismisses // the dialog while npm keeps running. Signals set after that update nothing @@ -1266,7 +1268,7 @@ function EngineInstallOfferDialog(props: EngineOfferProps) { let installing = false const command = () => props.offer.command - const canInstall = () => props.nodeMajor !== null && props.nodeMajor >= MIN_NODE_MAJOR && props.hasNpm + const canInstall = () => canInstallWith(props.nodeMajor, props.hasNpm) const title = () => { const n = props.offer.declared @@ -1296,8 +1298,6 @@ function EngineInstallOfferDialog(props: EngineOfferProps) { switch (phase()) { case "installing": return [{ title: "Installing… this can take a minute.", value: "busy" }] - case "installed": - return [{ title: "Installed — attaching integrations.", value: "close" }] case "failed": return [ { title: "Copy command", value: "copy", description: "Run it yourself, then start a new session." }, @@ -1356,7 +1356,6 @@ function EngineInstallOfferDialog(props: EngineOfferProps) { setPhase("failed") return } - setPhase("installed") // Only clear a dialog we still own — by now the user may have opened // another, and clearing the stack would take theirs down instead. if (mounted) props.api.ui.dialog.clear() @@ -1413,7 +1412,7 @@ function EngineInstallOfferDialog(props: EngineOfferProps) { // the box itself only collects stray keystrokes while the install runs. skipFilter renderFilter={false} - current={canInstall() ? "install" : "copy"} + current={phase() === "failed" ? "copy" : canInstall() ? "install" : "copy"} onSelect={(option) => { if (option.value === "busy") return if (option.value === "install") { @@ -1528,7 +1527,14 @@ async function showEngineInstallOffer(api: TuiPluginApi): Promise { // cannot leave it stale. if (!(await awaitKvReady(api.kv))) { log.warn("kv store not hydrated in time; holding the engine install offer until it is") - await awaitKvReady(api.kv, Number.POSITIVE_INFINITY) + // The hold has a local bound, set above the kv file lock's own timeout + // (Flock, five minutes, in the TUI's kv provider) so it only ends a hold + // the store itself has already given up on. Past it the slot is freed + // and the offer waits for a later raise rather than opening unlatched. + if (!(await awaitKvReady(api.kv, KV_READY_HOLD_MS))) { + log.warn("kv store still not hydrated; releasing the engine install offer for a later raise") + return release() + } } const offer = await describeOffer(api.state.path.directory) // Null means the situation resolved between the attach and this dialog — an @@ -1620,5 +1626,29 @@ 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, isEngineSkipActive, recordEngineSkip, awaitKvReady } +/** "Install now" is offered only with Node 20+ and npm on PATH — Node is not + * enough on its own, npm ships separately on several distributions. */ +export function canInstallWith(nodeMajor: number | null, hasNpm: boolean): boolean { + return nodeMajor !== null && nodeMajor >= MIN_NODE_MAJOR && hasNpm +} + +/** Test seam for the raise path's process-wide state. */ +export const engineOfferInternals = { + get visible() { + return engineOfferVisible + }, + get inFlight() { + return engineInstallInFlight + }, + set({ visible, inFlight }: { visible?: boolean; inFlight?: boolean }) { + if (visible !== undefined) engineOfferVisible = visible + if (inFlight !== undefined) engineInstallInFlight = inFlight + }, + reset() { + engineOfferVisible = false + engineInstallInFlight = false + }, +} + +export { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip, awaitKvReady, showEngineInstallOffer } // altimate_change end diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index 8fbfc056a..342d7c824 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -28,7 +28,16 @@ afterAll(() => { } }) -const { isSkipActive, recordSkip, isEngineSkipActive, recordEngineSkip, awaitKvReady } = await import( +const { + isSkipActive, + recordSkip, + isEngineSkipActive, + recordEngineSkip, + awaitKvReady, + canInstallWith, + engineOfferInternals, + showEngineInstallOffer, +} = await import( "../../../src/plugin/tui/altimate/workspace" ) const { projectNameFromRemote, detectProjectRemote } = await import( @@ -37,6 +46,7 @@ const { projectNameFromRemote, detectProjectRemote } = await import( const { cachePath, readLocalBinding, recordApprovedBinding } = await import( "../../../src/altimate/workspace/state" ) +const { syncInternals } = await import("../../../src/altimate/workspace/engine-seams") // Stub AltimateApi.getCredentials / isConfigured — used by readLocalBinding // and recordApprovedBinding for tenant/apiUrl scoping. Re-import allows @@ -604,6 +614,110 @@ describe("Engine install-offer latch", () => { }) }) +describe("engine install offer — raise path", () => { + // `showEngineInstallOffer` runs without a renderer: the dialog factory handed + // to `dialog.replace` is never invoked here. What is under test is the path + // up to it — the single-offer slot, the in-flight guard, the attach-host + // guard, and what a null offer does to the slot. + const binding = { + datamateId: 42, + datamateName: "analytics", + repoRemote: null, + projectPath: os.tmpdir(), + linkedAt: 0, + } + function api(directory: string) { + const h = { replaced: 0, toasts: [] as { message: string }[] } + const kv = makeKv() + const a = { + kv: { ...kv, ready: true }, + state: { path: { directory } }, + ui: { + toast: (t: { message: string }) => { + h.toasts.push(t) + }, + dialog: { + replace: () => { + h.replaced += 1 + }, + clear: () => {}, + }, + }, + } as any + return { a, h } + } + function missingEngine() { + syncInternals.resolveBinding = async () => binding as any + syncInternals.which = () => null + syncInternals.declared = async () => ({ keys: ["dbt_build_model"], extensionKeys: [] }) + syncInternals.nodeMajor = async () => 22 + syncInternals.npmAvailable = () => true + } + beforeEach(() => { + engineOfferInternals.reset() + stubCreds("acme", "https://api.acme.example.com") + }) + afterEach(() => { + engineOfferInternals.reset() + for (const key of Object.keys(syncInternals)) delete (syncInternals as Record)[key] + }) + + test("canInstallWith: Node 20+ is not enough, npm must be on PATH too", () => { + expect(canInstallWith(22, true)).toBe(true) + expect(canInstallWith(22, false)).toBe(false) + expect(canInstallWith(18, true)).toBe(false) + expect(canInstallWith(null, true)).toBe(false) + }) + test("a missing engine reaches the dialog once; a second raise while it is up does not", async () => { + missingEngine() + const { a, h } = api(os.tmpdir()) + await showEngineInstallOffer(a) + expect(h.replaced).toBe(1) + expect(engineOfferInternals.visible).toBe(true) + await showEngineInstallOffer(a) + expect(h.replaced).toBe(1) + }) + test("a raise while an install is in flight never reaches the dialog", async () => { + missingEngine() + engineOfferInternals.set({ inFlight: true }) + const { a, h } = api(os.tmpdir()) + await showEngineInstallOffer(a) + expect(h.replaced).toBe(0) + expect(engineOfferInternals.visible).toBe(false) + }) + test("a null offer releases the slot, so the next raise can proceed", async () => { + syncInternals.resolveBinding = async () => null + const { a, h } = api(os.tmpdir()) + await showEngineInstallOffer(a) + expect(h.replaced).toBe(0) + expect(engineOfferInternals.visible).toBe(false) + missingEngine() + await showEngineInstallOffer(a) + expect(h.replaced).toBe(1) + }) + test("a directory that does not exist here is not this host's to install for", async () => { + let resolved = 0 + syncInternals.resolveBinding = async () => { + resolved += 1 + return binding as any + } + const { a, h } = api(path.join(os.tmpdir(), "engine-offer-not-here", String(process.pid))) + await showEngineInstallOffer(a) + expect(h.replaced).toBe(0) + expect(resolved).toBe(0) + expect(h.toasts.map((t) => t.message).join(" ")).toContain("on the server") + expect(engineOfferInternals.visible).toBe(false) + }) + test("the 7-day latch suppresses the dialog and frees the slot", async () => { + missingEngine() + const { a, h } = api(os.tmpdir()) + recordEngineSkip(a, "42", { tenant: "acme", apiUrl: "https://api.acme.example.com" }, Date.now()) + await showEngineInstallOffer(a) + expect(h.replaced).toBe(0) + expect(engineOfferInternals.visible).toBe(false) + }) +}) + describe("engine install offer — kv hydration", () => { // The store starts empty until kv.json has been read; a "Not now" latch // checked before that reads as absent. The offer waits for `ready`. diff --git a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts index 7b0035b2e..4ec56fd29 100644 --- a/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts +++ b/packages/opencode/test/altimate/workspace/engine-install-offer.test.ts @@ -216,6 +216,22 @@ describe("offer routing — engine missing", () => { clock += 1 await beforeTurn("s1") expect(h.published).toBe(3) + // …and again an hour later: the cadence holds, not just the first repeat. + clock += OFFER_RECHECK_MS + await beforeTurn("s1") + expect(h.published).toBe(4) + }) + test("a clock that moves backwards re-raises rather than waiting out a longer window", async () => { + // The TUI's latch treats a negative delta as expired; the overlay must + // agree, or the TUI would show the offer that the overlay never raises. + let clock = 1_000_000 + syncInternals.now = () => clock + const h = install({}) + await beforeTurn("s1") + expect(h.published).toBe(1) + clock -= 60_000 + await beforeTurn("s1") + expect(h.published).toBe(2) }) test("a count that arrives on a later turn does not re-raise the offer in the session", async () => { // Same verdict, better number: the dialog was already raised for it. From 4f16f178a437bb43a4e15458dc8ab17462dba117 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Tue, 1 Sep 2026 15:35:42 +0800 Subject: [PATCH 21/21] fix(workspace): keep the install's SIGKILL timers referenced; name shadowing only on a real version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An unref'd timer never fires in a process draining its loop, and the SIGKILL is the only thing that ends a descendant that ignored SIGTERM — both the deadline's escalation and the post-exit sweep stay referenced; five seconds of loop is the price. And a binary first on PATH that reports no version is not evidence of shadowing: say what was observed and leave the diagnosis to a real, older version. --- .../src/altimate/workspace/engine-offer.ts | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/engine-offer.ts b/packages/opencode/src/altimate/workspace/engine-offer.ts index 2c6e187f3..4e282e147 100644 --- a/packages/opencode/src/altimate/workspace/engine-offer.ts +++ b/packages/opencode/src/altimate/workspace/engine-offer.ts @@ -200,21 +200,24 @@ export function runInstall( killTree("SIGTERM") hard = setTimeout(() => { // The group outlives its leader while any member is alive, so this - // reaches survivors even after npm itself has exited. Unref'd: it must - // not keep the process alive once the run has already resolved. + // reaches survivors even after npm itself has exited. Referenced on + // purpose: an unref'd timer never fires in a process that is draining + // its loop, and the SIGKILL is the only thing that ends a descendant + // that ignored SIGTERM — five seconds of loop is the price. killTree("SIGKILL") finish(null) - }, graceMs).unref() + }, graceMs) }, timeoutMs) child.once("exit", (code) => { finish(code) // npm is done, but a descendant it forked may still be alive in the // group (a lifecycle script's daemon). It is the install's straggler, // not the user's: reap it — SIGTERM now, SIGKILL after the grace — - // without holding the result or this process for it. + // without holding the result for it. The timer stays referenced for + // the same reason the deadline's does: it must actually fire. if (!timedOut && grouped) { killTree("SIGTERM") - setTimeout(() => killTree("SIGKILL"), graceMs).unref() + setTimeout(() => killTree("SIGKILL"), graceMs) } }) child.once("error", (err) => { @@ -244,15 +247,23 @@ export async function installEngine(): Promise { } } const installedVersion = await versionOf(installedBin) + if (!installedVersion) { + // Could not run or did not answer: say what was observed, no diagnosis. + return { + ok: false, + error: + `npm installed it, but the ${ENGINE_BINARY} first on PATH (${installedBin}) did not report a version — ` + + `try \`${ENGINE_BINARY} --version\` there before retrying`, + } + } if (!clearsFloor(installedVersion)) { - // The likeliest cause is not a bad install: an older engine earlier on - // PATH shadows the one npm just wrote. + // A real, older version first on PATH: an earlier install shadows the + // one npm just wrote. return { ok: false, error: - `npm installed it, but the ${ENGINE_BINARY} first on PATH (${installedBin}) reports ` + - `${installedVersion ?? "no version"} — an older install earlier on PATH is shadowing the new one; ` + - `remove it, or put npm's global bin directory ahead of it`, + `npm installed it, but the ${ENGINE_BINARY} first on PATH (${installedBin}) reports ${installedVersion} — ` + + `an older install earlier on PATH is shadowing the new one; remove it, or put npm's global bin directory ahead of it`, } } return { ok: true }