Skip to content

Commit d5227fe

Browse files
saravmajesticclaude
saravmajestic
and
claude
committed
fix(workspace): drop the unreachable engine-probes arm, keep the precedence fix
Review was right that the first cut reached too far. `engine-probes.resolveBinding` is called only from `engine-overlay` at two sites, and both sit behind `if (!isEnabled() || isServe())`. The pin exists only when `ALTIMATE_CODE_SERVE=1` (`pin.ts`), which is exactly the mode that bail covers, so the pin arm added there could not run in any configuration. `manage.ts` imports its `resolveBinding` from `./state`, which is already pin-aware, so nothing else reached it either. Reverted. That removes the second finding with it: the arm re-read credentials through `currentScope()` after `resolvePinnedBinding` had validated against its own snapshot, so a credential change between the two awaits could have paired one account's validated id with another's scope. Deleting the code is a better answer than guarding it, since it had no caller. `precedence.currentBinding` is a different matter and the fix stays: `derive` gates on `isEnabled()` alone, with no `isServe()` bail, so it does run in a pinned session — which is why the reported prompt named two workspaces at once. What this does NOT do, now stated plainly rather than implied: it does not make warehouse calls execute against the pinned workspace. In serve mode `atTurnStart` records `disabled`, `SERVING` rejects that, and `derive` settles `unattributed` — routing is off there by design, because the extension runs its own engine under the same key. The reachable effect is which workspace the section names, which is the contradiction the report opens with. Tests follow the code: the engine-probes cases are replaced with precedence ones driven through `refresh()` under a real `Instance`, so they exercise the path that actually runs instead of a function no caller reaches. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
1 parent d5cd388 commit d5227fe

2 files changed

Lines changed: 59 additions & 51 deletions

File tree

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

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { AltimateApi } from "@/altimate/api/client"
1111
import { AppRuntime } from "@/effect/app-runtime"
1212
import { EventV2Bridge } from "@/event-v2-bridge"
1313
import { TuiEvent } from "@/server/tui-event"
14-
import { currentScope, readLocalBindingScopedStrict, resolvePinnedBindingForRouting } from "./state"
14+
import { readLocalBindingScopedStrict } from "./state"
1515
import { log, syncInternals, type BindingRead, type ScopedBinding } from "./engine-seams"
1616
import type { Declared, DeclaredExtension, Toast } from "./engine-types"
1717

@@ -25,32 +25,9 @@ export const DECLARED_TIMEOUT_MS = 4_000
2525
* same path a production one does. */
2626
export async function resolveBinding(directory: string): Promise<BindingRead> {
2727
try {
28-
if (syncInternals.resolveBinding) {
29-
const seam = await syncInternals.resolveBinding(directory)
30-
return seam ? { kind: "bound", binding: seam } : { kind: "unbound" }
31-
}
32-
// altimate_change — honour the IDE extension's pin before the project's own link, so the
33-
// engine overlay claims the key for the workspace the panel selected rather than the one the
34-
// project happens to be linked to (#1337). Same precedence `resolveBindingOutcome` applies
35-
// for identity, skills and memory.
36-
const pinned = await resolvePinnedBindingForRouting(directory)
37-
if (pinned) {
38-
if (pinned.status !== "bound") {
39-
// Fail closed: a pin that cannot be honoured must not silently hand the overlay back to
40-
// the project's link, which is the mismatch this prevents. `failed` holds the key and
41-
// retries at the next turn boundary rather than releasing it as `unbound` would.
42-
return { kind: "failed", error: "the workspace pin could not be honoured" }
43-
}
44-
const scope = await currentScope()
45-
return {
46-
kind: "bound",
47-
binding: {
48-
...pinned.binding,
49-
scope: scope ? `${scope.tenant}|${scope.apiUrl}` : undefined,
50-
},
51-
}
52-
}
53-
const binding = await readScoped(directory)
28+
const binding = syncInternals.resolveBinding
29+
? await syncInternals.resolveBinding(directory)
30+
: await readScoped(directory)
5431
return binding ? { kind: "bound", binding } : { kind: "unbound" }
5532
} catch (err) {
5633
log.warn("could not read the workspace binding", { err: String(err) })

‎packages/opencode/test/altimate/workspace/routing-pin.test.ts‎

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ process.env.XDG_STATE_HOME = path.join(SANDBOX, "state")
2020
const { resolvePinnedBindingForRouting, recordApprovedBinding, __resetPinValidation } = await import(
2121
"../../../src/altimate/workspace/state"
2222
)
23-
const { resolveBinding } = await import("../../../src/altimate/workspace/engine-probes")
23+
const { refresh, precedenceInternals } = await import("../../../src/altimate/workspace/precedence")
24+
const { Instance } = await import("../../../src/project/instance")
25+
const { SNOWFLAKE_TOOLS } = await import("./precedence-fixture")
2426
const { AltimateApi } = await import("../../../src/altimate/api/client")
2527
const { WorkspaceApi } = await import("../../../src/altimate/workspace/api-client")
2628

@@ -82,7 +84,11 @@ async function seedLocalLink(datamateId = 7, datamateName = "project-link") {
8284
} as never)
8385
}
8486

