Skip to content

Commit b2e5c6a

Browse files
author
ralphstodomingo
committed
fix(workspace): guard connect/disconnect; keep the engine through a transient fault
The SDK and HTTP connect/disconnect routes refuse the workspace-managed key before `MCP` would restart or close it and persist `enabled`. The add guard runs before the API lookup; create/remove preflights sit inside their error boundaries. A managed-preferences key settles as disabled with no per-turn reload; an overlay that throws after attach keeps the running engine. The headless env scrub is tested.
1 parent 72dcae1 commit b2e5c6a

7 files changed

Lines changed: 225 additions & 61 deletions

File tree

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

Lines changed: 52 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,30 @@ async function handleAdd(args: { datamate_id?: string; name?: string; scope?: "p
191191
}
192192
}
193193
try {
194-
const datamate = await AltimateApi.getDatamate(args.datamate_id)
195194
// readDatamateTransportFromIde returns the exact command from the IDE config so we
196195
// reuse the same process the extension already manages, not a second one.
197196
const transport = await readDatamateTransportFromIde(projectRoot())
198197

198+
// altimate_change start — in workspace mode the shared `datamate` key is the
199+
// bound workspace's own engine, derived at config load. With an IDE transport
200+
// the add would go under that key; refuse and say why, before anything is
201+
// looked up — the refusal must not depend on the API being reachable.
202+
// Standalone `datamate-<name>` entries are a different key and stay the user's.
203+
const managed = transport !== null ? await managedWorkspaceLoaded() : null
204+
if (managed) {
205+
return {
206+
title: `Datamate add: '${DATAMATE_KEY}' is managed by workspace "${managed.name}"`,
207+
metadata: { serverName: DATAMATE_KEY, managedBy: managed.id, datamateId: args.datamate_id },
208+
output:
209+
`This project is linked to workspace "${managed.name}", whose integrations are served by the ` +
210+
`workspace's own engine under the '${DATAMATE_KEY}' MCP server. Adding datamate '${args.datamate_id}' ` +
211+
`there is not applied. Unlink the project, or run without ALTIMATE_WORKSPACE, to manage that entry by hand.`,
212+
}
213+
}
214+
// altimate_change end
215+
216+
const datamate = await AltimateApi.getDatamate(args.datamate_id)
217+
199218
if (transport !== null) {
200219
log.info("handleAdd: IDE transport detected, entering single-gateway mode", {
201220
serverName: DATAMATE_KEY,
@@ -216,23 +235,6 @@ async function handleAdd(args: { datamate_id?: string; name?: string; scope?: "p
216235
? DATAMATE_KEY
217236
: (args.name ?? `datamate-${slugify(datamate.name)}`)
218237

219-
// altimate_change start — in workspace mode the shared `datamate` key is the
220-
// bound workspace's own engine, derived at config load. Adding a datamate
221-
// under that key would replace it; refuse and say why. Standalone
222-
// `datamate-<name>` entries are a different key and stay the user's.
223-
const managed = serverName === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
224-
if (managed) {
225-
return {
226-
title: `Datamate add: '${DATAMATE_KEY}' is managed by workspace "${managed.name}"`,
227-
metadata: { serverName: DATAMATE_KEY, managedBy: managed.id, datamateId: args.datamate_id },
228-
output:
229-
`This project is linked to workspace "${managed.name}", whose integrations are served by the ` +
230-
`workspace's own engine under the '${DATAMATE_KEY}' MCP server. Adding datamate '${datamate.name}' ` +
231-
`there is not applied. Unlink the project, or run without ALTIMATE_WORKSPACE, to manage that entry by hand.`,
232-
}
233-
}
234-
// altimate_change end
235-
236238
const creds = transport ? undefined : await AltimateApi.getCredentials()
237239
const mcpConfig =
238240
transport?.type === "remote"
@@ -356,24 +358,24 @@ async function handleCreate(args: {
356358
output: "Missing required parameter 'name'.",
357359
}
358360
}
359-
// altimate_change start — with an IDE transport the add that follows would go
360-
// under the shared `datamate` key; in workspace mode that add is refused, so
361-
// refuse here before creating an API datamate nothing would connect to.
362-
if ((await readDatamateTransportFromIde(projectRoot())) !== null) {
363-
const managedKey = await managedWorkspaceLoaded()
364-
if (managedKey) {
365-
return {
366-
title: `Datamate create: '${DATAMATE_KEY}' is managed by workspace "${managedKey.name}"`,
367-
metadata: { serverName: DATAMATE_KEY, managedBy: managedKey.id },
368-
output:
369-
`This project is linked to workspace "${managedKey.name}", whose integrations are served by the ` +
370-
`workspace's own engine under the '${DATAMATE_KEY}' MCP server. Creating datamate '${args.name}' ` +
371-
`here would not connect it. Unlink the project, or run without ALTIMATE_WORKSPACE, first.`,
361+
try {
362+
// altimate_change start — with an IDE transport the add that follows would go
363+
// under the shared `datamate` key; in workspace mode that add is refused, so
364+
// refuse here before creating an API datamate nothing would connect to.
365+
if ((await readDatamateTransportFromIde(projectRoot())) !== null) {
366+
const managedKey = await managedWorkspaceLoaded()
367+
if (managedKey) {
368+
return {
369+
title: `Datamate create: '${DATAMATE_KEY}' is managed by workspace "${managedKey.name}"`,
370+
metadata: { serverName: DATAMATE_KEY, managedBy: managedKey.id },
371+
output:
372+
`This project is linked to workspace "${managedKey.name}", whose integrations are served by the ` +
373+
`workspace's own engine under the '${DATAMATE_KEY}' MCP server. Creating datamate '${args.name}' ` +
374+
`here would not connect it. Unlink the project, or run without ALTIMATE_WORKSPACE, first.`,
375+
}
372376
}
373377
}
374-
}
375-
// altimate_change end
376-
try {
378+
// altimate_change end
377379
const integrations = args.integration_ids
378380
? await AltimateApi.resolveIntegrations(args.integration_ids)
379381
: undefined
@@ -535,23 +537,23 @@ async function handleRemove(args: { server_name?: string; scope?: "project" | "g
535537
"Missing required parameter 'server_name'. Use 'status' to see active servers or 'list-config' to see saved configs.",
536538
}
537539
}
538-
// altimate_change start — the workspace-managed `datamate` key is not the
539-
// user's to remove either: it would stop the engine under a turn and delete
540-
// the entry that unlinking hands back. Standalone `datamate-<name>` entries
541-
// are unaffected.
542-
const managedKey = args.server_name === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
543-
if (managedKey) {
544-
return {
545-
title: `Datamate remove: '${DATAMATE_KEY}' is managed by workspace "${managedKey.name}"`,
546-
metadata: { serverName: DATAMATE_KEY, managedBy: managedKey.id },
547-
output:
548-
`This project is linked to workspace "${managedKey.name}", whose integrations are served by the ` +
549-
`workspace's own engine under the '${DATAMATE_KEY}' MCP server. It is not removed. Unlink the project, ` +
550-
`or run without ALTIMATE_WORKSPACE, to manage that entry by hand.`,
551-
}
552-
}
553-
// altimate_change end
554540
try {
541+
// altimate_change start — the workspace-managed `datamate` key is not the
542+
// user's to remove either: it would stop the engine under a turn and delete
543+
// the entry that unlinking hands back. Standalone `datamate-<name>` entries
544+
// are unaffected.
545+
const managedKey = args.server_name === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
546+
if (managedKey) {
547+
return {
548+
title: `Datamate remove: '${DATAMATE_KEY}' is managed by workspace "${managedKey.name}"`,
549+
metadata: { serverName: DATAMATE_KEY, managedBy: managedKey.id },
550+
output:
551+
`This project is linked to workspace "${managedKey.name}", whose integrations are served by the ` +
552+
`workspace's own engine under the '${DATAMATE_KEY}' MCP server. It is not removed. Unlink the project, ` +
553+
`or run without ALTIMATE_WORKSPACE, to manage that entry by hand.`,
554+
}
555+
}
556+
// altimate_change end
555557
// Fully remove from runtime state (disconnect + purge from MCP list)
556558
// altimate_change start — MCP.remove (was disconnect): delete the status entry + publish
557559
// ToolsChanged so the removed server's tools stop being offered without a restart.

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ type DirectoryState = {
124124
/** When the last overlay attempt threw. A failed attempt is retried at the
125125
* probe TTL, not on every turn — each retry invalidates the whole config. */
126126
failedAt?: number
127+
/** The key is set by organisation-managed config: nothing here claims it. */
128+
managed?: boolean
127129
}
128130
const directories = new Map<string, DirectoryState>()
129131

@@ -152,6 +154,7 @@ export async function overlay(
152154
): Promise<void> {
153155
const state = stateFor(directory)
154156
state.failedAt = undefined
157+
state.managed = opts.managed === true
155158
try {
156159
if (!isEnabled() || isServe()) {
157160
state.current = null
@@ -362,6 +365,15 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS
362365
// the overlay as it stands now.
363366
if (state.applied === undefined) state.applied = state.current
364367

368+
// Organisation-managed config owns the key: the feature is off for this
369+
// directory, whatever the binding says. Nothing to reload per turn.
370+
if (state.managed) {
371+
if (state.applied?.entry) await releaseKey(await config().get(), true)
372+
state.applied = null
373+
record(sessionID, { kind: "disabled" })
374+
return
375+
}
376+
365377
const binding = await resolveBinding(directory)
366378
if (!binding) {
367379
// Unlinked (or never linked): the key is not ours to fill.
@@ -394,7 +406,9 @@ async function reconcile(sessionID: string, directory: string, state: DirectoryS
394406
loaded = await config().get()
395407
}
396408

397-
const overlayNow = state.current
409+
// A transient overlay failure (its retry is throttled above) keeps what was
410+
// last applied: a running engine is not released over a fault in the probe.
411+
const overlayNow = state.current ?? (state.failedAt !== undefined ? (state.applied ?? null) : null)
398412
if (!overlayNow) {
399413
if (state.applied) await releaseKey(loaded, !!state.applied.entry)
400414
state.applied = null

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,9 @@ export const McpApi = HttpApi.make("mcp")
127127
params: { name: Schema.String },
128128
query: WorkspaceRoutingQuery,
129129
success: described(Schema.Boolean, "MCP server connected successfully"),
130-
error: McpServerNotFoundError,
130+
// altimate_change start — the workspace-managed key refuses connect/disconnect
131+
error: [McpServerNotFoundError, McpServerManagedError],
132+
// altimate_change end
131133
}).annotateMerge(
132134
OpenApi.annotations({
133135
identifier: "mcp.connect",
@@ -138,7 +140,9 @@ export const McpApi = HttpApi.make("mcp")
138140
params: { name: Schema.String },
139141
query: WorkspaceRoutingQuery,
140142
success: described(Schema.Boolean, "MCP server disconnected successfully"),
141-
error: McpServerNotFoundError,
143+
// altimate_change start — the workspace-managed key refuses connect/disconnect
144+
error: [McpServerNotFoundError, McpServerManagedError],
145+
// altimate_change end
142146
}).annotateMerge(
143147
OpenApi.annotations({
144148
identifier: "mcp.disconnect",

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,28 @@ export const mcpHandlers = HttpApiBuilder.group(InstanceHttpApi, "mcp", (handler
101101
return { success: true as const }
102102
})
103103

104+
// altimate_change start — connect/disconnect persist `enabled` for the key and
105+
// restart or close its client: neither may touch the workspace-managed
106+
// `datamate`, which is derived per process and never written to a file.
107+
const refuseManaged = Effect.fn("McpHttpApi.refuseManaged")(function* (name: string) {
108+
if (name !== DATAMATE_KEY) return
109+
yield* configSvc.get()
110+
const managed = managedWorkspace(yield* InstanceState.directory)
111+
if (!managed) return
112+
yield* Effect.logWarning("mcp connect/disconnect refused: key is managed by a workspace", {
113+
name: DATAMATE_KEY,
114+
workspace: managed.id,
115+
})
116+
return yield* new McpServerManagedError({
117+
error: `MCP server "${DATAMATE_KEY}" is managed by workspace "${managed.name}" in this project`,
118+
})
119+
})
120+
// altimate_change end
121+
104122
const connect = Effect.fn("McpHttpApi.connect")(function* (ctx: { params: { name: string } }) {
123+
// altimate_change start
124+
yield* refuseManaged(ctx.params.name)
125+
// altimate_change end
105126
yield* mcp
106127
.connect(ctx.params.name)
107128
.pipe(
@@ -115,6 +136,9 @@ export const mcpHandlers = HttpApiBuilder.group(InstanceHttpApi, "mcp", (handler
115136
})
116137

117138
const disconnect = Effect.fn("McpHttpApi.disconnect")(function* (ctx: { params: { name: string } }) {
139+
// altimate_change start
140+
yield* refuseManaged(ctx.params.name)
141+
// altimate_change end
118142
yield* mcp
119143
.disconnect(ctx.params.name)
120144
.pipe(

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,15 @@ export const McpRoutes = lazy(() =>
215215
validator("param", z.object({ name: z.string() })),
216216
async (c) => {
217217
const { name } = c.req.valid("param")
218+
// altimate_change start — the workspace-managed key is not restarted or persisted from here
219+
const managed = name === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
220+
if (managed) {
221+
return c.json(
222+
{ error: `MCP server "${name}" is managed by workspace "${managed.name}" in this project` },
223+
409,
224+
)
225+
}
226+
// altimate_change end
218227
await MCP.connect(name)
219228
return c.json(true)
220229
},
@@ -238,6 +247,15 @@ export const McpRoutes = lazy(() =>
238247
validator("param", z.object({ name: z.string() })),
239248
async (c) => {
240249
const { name } = c.req.valid("param")
250+
// altimate_change start — the workspace-managed key is not closed or persisted from here
251+
const managed = name === DATAMATE_KEY ? await managedWorkspaceLoaded() : null
252+
if (managed) {
253+
return c.json(
254+
{ error: `MCP server "${name}" is managed by workspace "${managed.name}" in this project` },
255+
409,
256+
)
257+
}
258+
// altimate_change end
241259
await MCP.disconnect(name)
242260
return c.json(true)
243261
},

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

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ function install(opts: {
7070
tools?: Record<string, unknown>
7171
mcp?: Record<string, unknown>
7272
noMcpKey?: boolean
73+
managed?: boolean
7374
}): Harness {
7475
const h: Harness = {
7576
config: opts.noMcpKey ? {} : { mcp: opts.mcp ?? {} },
@@ -138,7 +139,7 @@ function install(opts: {
138139
h.gets += 1
139140
if (loaded) return h.config
140141
h.config = opts.noMcpKey ? {} : { mcp: structuredClone(initialMcp ?? {}) }
141-
await overlay(DIR, h.config)
142+
await overlay(DIR, h.config, { managed: opts.managed === true })
142143
// MCP bootstraps from the config as first loaded — after the overlay had
143144
// its say — and keeps whatever client that started until told otherwise.
144145
if (h.live === undefined) h.live = DATAMATE_KEY in (h.config.mcp ?? {})
@@ -492,11 +493,42 @@ describe("beforeTurn — what a turn boundary does", () => {
492493
})
493494

494495
test("a datamate key set by managed preferences is left alone and is not managed here", async () => {
495-
const h = install({ mcp: { datamate: IDE_ENTRY } })
496-
await overlay(DIR, h.config, { managed: true })
496+
const h = install({ mcp: { datamate: IDE_ENTRY }, managed: true })
497+
await beforeTurn("s1")
498+
await beforeTurn("s1")
497499
expect(h.config.mcp).toEqual({ datamate: IDE_ENTRY })
498500
expect(managedWorkspace()).toBeNull()
499501
expect(h.probes).toBe(0)
502+
// The feature is off here: no per-turn reload, no toast, nothing removed.
503+
expect(h.invalidates).toBe(0)
504+
expect(h.removes).toBe(0)
505+
expect(h.toasts).toEqual([])
506+
expect(settledOutcome("s1")).toEqual({ kind: "disabled" })
507+
})
508+
509+
test("a transient overlay failure after attach keeps the running engine", async () => {
510+
const h = install({})
511+
// MCP bootstrapped the engine from the config as loaded; the hook adds nothing.
512+
await beforeTurn("s1")
513+
expect(h.added).toHaveLength(0)
514+
expect(settledOutcome("s1")?.kind).toBe("attached")
515+
// Something else invalidates config, and the overlay's probe now throws.
516+
syncInternals.which = () => {
517+
throw new Error("PATH unreadable")
518+
}
519+
await syncInternals.config!.invalidate()
520+
await beforeTurn("s1")
521+
expect(h.removes).toBe(0)
522+
expect(h.added).toHaveLength(0)
523+
expect(settledOutcome("s1")?.kind).toBe("attached")
524+
// The fault clears and the TTL passes: still the same engine, not a second one.
525+
h.which = "/usr/local/bin/datamate"
526+
syncInternals.which = () => h.which
527+
h.clock += FAILED_PROBE_TTL_MS
528+
await beforeTurn("s1")
529+
expect(h.added).toHaveLength(0)
530+
expect(h.removes).toBe(0)
531+
expect(settledOutcome("s1")?.kind).toBe("attached")
500532
})
501533

502534
test("an unlink hands the key back to the entry the reloaded config restores", async () => {

0 commit comments

Comments
 (0)