fix: list and count extension-type integrations when a live IDE bridge serves them #1236
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ | |
| // | ||
| // Everything that asks the outside world a question: the binary, its | ||
| // version, the workspace allowlist, and the user-facing surfaces. | ||
| import { readFileSync, readdirSync, statSync } from "fs" | ||
| import { homedir } from "os" | ||
| import { isAbsolute, join, relative, resolve, sep } from "path" | ||
| import launch from "cross-spawn" | ||
| import { which as whichBinary } from "@opencode-ai/core/util/which" | ||
| import { AltimateApi } from "@/altimate/api/client" | ||
|
|
@@ -164,6 +166,85 @@ export async function declaredBounded(workspaceId: string): Promise<Declared | n | |
| } | ||
| } | ||
|
|
||
| /** Whether a live VS Code bridge would serve `cwd`, resolved the way the | ||
| * engine resolves it at spawn (see the engine's extensionRpcDiscovery): a | ||
| * sidecar whose recorded workspaceFolders contain `cwd`, else the sole live | ||
| * bridge. Read-only — a dead pid is skipped, never unlinked; GC of stale | ||
| * sidecars belongs to the engine and the extension. Presentation only: the | ||
| * engine remains the authority on what actually connects. */ | ||
| export function liveBridge(cwd: string, dir: string = join(homedir(), ".altimate", "extension-rpc")): boolean { | ||
| if (syncInternals.liveBridge) return syncInternals.liveBridge(cwd) | ||
| const bridges: string[][] = [] | ||
| try { | ||
| for (const entry of readdirSync(dir)) { | ||
| if (!entry.endsWith(".json")) continue | ||
| try { | ||
| const data = JSON.parse(readFileSync(join(dir, entry), "utf8")) as { | ||
| socketPath?: string | ||
| workspaceFolders?: string[] | ||
| pid?: number | ||
| } | ||
| if (typeof data.socketPath !== "string" || !data.socketPath) continue | ||
| // A sidecar without a pid counts as live, matching the engine's own | ||
| // discovery. A PRESENT pid must be a live real process: the bridge | ||
| // extension always writes a positive integer, so a string, null, or | ||
| // non-positive value is a corrupt record, not a legacy shape — and | ||
| // unlike the engine, this probe has no connection attempt behind it | ||
| // to catch a bad guess. kill(0)/kill(-1) probe process groups, which | ||
| // would read garbage pids as alive. (codex r3, cubic) | ||
| if ("pid" in data && !(typeof data.pid === "number" && Number.isInteger(data.pid) && data.pid > 0 && pidAlive(data.pid))) | ||
| continue | ||
| // Validate the folders shape: this is an unvalidated JSON file, and a | ||
| // non-array must degrade to "live bridge, no recorded folders", not | ||
| // throw out of the probe. Only fully qualified strings survive — | ||
| // anything resolve() would complete from the process's own cwd or | ||
| // drive could spuriously match and bypass the two-bridge decline. | ||
| // (codex r3+r4) | ||
| const folders = Array.isArray(data.workspaceFolders) | ||
| ? data.workspaceFolders.filter((f): f is string => typeof f === "string" && qualifiedFolder(f)) | ||
| : [] | ||
|
Comment on lines
+203
to
+205
There was a problem hiding this comment.
The new shape filter accepts empty and relative strings even though they are not valid recorded workspace-folder paths. Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Confirmed and fixed in 179e433: folder entries are filtered to nonempty absolute strings, so resolve("") can no longer alias the process cwd or defeat the two-bridge decline. Test covers the exact two-sidecar scenario. |
||
| bridges.push(folders) | ||
| } catch { | ||
| // An unreadable sidecar is not a live bridge. | ||
| } | ||
| } | ||
| } catch { | ||
| return false | ||
| } | ||
| if (bridges.length === 0) return false | ||
| const within = (folder: string) => { | ||
| const rel = relative(resolve(folder), resolve(cwd)) | ||
|
There was a problem hiding this comment. P2: Canonicalize the recorded folder and Prompt for AI agents |
||
| // ".." must be a complete path component: a child literally named | ||
| // "..cache" yields rel "..cache", which is inside. (bot review) | ||
| return rel === "" || (rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel)) | ||
| } | ||
| if (bridges.some((folders) => folders.some(within))) return true | ||
| return bridges.length === 1 | ||
| } | ||
|
|
||
| /** A recorded folder must be fully qualified. On Windows, drive-relative | ||
| * paths like "\repo" count as absolute to Node, but resolve() completes them | ||
| * with the process's CURRENT drive — so a corrupt entry could match any cwd | ||
| * on that drive and defeat the two-bridge decline. Drive-qualified (C:\ or | ||
| * C:/) or complete UNC only: a UNC value needs nonempty server AND share | ||
| * components — resolve("\\\\") is "C:\\" and resolve("\\\\server") is | ||
| * "C:\\server", both on the current drive again. POSIX keeps plain | ||
| * isAbsolute. The platform parameter exists for tests. (codex r4+r5) */ | ||
| export function qualifiedFolder(f: string, win: boolean = process.platform === "win32"): boolean { | ||
| if (!f) return false | ||
| return win ? /^([a-zA-Z]:[\\/]|[\\/]{2}[^\\/]+[\\/]+[^\\/]+)/.test(f) : isAbsolute(f) | ||
|
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: bun -e 'import { isAbsolute, posix } from "node:path"; if (isAbsolute("\\repo") !== true || posix.isAbsolute("\\repo") !== false || isAbsolute("C:\\ws") !== true || posix.isAbsolute("C:\\ws") !== false) process.exit(1)'Repository: AltimateAI/altimate-code Length of output: 203 🏁 Script executed: sed -n '1,80p;190,255p' packages/opencode/src/altimate/workspace/engine-probes.ts
printf '\n--- changed-file references ---\n'
rg -n "qualifiedFolder|isAbsolute|win" packages/opencode/src/altimate/workspace/engine-probes.tsRepository: AltimateAI/altimate-code Length of output: 7689 🌐 Web query:
💡 Result: In Node.js, the path.isAbsolute method determines if a path is absolute by checking the literal string format [1][2]. On Windows, this method returns true for paths that are considered "absolute" by the Node.js path module's logic, which includes paths that start with a drive letter (e.g., C:\ws) or those starting with a UNC server/share path (e.g., \server) [1][3][4]. Regarding root-relative paths on Windows (e.g., \repo), Node.js currently treats these as absolute in the context of path.isAbsolute [5]. While standard Windows file system APIs classify paths that start with a directory separator but lack a drive letter as "relative to the root of the current drive" (not fully absolute) [5], the Node.js implementation of path.isAbsolute returns true for these strings [5]. This is a known ambiguity in the Node.js path module, as it does not implement a distinct "drive-relative" category to align with Windows' three-tier classification of paths (relative, drive-relative, and absolute) [5]. Therefore, if you provide the string "\repo" to path.isAbsolute on Windows, it will return true [5]. Similarly, a drive-qualified path like "C:\ws" is correctly identified as absolute and will return true [1][3]. For developers needing to handle drive-relative paths safely, relying solely on path.isAbsolute may be insufficient as it does not distinguish between fully qualified absolute paths and those anchored to the current drive's root [5]. Citations:
Use POSIX path semantics when
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
|
|
||
| function pidAlive(pid: number): boolean { | ||
| try { | ||
| process.kill(pid, 0) | ||
| return true | ||
| } catch (err) { | ||
| // EPERM is a live process owned by someone else. | ||
| return (err as NodeJS.ErrnoException).code === "EPERM" | ||
| } | ||
| } | ||
|
|
||
| export async function notify(toast: Toast): Promise<void> { | ||
| if (syncInternals.notify) return syncInternals.notify(toast) | ||
| try { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| // altimate_change - new file | ||
| // | ||
| // `datamate_manager list-integrations` and extension-type rows. Extension | ||
| // integrations are RPC into a live VS Code host: with a bridge running for | ||
| // this project they serve from the CLI like any other integration; without | ||
| // one they are dormant, not impossible. The listing must say which of those | ||
| // is true rather than a blanket "not available from the CLI". | ||
| import { afterEach, describe, expect, test } from "bun:test" | ||
| import { initTool } from "../tool-fixture" | ||
| import { tmpdir } from "../../fixture/fixture" | ||
| import { Instance } from "../../../src/project/instance" | ||
| import { AltimateApi } from "../../../src/altimate/api/client" | ||
| import { DatamateManagerTool } from "../../../src/altimate/tools/datamate" | ||
| import { syncInternals } from "../../../src/altimate/workspace/engine-seams" | ||
| import { SessionID, MessageID } from "../../../src/session/schema" | ||
|
|
||
| const ctx = { | ||
| sessionID: SessionID.make("ses_test"), | ||
| messageID: MessageID.make("msg_test"), | ||
| callID: "call_test", | ||
| agent: "build", | ||
| abort: AbortSignal.any([]), | ||
| messages: [], | ||
| metadata: () => {}, | ||
| ask: async () => {}, | ||
| } | ||
|
|
||
| const CATALOG = [ | ||
| { id: "snowflake", name: "Snowflake", type: "connection", tools: [{ key: "execute_query" }] }, | ||
| { | ||
| id: "power-user-for-dbt", | ||
| name: "Power User for dbt", | ||
| type: "extension", | ||
| tools: [{ key: "compile_model" }, { key: "run_model" }], | ||
| }, | ||
| ] | ||
|
|
||
| const originalList = AltimateApi.listIntegrations | ||
| const originalIsConfigured = AltimateApi.isConfigured | ||
|
|
||
| afterEach(() => { | ||
| ;(AltimateApi as unknown as { listIntegrations: typeof originalList }).listIntegrations = originalList | ||
| ;(AltimateApi as unknown as { isConfigured: typeof originalIsConfigured }).isConfigured = originalIsConfigured | ||
| for (const key of Object.keys(syncInternals)) delete (syncInternals as Record<string, unknown>)[key] | ||
| }) | ||
|
|
||
| function serveCatalog(catalog: unknown[] = CATALOG): void { | ||
| ;(AltimateApi as unknown as { isConfigured: () => Promise<boolean> }).isConfigured = async () => true | ||
| ;(AltimateApi as unknown as { listIntegrations: () => Promise<unknown> }).listIntegrations = async () => catalog | ||
| } | ||
|
|
||
| async function list() { | ||
| await using tmp = await tmpdir() | ||
| return await Instance.provide({ | ||
| directory: tmp.path, | ||
| fn: async () => { | ||
| const tool = await initTool(DatamateManagerTool) | ||
| return tool.execute({ operation: "list-integrations" }, ctx as any) | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| describe("datamate_manager list-integrations and extension-type integrations", () => { | ||
| test("without a bridge they are omitted with copy that says how they come back", async () => { | ||
| serveCatalog() | ||
| syncInternals.liveBridge = () => false | ||
| const result = await list() | ||
| expect(result.metadata).toMatchObject({ count: 1, hidden: 1, bridge: false }) | ||
| expect(result.output).toContain("snowflake | Snowflake") | ||
| expect(result.output).not.toContain("power-user-for-dbt") | ||
| expect(result.output).toContain( | ||
| "1 extension-type integration was omitted — they serve while VS Code with the Altimate extension is open on this project.", | ||
| ) | ||
| expect(result.output).not.toContain("not available from the CLI") | ||
| }) | ||
|
|
||
| test("with a live bridge they are listed, marked, and counted", async () => { | ||
| serveCatalog() | ||
| syncInternals.liveBridge = () => true | ||
| const result = await list() | ||
| expect(result.metadata).toMatchObject({ count: 2, hidden: 0, bridge: true }) | ||
| expect(result.output).toContain("power-user-for-dbt | Power User for dbt (via VS Code) | compile_model, run_model") | ||
| expect(result.output).toContain("1 extension-type integration is served via the connected VS Code window.") | ||
| expect(result.title).toBe("Integrations: 2 available") | ||
| }) | ||
|
|
||
| test("a catalog with no extension rows never probes for a bridge", async () => { | ||
| serveCatalog([CATALOG[0]]) | ||
| syncInternals.liveBridge = () => { | ||
| throw new Error("must not probe") | ||
| } | ||
| const result = await list() | ||
| expect(result.metadata).toMatchObject({ count: 1, hidden: 0, bridge: false }) | ||
| expect(result.output).not.toContain("extension-type") | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: When the catalog contains an extension integration,
list-integrationsnow initializes the entire MCP state just to discover the Datamatecwd, spawning local servers and opening remote connections before the user connects anything. This adds side effects and startup latency to a presentation-only operation, and a configured MCP failure can make the listing fail; read the effective entry without initializing the MCP service, or expose a non-initializing config lookup for this probe.Prompt for AI agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declined, with evidence. (1)
MCP.entryruns through the samemakeRuntimebridge the workspace overlay has used at every turn boundary since #1167 (MCP.status/MCP.add) — its runtime shares the app-wide layermemoMap(run-service.ts:52), so it resolves the SAME MCP service instance the session already built, not a second one. (2) This tool only executes inside a session turn, and the session's tool catalog has already calledMCP.tools()— the per-instance state (and its server bootstrap) is initialized before any tool can run, so there is no 'before the user connects anything' window this call could open. (3)getMcpConfigreads instance state + merged config only; it spawns and connects nothing. (4) A failure cannot make the listing fail:engineSpawnCwdcatches and falls back toInstance.directory(connectLocal's own default).