Skip to content

Commit 3ab191c

Browse files
anandgupta42claude
andauthored
fix(workspace): keep tool order stable across steps so the prompt cache hits (#1401)
* fix(workspace): keep tool order stable across steps so the prompt cache hits `pinTurnTools` deleted every `datamate_`-prefixed tool and re-added the pinned ones on each step after the first. Re-added keys move to the end of the record, and the provider receives tools in record order. The prefix also matches the native `datamate_manager`, which sits mid-record, so in workspace mode it moved from position 79 to 92 on the second call of every turn and the whole ~35k-token prompt missed Anthropic's cache. The first catalog's key order is now recorded with the pinned engine tools and the record is rebuilt in that order; keys new since then go last. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(workspace): keep tools named after Object.prototype properties when pinning The rebuild used `in` checks, which are true for inherited names such as `constructor` or `toString`, so a tool with that name was dropped on every step after the first. Use own-key checks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent a94792a commit 3ab191c

2 files changed

Lines changed: 75 additions & 5 deletions

File tree

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -436,15 +436,19 @@ export async function atTurnStart<T>(sessionID: string, body: () => Promise<T>):
436436
* another session's boundary mid-turn would otherwise be re-catalogued here;
437437
* pinning keeps this turn on the engine its boundary read. A call through a
438438
* pinned wrapper after a replacement reaches the closed client and fails — it
439-
* never routes to the other workspace. */
440-
const turnTools = new Map<string, Record<string, unknown>>()
439+
* never routes to the other workspace.
440+
*
441+
* The first catalog's key order is kept too. The provider receives tools in
442+
* record order, and a reorder between steps misses its prompt cache for the
443+
* whole request. */
444+
const turnTools = new Map<string, { engine: Record<string, unknown>; order: string[] }>()
441445

442446
export function pinTurnTools<T>(sessionID: string, firstCatalog: boolean, tools: Record<string, T>): void {
443447
if (!isEnabled() || isServe()) return
444448
const engine = Object.fromEntries(Object.entries(tools).filter(([key]) => key.startsWith(TOOL_PREFIX)))
445449
if (firstCatalog) {
446450
turnTools.delete(sessionID)
447-
turnTools.set(sessionID, engine)
451+
turnTools.set(sessionID, { engine, order: Object.keys(tools) })
448452
while (turnTools.size > MAX_TRACKED_SESSIONS) {
449453
const oldest = turnTools.keys().next().value
450454
if (oldest === undefined) break
@@ -454,8 +458,15 @@ export function pinTurnTools<T>(sessionID: string, firstCatalog: boolean, tools:
454458
}
455459
const pinned = turnTools.get(sessionID)
456460
if (!pinned) return
457-
for (const key of Object.keys(engine)) delete tools[key]
458-
for (const [key, tool] of Object.entries(pinned)) tools[key] = tool as T
461+
// Own-key checks only: a tool may be named `constructor` or `toString`.
462+
const next: Record<string, T> = Object.fromEntries(
463+
Object.entries(tools).filter(([key]) => !Object.hasOwn(engine, key)),
464+
)
465+
for (const [key, tool] of Object.entries(pinned.engine)) next[key] = tool as T
466+
// Rebuild in first-catalog order; keys new since then go last, in arrival order.
467+
const ordered = new Set([...pinned.order.filter((key) => Object.hasOwn(next, key)), ...Object.keys(next)])
468+
for (const key of Object.keys(tools)) delete tools[key]
469+
for (const key of ordered) tools[key] = next[key]
459470
}
460471

461472
async function reconcile(sessionID: string, directory: string, state: DirectoryState): Promise<void> {

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,65 @@ describe("beforeTurn — what a turn boundary does", () => {
916916
expect(step2).toEqual({ datamate_b: { id: "b1" } })
917917
})
918918

919+
test("pinning keeps the tool order of the first catalog, so the provider's prompt cache still hits", async () => {
920+
install({})
921+
// The native `datamate_manager` shares the engine prefix and sits between native tools.
922+
const catalog = () => ({
923+
read: { id: "read" },
924+
datamate_manager: { id: "native" },
925+
feedback_submit: { id: "feedback" },
926+
dbt_pr_review: { id: "review" },
927+
datamate_add_memories: { id: "add" },
928+
datamate_search_memory: { id: "search" },
929+
})
930+
const first = catalog()
931+
pinTurnTools("s1", true, first)
932+
const later = catalog()
933+
pinTurnTools("s1", false, later)
934+
expect(Object.keys(later)).toEqual(Object.keys(first))
935+
expect(later.datamate_manager).toBe(first.datamate_manager)
936+
})
937+
938+
test("pinning keeps the first catalog's order when the engine's tools change mid-turn", async () => {
939+
install({})
940+
pinTurnTools("s1", true, {
941+
read: { id: "read" },
942+
datamate_a: { id: "a1" },
943+
datamate_b: { id: "b1" },
944+
sql: { id: "sql" },
945+
})
946+
// `datamate_a` vanished, the survivor and a native tool arrive reversed, and a new engine tool appeared.
947+
const later: Record<string, { id: string }> = {
948+
sql: { id: "sql" },
949+
datamate_c: { id: "c2" },
950+
datamate_b: { id: "b2" },
951+
read: { id: "read" },
952+
write: { id: "write" },
953+
}
954+
pinTurnTools("s1", false, later)
955+
expect(Object.keys(later)).toEqual(["read", "datamate_a", "datamate_b", "sql", "write"])
956+
expect(later.datamate_a).toEqual({ id: "a1" })
957+
expect(later.datamate_b).toEqual({ id: "b1" })
958+
})
959+
960+
test("pinning keeps tools whose names are Object.prototype properties", async () => {
961+
install({})
962+
const catalog = (): Record<string, { id: string }> =>
963+
Object.fromEntries([
964+
["constructor", { id: "ctor" }],
965+
["datamate_a", { id: "a1" }],
966+
["toString", { id: "str" }],
967+
])
968+
pinTurnTools("s1", true, catalog())
969+
const later = catalog()
970+
pinTurnTools("s1", false, later)
971+
expect(Object.entries(later)).toEqual([
972+
["constructor", { id: "ctor" }],
973+
["datamate_a", { id: "a1" }],
974+
["toString", { id: "str" }],
975+
])
976+
})
977+
919978
test("pinning is a no-op with the flag off and for a session with no step-1 snapshot", async () => {
920979
install({ flag: false })
921980
const tools: Record<string, { id: string }> = { datamate_a: { id: "a1" } }

0 commit comments

Comments
 (0)