Skip to content

Commit 62a855d

Browse files
anandgupta42claude
andcommitted
fix: keep shared credentials for untouched open projects and handle SSH forwarding errors
- `remove()` also checks the local connections file of every open instance (`Instance.directories()`), not only projects that already loaded the registry. - SSH forwarding attaches `error` handlers to the local socket and remote stream and destroys both, so a reset no longer throws an uncaught exception. - Tests: `registry-per-project.test.ts` (untouched open project, socket error). Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 1c37bc7 commit 62a855d

4 files changed

Lines changed: 88 additions & 6 deletions

File tree

‎packages/opencode/src/altimate/native/connections/registry.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,9 @@ export async function remove(name: string): Promise<{ success: boolean; error?:
969969
// only, so they are kept while an env var or the local config of a project this process serves still defines a
970970
// connection by that name (see forgetCredentials); the served set is read now, after the awaits above. (Own
971971
// properties only: a connection named "constructor" must not count as defined.)
972-
const projectConfigPaths = [projectConfigPath, ...[...states.keys()].map(localConfigPathFor)]
972+
// Open instances may not have performed a warehouse operation yet, so states alone is incomplete.
973+
const projectDirs = new Set([...states.keys(), ...Instance.directories()])
974+
const projectConfigPaths = [projectConfigPath, ...[...projectDirs].map(localConfigPathFor)]
973975
const stillDefined =
974976
projectConfigPaths.some((file) => Object.hasOwn(loadFromFile(file), name)) || Object.hasOwn(loadFromEnv(), name)
975977
const notRemoved = stillDefined ? [] : await forgetCredentials(name)

‎packages/opencode/src/altimate/native/connections/ssh-tunnel.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import type { ConnectionConfig } from "@altimateai/drivers"
1414
import type { Server, Socket } from "net"
15+
import type { Duplex } from "stream"
1516

1617
export interface TunnelInfo {
1718
localPort: number
@@ -106,14 +107,25 @@ export function openTunnel(config: SshConfig): TunnelHandle {
106107
if (closed) return
107108
// Create a local TCP server that forwards to remote host:port via SSH
108109
server = net.createServer((localSocket) => {
110+
let remoteStream: Duplex | undefined
111+
localSocket.on("error", () => {
112+
remoteStream?.destroy()
113+
localSocket.destroy()
114+
})
109115
if (closed) {
110116
localSocket.destroy()
111117
return
112118
}
113119
sockets.add(localSocket)
114120
localSocket.on("close", () => sockets.delete(localSocket))
115121
client.forwardOut("127.0.0.1", 0, config.host, config.port, (err: Error | undefined, stream: any) => {
116-
if (err || closed) {
122+
remoteStream = stream
123+
stream?.on("error", () => {
124+
localSocket.destroy()
125+
stream.destroy()
126+
})
127+
if (err || closed || localSocket.destroyed) {
128+
stream?.destroy()
117129
localSocket.destroy()
118130
return
119131
}

‎packages/opencode/src/project/instance.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ export const Instance = {
8181
return input.fn()
8282
})
8383
},
84+
// altimate_change start — enumerate open projects even before their warehouse registry is loaded
85+
directories(): string[] {
86+
return [...cache.keys()]
87+
},
88+
// altimate_change end
8489
get current() {
8590
return context.use()
8691
},

‎packages/opencode/test/altimate/registry-per-project.test.ts‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,13 @@
77
* process, through `process.chdir` (the CLI path) and through `Instance.provide` (the server path).
88
*/
99

10-
import { describe, test, expect, beforeEach, afterEach, afterAll, mock } from "bun:test"
10+
import { describe, test, expect, beforeEach, afterEach, afterAll, mock, spyOn } from "bun:test"
1111
import * as fs from "fs"
1212
import * as os from "os"
1313
import * as path from "path"
14+
import * as net from "net"
15+
import { EventEmitter } from "events"
16+
import { PassThrough } from "stream"
1417

1518
// ssh2 is an optional dependency, normally not installed for tests; a fake client lets the real tunnel module run.
1619
// Bun's mock.module leaks across test files, so the genuine module (or its absence) is restored in afterAll.
@@ -41,6 +44,7 @@ const sshClients: { host: string; ended: boolean }[] = []
4144
const heldReady = new Map<string, () => void>()
4245
/** Simulates the SSH server closing the connection cleanly (no error event). */
4346
const remoteClose = new Map<string, () => void>()
47+
let forwardedStream: PassThrough | undefined
4448
mock.module("ssh2", () => ({
4549
Client: class {
4650
private handlers = new Map<string, (...args: unknown[]) => void>()
@@ -57,8 +61,9 @@ mock.module("ssh2", () => ({
5761
if (options.host.startsWith("held-")) heldReady.set(options.host, ready)
5862
else queueMicrotask(ready)
5963
}
60-
forwardOut(_a: string, _b: number, _c: string, _d: number, cb: (err?: Error) => void) {
61-
cb(new Error("no forwarding in tests"))
64+
forwardOut(_a: string, _b: number, _c: string, _d: number, cb: (err?: Error, stream?: PassThrough) => void) {
65+
if (forwardedStream) cb(undefined, forwardedStream)
66+
else cb(new Error("no forwarding in tests"))
6267
}
6368
end() {
6469
this.record.ended = true
@@ -67,7 +72,7 @@ mock.module("ssh2", () => ({
6772
}))
6873

6974
import * as Registry from "../../src/altimate/native/connections/registry"
70-
import { closeAllTunnels, openTunnelCount } from "../../src/altimate/native/connections/ssh-tunnel"
75+
import { openTunnel, closeAllTunnels, openTunnelCount } from "../../src/altimate/native/connections/ssh-tunnel"
7176
import { Instance } from "../../src/project/instance"
7277
import { disposeInstance } from "../../src/effect/instance-registry"
7378

@@ -119,6 +124,7 @@ beforeEach(() => {
119124
process.env.OPENCODE_TEST_HOME = home
120125
projectA = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "rpp-a-")))
121126
projectB = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "rpp-b-")))
127+
forwardedStream = undefined
122128
sshClients.length = 0
123129
heldReady.clear()
124130
remoteClose.clear()
@@ -265,6 +271,40 @@ describe("connectors are per project", () => {
265271
})
266272
})
267273

274+
describe("SSH forwarding errors", () => {
275+
for (const side of ["local", "remote"] as const) {
276+
test(`${side} socket errors destroy both forwarding endpoints without throwing`, async () => {
277+
const stream = new PassThrough()
278+
forwardedStream = stream
279+
let accept: ((socket: net.Socket) => void) | undefined
280+
const server = Object.assign(new EventEmitter(), {
281+
listen: (_port: number, _host: string, ready: () => void) => ready(),
282+
address: () => ({ port: 12345 }),
283+
close: () => {},
284+
})
285+
const createServer = spyOn(net, "createServer").mockImplementation(((listener: (socket: net.Socket) => void) => {
286+
accept = listener
287+
return server
288+
}) as typeof net.createServer)
289+
const tunnel = openTunnel({ ssh_host: "forwarding", host: "db", port: 5432 })
290+
const local = new net.Socket()
291+
try {
292+
await tunnel.ready
293+
expect(accept).toBeDefined()
294+
accept!(local)
295+
const endpoint = side === "local" ? local : stream
296+
expect(() => endpoint.emit("error", new Error("ECONNRESET"))).not.toThrow()
297+
expect(local.destroyed).toBe(true)
298+
expect(stream.destroyed).toBe(true)
299+
} finally {
300+
local.destroy()
301+
tunnel.close()
302+
createServer.mockRestore()
303+
}
304+
})
305+
}
306+
})
307+
268308
describe("each connect attempt owns its SSH tunnel", () => {
269309
test("a stale tunnel start that finishes after a newer start never publishes or closes the newer tunnel", async () => {
270310
writeLocal(projectA, { store: sshStore("held-old") })
@@ -390,6 +430,29 @@ describe("remove()", () => {
390430
})
391431

392432
describe("credentials of a removed connection", () => {
433+
test("are kept for an open project that has never used the warehouse registry", async () => {
434+
const { setSecretBackendForTests } = await import("../../src/altimate/native/connections/credential-store")
435+
const store = new Map<string, string>()
436+
setSecretBackendForTests({
437+
name: "test",
438+
get: async (k: string) => store.get(k) ?? null,
439+
set: async (k: string, v: string) => void store.set(k, v),
440+
delete: async (k: string) => store.delete(k),
441+
} as any)
442+
try {
443+
writeLocal(projectB, { shared: { type: "postgres", host: "b" } })
444+
await Instance.provide({ directory: projectB, fn: async () => {} })
445+
await inProject(projectA, () => Registry.add("shared", { type: "postgres", host: "g", password: "pw-test" } as any))
446+
expect(store.get("shared/password")).toBe("pw-test")
447+
448+
expect((await inProject(projectA, () => Registry.remove("shared"))).success).toBe(true)
449+
expect(store.get("shared/password")).toBe("pw-test")
450+
} finally {
451+
await Instance.provide({ directory: projectB, fn: () => Instance.dispose() })
452+
setSecretBackendForTests(undefined)
453+
}
454+
})
455+
393456
test("are kept while another served project's local config still names the connection", async () => {
394457
const { setSecretBackendForTests } = await import("../../src/altimate/native/connections/credential-store")
395458
const store = new Map<string, string>()

0 commit comments

Comments
 (0)