fix(workspace): close the review findings on the engine overlay

- every writer of the `datamate` key loads config before asking who owns it,
  so a fresh instance's first request cannot slip past the guard; `remove`
  and `create` refuse the managed key like `add`; the SDK route answers with a
  declared 409 and a reason
- a client that predates a mid-session link is removed when the overlay
  refuses, so nothing serves the workspace under the key
- an overlay that threw is retried at the probe TTL, not every turn
- `--version` settles on the engine's exit with its own deadline
- a key set by managed preferences (MDM) is left alone
- `ALTIMATE_CODE_HEADLESS` is scrubbed from bash-tool children

Author: ralphstodomingo

packages/opencode/src/altimate/tools/datamate.ts

@@ -14,7 +14,7 @@ import { Global } from "../../global"
 import { Log } from "@/altimate/util/log"
 import { DATAMATE_KEY, readDatamateTransportFromIde } from "../datamate-transport"
 // altimate_change - workspace mode owns the datamate key
-import { managedWorkspace } from "../workspace/engine-overlay"
+import { managedWorkspaceLoaded } from "../workspace/engine-overlay"
 
 const log = Log.create({ service: "datamate" })
 
@@ -220,7 +220,7 @@ async function handleAdd(args: { datamate_id?: string; name?: string; scope?: "p
     // bound workspace's own engine, derived at config load. Adding a datamate
     // under that key would replace it; refuse and say why. Standalone
     // `datamate-<name>` entries are a different key and stay the user's.
-    const managed = serverName === DATAMATE_KEY ? managedWorkspace() : null
+    const managed = serverName === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
     if (managed) {
       return {
         title: `Datamate add: '${DATAMATE_KEY}' is managed by workspace "${managed.name}"`,
@@ -356,6 +356,23 @@ async function handleCreate(args: {
       output: "Missing required parameter 'name'.",
     }
   }
+  // altimate_change start — with an IDE transport the add that follows would go
+  // under the shared `datamate` key; in workspace mode that add is refused, so
+  // refuse here before creating an API datamate nothing would connect to.
+  if ((await readDatamateTransportFromIde(projectRoot())) !== null) {
+    const managedKey = await managedWorkspaceLoaded()
+    if (managedKey) {
+      return {
+        title: `Datamate create: '${DATAMATE_KEY}' is managed by workspace "${managedKey.name}"`,
+        metadata: { serverName: DATAMATE_KEY, managedBy: managedKey.id },
+        output:
+          `This project is linked to workspace "${managedKey.name}", whose integrations are served by the ` +
+          `workspace's own engine under the '${DATAMATE_KEY}' MCP server. Creating datamate '${args.name}' ` +
+          `here would not connect it. Unlink the project, or run without ALTIMATE_WORKSPACE, first.`,
+      }
+    }
+  }
+  // altimate_change end
   try {
     const integrations = args.integration_ids
       ? await AltimateApi.resolveIntegrations(args.integration_ids)
@@ -518,6 +535,22 @@ async function handleRemove(args: { server_name?: string; scope?: "project" | "g
         "Missing required parameter 'server_name'. Use 'status' to see active servers or 'list-config' to see saved configs.",
     }
   }
+  // altimate_change start — the workspace-managed `datamate` key is not the
+  // user's to remove either: it would stop the engine under a turn and delete
+  // the entry that unlinking hands back. Standalone `datamate-<name>` entries
+  // are unaffected.
+  const managedKey = args.server_name === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
+  if (managedKey) {
+    return {
+      title: `Datamate remove: '${DATAMATE_KEY}' is managed by workspace "${managedKey.name}"`,
+      metadata: { serverName: DATAMATE_KEY, managedBy: managedKey.id },
+      output:
+        `This project is linked to workspace "${managedKey.name}", whose integrations are served by the ` +
+        `workspace's own engine under the '${DATAMATE_KEY}' MCP server. It is not removed. Unlink the project, ` +
+        `or run without ALTIMATE_WORKSPACE, to manage that entry by hand.`,
+    }
+  }
+  // altimate_change end
   try {
     // Fully remove from runtime state (disconnect + purge from MCP list)
     // altimate_change start — MCP.remove (was disconnect): delete the status entry + publish

packages/opencode/src/altimate/workspace/engine-overlay.ts

@@ -121,6 +121,9 @@ type DirectoryState = {
    * invalidate and reload config between turns, re-running the overlay without
    * touching MCP. */
   applied: Overlay | null | undefined
+  /** When the last overlay attempt threw. A failed attempt is retried at the
+   * probe TTL, not on every turn — each retry invalidates the whole config. */
+  failedAt?: number
 }
 const directories = new Map<string, DirectoryState>()
 
@@ -142,13 +145,26 @@ function sameEntry(a: LocalMcpConfig | null, b: LocalMcpConfig | null): boolean
  * Called from the config loader after external MCP discovery, so it has the
  * last word over every other source of the key. Mutates `config.mcp` only when
  * the directory is bound with the pilot on. Never throws. */
-export async function overlay(directory: string, config: { mcp?: Record<string, unknown> }): Promise<void> {
+export async function overlay(
+  directory: string,
+  config: { mcp?: Record<string, unknown> },
+  opts: { managed?: boolean } = {},
+): Promise<void> {
   const state = stateFor(directory)
+  state.failedAt = undefined
   try {
     if (!isEnabled() || isServe()) {
       state.current = null
       return
     }
+    if (opts.managed) {
+      // Organisation-managed config (MDM) is authoritative over everything,
+      // this overlay included: the key stays as managed, and nothing here
+      // claims it, so its writers are not refused either.
+      log.info("workspace engine overlay skipped: the datamate key is set by managed preferences", { directory })
+      state.current = null
+      return
+    }
     const binding = await resolveBinding(directory)
     if (!binding) {
       // Logged because "flag on, nothing happened" is the question every
@@ -182,6 +198,7 @@ export async function overlay(directory: string, config: { mcp?: Record<string,
   } catch (err) {
     log.warn("workspace engine overlay failed; leaving the MCP config as loaded", { err: String(err) })
     state.current = null
+    state.failedAt = now()
   }
 }
 
@@ -196,6 +213,17 @@ export function managedWorkspace(directory: string | null = currentDirectory()):
   return directories.get(directory)?.current?.workspace ?? null
 }
 
+/** `managedWorkspace` once the overlay has run for this instance. The overlay
+ * runs inside config load, and on a fresh instance a writer's request can be
+ * the first thing that happens — asked before the load, the key looks free. */
+export async function managedWorkspaceLoaded(
+  directory: string | null = currentDirectory(),
+): Promise<{ id: string; name: string } | null> {
+  if (!directory) return null
+  await config().get()
+  return managedWorkspace(directory)
+}
+
 // ── per-session outcome ─────────────────────────────────────────────────────
 
 /** `retried`: this session already spent its one re-add on a failed handshake.
@@ -353,7 +381,9 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS
 
   // Reload the overlay when the binding moved, or when a refused engine may
   // have appeared since (the probe memo bounds how often that is asked).
-  let reload = !state.current || state.current.workspace.id !== workspaceId
+  let reload = state.current
+    ? state.current.workspace.id !== workspaceId
+    : state.failedAt === undefined || now() - state.failedAt >= FAILED_PROBE_TTL_MS
   if (!reload && state.current && !state.current.entry) {
     const probe = await probeEngine()
     reload = probe.kind === "ok"
@@ -377,7 +407,10 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS
   // derived entry changed, drop it when there is none any more.
   if (overlayNow.entry) {
     if (!sameEntry(state.applied?.entry ?? null, overlayNow.entry)) await mcp().add(DATAMATE_KEY, overlayNow.entry)
-  } else if (state.applied?.entry) {
+  } else if (state.applied?.entry || DATAMATE_KEY in (await mcp().status())) {
+    // Ours to drop — or a client that predates the link, which MCP bootstrapped
+    // from an IDE or hosted entry while the directory was unbound. With the
+    // overlay refusing, nothing may serve the workspace under the key.
     await mcp().remove(DATAMATE_KEY)
   }
   state.applied = overlayNow
@@ -480,7 +513,8 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS
 export async function announceRefusal(sessionID: string, outcome: Outcome, toast: Toast): Promise<void> {
   const rec = sessions.get(sessionID) ?? record(sessionID, outcome)
   const detail = "error" in outcome ? outcome.error : "found" in outcome ? String(outcome.found) : ""
-  const signature = `${outcome.kind}:${detail}:${toast.title}`
+  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 (isHeadless()) {

packages/opencode/src/altimate/workspace/engine-probes.ts

@@ -35,26 +35,48 @@ export function which(cmd: string): string | null {
  *
  * cross-spawn, not execFile: an npm-installed engine on Windows resolves to a
  * `.cmd` shim that Node cannot execute without a shell. */
+/** How long `--version` may take before the engine counts as unreadable. */
+export const VERSION_TIMEOUT_MS = 5_000
+
 export function versionOf(bin: string): Promise<string | null> {
   if (syncInternals.versionOf) return syncInternals.versionOf(bin)
   return new Promise((resolve) => {
     let settled = false
+    let timer: ReturnType<typeof setTimeout> | undefined
     const done = (value: string | null) => {
       if (settled) return
       settled = true
+      if (timer) clearTimeout(timer)
       resolve(value)
     }
     try {
-      const child = launch(bin, ["--version"], { stdio: ["ignore", "pipe", "ignore"], timeout: 5000 })
+      const child = launch(bin, ["--version"], { stdio: ["ignore", "pipe", "ignore"] })
       let out = ""
       child.stdout?.on("data", (chunk) => {
         out += String(chunk)
       })
+      // Settle on `exit`, not `close`: a descendant that inherited stdout would
+      // keep `close` from firing after the engine itself has answered. The
+      // deadline is ours as well — the runtime's `timeout` only signals the
+      // direct child, so it could not end a wait on a straggler's pipe.
+      timer = setTimeout(() => {
+        try {
+          child.kill("SIGKILL")
+        } catch {
+          // Already gone.
+        }
+        child.stdout?.destroy()
+        done(null)
+      }, VERSION_TIMEOUT_MS)
       child.on("error", () => done(null))
-      child.on("close", (code) => {
-        if (code !== 0) return done(null)
-        const line = out.trim().split(/\r?\n/)[0] ?? ""
-        done(line || null)
+      child.on("exit", (code) => {
+        // Let any bytes still in flight land before reading `out`.
+        setImmediate(() => {
+          child.stdout?.destroy()
+          if (code !== 0) return done(null)
+          const line = out.trim().split(/\r?\n/)[0] ?? ""
+          done(line || null)
+        })
       })
     } catch {
       done(null)

packages/opencode/src/config/config.ts

@@ -33,6 +33,9 @@ import { ConfigPluginV1 } from "@opencode-ai/core/v1/config/plugin"
 import { ConfigAgent } from "./agent"
 import { ConfigCommand } from "./command"
 import { ConfigManaged } from "./managed"
+// altimate_change start — the workspace engine overlay yields to managed config for this key
+import { DATAMATE_KEY } from "../altimate/datamate-transport"
+// altimate_change end
 import { ConfigParse } from "./parse"
 import { ConfigPaths } from "./paths"
 import { ConfigPlugin } from "./plugin"
@@ -641,6 +644,9 @@ export const layer = Layer.effect(
           )
         }
 
+        // altimate_change start — whether organisation-managed config sets the datamate MCP key
+        let managedOwnsDatamate = false
+        // altimate_change end
         const managedDir = ConfigManaged.managedConfigDir()
         if (existsSync(managedDir)) {
           // altimate_change start - support altimate-code.json config filename
@@ -654,20 +660,25 @@ export const layer = Layer.effect(
           ]) {
             // altimate_change end
             const source = path.join(managedDir, file)
-            yield* merge(source, yield* loadFile(source), "global")
+            // altimate_change start — note a managed datamate key before merging
+            const managedFile = yield* loadFile(source)
+            if (managedFile?.mcp && DATAMATE_KEY in managedFile.mcp) managedOwnsDatamate = true
+            yield* merge(source, managedFile, "global")
+            // altimate_change end
           }
         }
 
         // macOS managed preferences (.mobileconfig deployed via MDM) override everything
         const managed = yield* Effect.promise(() => ConfigManaged.readManagedPreferences())
         if (managed) {
-          result = mergeConfigConcatArrays(
-            result,
-            yield* loadConfig(managed.text, {
-              dir: path.dirname(managed.source),
-              source: managed.source,
-            }),
-          )
+          // altimate_change start — note a managed datamate key before merging
+          const managedPrefs = yield* loadConfig(managed.text, {
+            dir: path.dirname(managed.source),
+            source: managed.source,
+          })
+          if (managedPrefs.mcp && DATAMATE_KEY in managedPrefs.mcp) managedOwnsDatamate = true
+          result = mergeConfigConcatArrays(result, managedPrefs)
+          // altimate_change end
         }
 
         for (const [name, mode] of Object.entries(result.mode ?? {})) {
@@ -747,7 +758,9 @@ export const layer = Layer.effect(
         // to any file. See altimate/workspace/engine-overlay.ts.
         if (Flag.ALTIMATE_WORKSPACE) {
           const { overlay } = yield* Effect.promise(() => import("../altimate/workspace/engine-overlay"))
-          yield* Effect.promise(() => overlay(ctx.directory, result as { mcp?: Record<string, unknown> }))
+          yield* Effect.promise(() =>
+            overlay(ctx.directory, result as { mcp?: Record<string, unknown> }, { managed: managedOwnsDatamate }),
+          )
         }
         // altimate_change end
 

packages/opencode/src/server/routes/instance/httpapi/groups/mcp.ts

@@ -28,6 +28,13 @@ export class UnsupportedOAuthError extends Schema.ErrorClass<UnsupportedOAuthErr
   { error: Schema.String },
   { httpApiStatus: 400 },
 ) {}
+// altimate_change start — workspace mode owns the `datamate` key; an add over it
+// is refused with a reason the SDK caller can show, not a bare 400.
+export class McpServerManagedError extends Schema.ErrorClass<McpServerManagedError>("McpServerManagedError")(
+  { error: Schema.String },
+  { httpApiStatus: 409 },
+) {}
+// altimate_change end
 
 export const McpPaths = {
   status: "/mcp",
@@ -56,7 +63,9 @@ export const McpApi = HttpApi.make("mcp")
           query: WorkspaceRoutingQuery,
           payload: AddPayload,
           success: described(StatusMap, "MCP server added successfully"),
-          error: HttpApiError.BadRequest,
+          // altimate_change start — the workspace-managed refusal is a declared error
+          error: [HttpApiError.BadRequest, McpServerManagedError],
+          // altimate_change end
         }).annotateMerge(
           OpenApi.annotations({
             identifier: "mcp.add",

packages/opencode/src/server/routes/instance/httpapi/handlers/mcp.ts

@@ -6,13 +6,18 @@ import { McpServerNotFoundError } from "../errors"
 import { AddPayload, AuthCallbackPayload, StatusMap, UnsupportedOAuthError } from "../groups/mcp"
 // altimate_change start — workspace mode owns the datamate key
 import { InstanceState } from "@/effect/instance-state"
+import { Config } from "@/config/config"
 import { DATAMATE_KEY } from "@/altimate/datamate-transport"
 import { managedWorkspace } from "@/altimate/workspace/engine-overlay"
+import { McpServerManagedError } from "../groups/mcp"
 // altimate_change end
 
 export const mcpHandlers = HttpApiBuilder.group(InstanceHttpApi, "mcp", (handlers) =>
   Effect.gen(function* () {
     const mcp = yield* MCP.Service
+    // altimate_change start — config is loaded before the managed-key check
+    const configSvc = yield* Config.Service
+    // altimate_change end
 
     const status = Effect.fn("McpHttpApi.status")(function* () {
       return yield* mcp.status()
@@ -23,14 +28,18 @@ export const mcpHandlers = HttpApiBuilder.group(InstanceHttpApi, "mcp", (handler
       // workspace's own engine, derived at config load; adding over it would replace
       // the engine underneath a turn. Refuse and say why.
       if (ctx.payload.name === DATAMATE_KEY) {
+        // The overlay runs inside config load; on a fresh instance this can be
+        // the first request, so load before asking who owns the key.
+        yield* configSvc.get()
         const managed = managedWorkspace(yield* InstanceState.directory)
         if (managed) {
-          // BadRequest carries no body on this endpoint; the reason is logged.
           yield* Effect.logWarning("mcp add refused: key is managed by a workspace", {
             name: DATAMATE_KEY,
             workspace: managed.id,
           })
-          return yield* new HttpApiError.BadRequest({})
+          return yield* new McpServerManagedError({
+            error: `MCP server "${DATAMATE_KEY}" is managed by workspace "${managed.name}" in this project`,
+          })
         }
       }
       // altimate_change end

packages/opencode/src/server/routes/mcp.ts

@@ -4,7 +4,7 @@ import z from "zod"
 import { MCP } from "../../mcp"
 // altimate_change start — workspace mode owns the datamate key
 import { DATAMATE_KEY } from "../../altimate/datamate-transport"
-import { managedWorkspace } from "../../altimate/workspace/engine-overlay"
+import { managedWorkspaceLoaded } from "../../altimate/workspace/engine-overlay"
 // altimate_change end
 // altimate_change start — Config.Mcp + MCP.Status migrated to Effect Schema in v1.17.9; convert to zod for HTTP schemas
 import { ConfigMCPV1 } from "@opencode-ai/core/v1/config/mcp"
@@ -64,7 +64,7 @@ export const McpRoutes = lazy(() =>
       async (c) => {
         const { name, config } = c.req.valid("json")
         // altimate_change start — workspace mode owns the `datamate` key
-        const managed = name === DATAMATE_KEY ? managedWorkspace() : null
+        const managed = name === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
         if (managed) {
           return c.json(
             { error: `MCP server "${name}" is managed by workspace "${managed.name}" in this project` },

packages/opencode/src/server/server.ts

@@ -35,7 +35,7 @@ import { MCP } from "../mcp"
 // Using datamate-transport.ts instead of serve.ts avoids a dep on a cmd handler.
 import { syncDatamateUrlFromVscodeMcp } from "../altimate/datamate-transport"
 // altimate_change - workspace mode owns the datamate key
-import { managedWorkspace } from "../altimate/workspace/engine-overlay"
+import { managedWorkspaceLoaded } from "../altimate/workspace/engine-overlay"
 import { readMcpEntryFromDisk } from "../mcp/config"
 import { resolveConfigPath } from "../mcp/config"
 import { enhancePrompt, isAutoEnhanceEnabled } from "../altimate/enhance-prompt"
@@ -689,7 +689,7 @@ export namespace Server {
           const directory = Instance.directory
           // In workspace mode the `datamate` key is the bound workspace's own engine,
           // derived at config load; an IDE reload must not replace it under a turn.
-          const managed = managedWorkspace()
+          const managed = await managedWorkspaceLoaded()
           if (managed) {
             log.info("reload-datamate: refused, key is managed by a workspace", { workspace: managed.id })
             return c.json(

packages/opencode/src/tool/bash.ts

@@ -176,6 +176,11 @@ export const BashTool = Tool.define("bash", async () => {
       // process.env spread above would silently disable that path in every
       // nested server invocation. See PR #937 review (Issue #3).
       delete mergedEnv["ALTIMATE_NON_INTERACTIVE"]
+      // Same reasoning for the headless marker: `run` sets it so the workspace
+      // engine's refusals degrade to a printed line, but a nested entrypoint
+      // launched from here may well have a TUI. Left in place, the child would
+      // inherit "headless" and print to stderr instead of showing its surface.
+      delete mergedEnv["ALTIMATE_CODE_HEADLESS"]
       // altimate_change end
       const sep = process.platform === "win32" ? ";" : ":"
       const basePath = mergedEnv.PATH ?? mergedEnv.Path ?? ""