87+
const ORIGINAL_PILOT = process.env.ALTIMATE_WORKSPACE
88+
8589
beforeEach(() => {
90+
// `derive` short-circuits on `pilot-off` before it ever reads a binding.
91+
process.env.ALTIMATE_WORKSPACE = "1"
8692
__resetPinValidation()
8793
stubCreds()
8894
stubList([
@@ -98,6 +104,8 @@ afterEach(() => {
98104
})
99105

100106
afterAll(() => {
107+
if (ORIGINAL_PILOT === undefined) delete process.env.ALTIMATE_WORKSPACE
108+
else process.env.ALTIMATE_WORKSPACE = ORIGINAL_PILOT
101109
;(AltimateApi as unknown as { isConfigured: unknown }).isConfigured = originalIsConfigured
102110
;(AltimateApi as unknown as { getCredentials: unknown }).getCredentials = originalGetCreds
103111
;(WorkspaceApi as unknown as { listDatamates: unknown }).listDatamates = originalList
@@ -135,35 +143,58 @@ describe("resolvePinnedBindingForRouting", () => {
135143
})
136144
})
137145

138-
describe("engine-probes.resolveBinding — the routing read", () => {
139-
test("routes at the pin, not the project's own link", async () => {
140-
await seedLocalLink(7)
141-
setPin()
142-
const read = await resolveBinding(ROOT)
143-
expect(read.kind).toBe("bound")
144-
// The regression: this returned 7 — the identity section said 42 in the same prompt.
145-
expect(read.kind === "bound" && read.binding.datamateId).toBe(42)
146-
})
146+
describe("precedence.derive — the routing read", () => {
147+
const SESSION = "ses_routing_pin"
147148

148-
test("still reads the project's own link when nothing is pinned", async () => {
149-
await seedLocalLink(7)
150-
const read = await resolveBinding(ROOT)
151-
expect(read.kind).toBe("bound")
152-
expect(read.kind === "bound" && read.binding.datamateId).toBe(7)
149+
/** Serve mode never attributes an engine: `engine-overlay.atTurnStart` short-circuits on
150+
* `isServe()` and records `disabled`, which `SERVING` rejects. So the reachable effect of the
151+
* pin here is WHICH workspace the section names, not whether routing turns on. Stubbed to
152+
* `undefined` to reproduce that without a live engine. */
153+
function unattributedEngine() {
154+
precedenceInternals.attachOutcome = async () => undefined
155+
}
156+
157+
afterEach(() => {
158+
delete precedenceInternals.attachOutcome
159+
delete precedenceInternals.binding
153160
})
154161

155-
test("fails closed when the pin cannot be honoured, rather than routing at the project's link", async () => {
156-
await seedLocalLink(7)
157-
stubList([{ id: 7, name: "project-link" }])
162+
async function derivedIn(directory: string) {
163+
return await Instance.provide({
164+
directory,
165+
fn: async () => {
166+
const p = await refresh(SESSION, SNOWFLAKE_TOOLS)
167+
await Instance.dispose()
168+
return p
169+
},
170+
})
171+
}
172+
173+
test("names the pinned workspace, not the project's own link", async () => {
174+
await seedLocalLink(7, "project-link")
175+
unattributedEngine()
158176
setPin()
159-
const read = await resolveBinding(ROOT)
160-
// Not `bound` at 7: falling back to the project's link is the confusion this prevents.
161-
expect(read.kind).toBe("failed")
177+
const p = await derivedIn(ROOT)
178+
// The regression: this said "project-link" while the identity section said the pinned one —
179+
// two workspaces named in a single prompt.
180+
expect(p.workspaceName).toBe("pinned-workspace")
162181
})
163182

164-
test("carries the credential scope so the engine key stays account-partitioned", async () => {
183+
test("still names the project's own link when nothing is pinned", async () => {
184+
await seedLocalLink(7, "project-link")
185+
unattributedEngine()
186+
const p = await derivedIn(ROOT)
187+
expect(p.workspaceName).toBe("project-link")
188+
})
189+
190+
test("fails closed when the pin cannot be honoured, rather than naming the project's link", async () => {
191+
await seedLocalLink(7, "project-link")
192+
unattributedEngine()
193+
stubList([{ id: 7, name: "project-link" }])
165194
setPin()
166-
const read = await resolveBinding(ROOT)
167-
expect(read.kind === "bound" && read.binding.scope).toBe("acme|https://api.test")
195+
const p = await derivedIn(ROOT)
196+
expect(p.enabled).toBe(false)
197+
expect(p.disabledReason).toBe("binding-unreadable")
198+
expect(p.workspaceName).not.toBe("project-link")
168199
})
169200
})

0 commit comments

Comments
 (0)