From cbf578e78bb6e690257297e69aea2170f987cc89 Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 21 Aug 2026 05:48:48 +0530 Subject: [PATCH 1/5] feat(workspace): load workspace memory on demand, and show it in the memory tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the half of the read requirement that was never built. Memory loaded at session start, but "on the user's demand" had no entry point at all, and the memory tool could not see workspace memory even when the model had been given it. **A session could never pick up newer memory.** `hydrate` is idempotent for the life of a session — deliberately, so the per-turn injection stays cheap — but nothing reset it and no command forced a reload. A session started before a teammate (or this user on another machine) wrote a block never saw it, for its entire life. Adds `refresh(sessionID)`, which drops the session's cached state so the next hydrate genuinely refetches, exposed as a new `altimate_memory_refresh` tool so the user can just ask for it. **The memory tool disagreed with the prompt.** `altimate_memory_read` read the local store only, while injection merged the workspace overlay — so a user asking "what do you remember?" was shown strictly less than the model actually had. It now merges the same overlay, filtered to the requested scope. `mergeOverlay` is exported rather than duplicated, so the tool and the injection path cannot drift. Verified against a live backend and real mem0, with controls that fail on the old behaviour: a block written mid-session reaches the workspace, plain `hydrate` still cannot see it, `refresh` pulls it in, and the read tool then lists both the early and the late block. Both fixes are mutation-checked — removing the state clear in `refresh`, or the merge in the read tool, each fails a test. Tests: 11286 pass. The two failures in the full run are unrelated and pre-existing (`Truncate > cleanup` fails identically at the merge-base; the subprocess suite is load-flaky and passes 5/5 on its own). --- .../src/altimate/workspace/memory-sync.ts | 15 +++++ packages/opencode/src/memory/prompt.ts | 2 +- .../opencode/src/memory/tools/memory-read.ts | 17 +++++- .../src/memory/tools/memory-refresh.ts | 60 +++++++++++++++++++ packages/opencode/src/tool/registry.ts | 2 + .../test/memory/overlay-merge.test.ts | 58 +++++++++++++++++- 6 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 packages/opencode/src/memory/tools/memory-refresh.ts diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 4c0b59f4d4..48e780b050 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -806,6 +806,21 @@ export function overlayBlocks(sessionID: string): RemoteMemoryBlock[] { return [...(sessions.get(sessionID)?.overlay ?? [])] } +/** Re-read this session's workspace memory, discarding what it already holds. + * + * ``hydrate`` is idempotent for the life of a session, which is what keeps the + * per-turn call cheap -- but it also means a session started before a teammate + * (or this user on another machine) wrote a block never sees it. This is the + * on-demand path: drop the session's state so the next hydrate genuinely + * refetches. Returns how many blocks the session now holds. */ +export async function refresh(sessionID: string): Promise { + if (!isEnabled()) return 0 + // Clears overlay, hydration promise and the timed-out latch together. + sessions.delete(sessionID) + await hydrate(sessionID) + return overlayBlocks(sessionID).length +} + /** Forget a session's hydration, or all of them. * * Not called per turn: doing so defeated ``hydrate``'s idempotence and made diff --git a/packages/opencode/src/memory/prompt.ts b/packages/opencode/src/memory/prompt.ts index 9d7ba753b2..89c4cb2208 100644 --- a/packages/opencode/src/memory/prompt.ts +++ b/packages/opencode/src/memory/prompt.ts @@ -66,7 +66,7 @@ function isRemote(block: MemoryBlock): block is RemoteMemoryBlock { return (block as RemoteMemoryBlock).remote === true } -function mergeOverlay(local: MemoryBlock[], remote: RemoteMemoryBlock[]): MemoryBlock[] { +export function mergeOverlay(local: MemoryBlock[], remote: RemoteMemoryBlock[]): MemoryBlock[] { if (remote.length === 0) return local const localKeys = new Set(local.map((b) => `${b.scope}:${b.id}`)) const merged: MemoryBlock[] = [...local] diff --git a/packages/opencode/src/memory/tools/memory-read.ts b/packages/opencode/src/memory/tools/memory-read.ts index e8cd063421..e595e65eef 100644 --- a/packages/opencode/src/memory/tools/memory-read.ts +++ b/packages/opencode/src/memory/tools/memory-read.ts @@ -1,7 +1,9 @@ import z from "zod" import { Tool } from "../../tool/tool" import { MemoryStore, isExpired } from "../store" -import { MemoryPrompt } from "../prompt" +import { MemoryPrompt, mergeOverlay } from "../prompt" +// altimate_change - workspace memory overlay +import { overlayBlocks, whenHydrated } from "@/altimate/workspace/memory-sync" import { MemoryBlockSchema } from "../types" export const MemoryReadTool = Tool.define("altimate_memory_read", { @@ -52,6 +54,19 @@ export const MemoryReadTool = Tool.define("altimate_memory_read", { ? await MemoryStore.listAll(listOpts) : await MemoryStore.list(args.scope as "global" | "project", listOpts) + // altimate_change start — fold in this session's workspace memory. Reading + // only the local store made this tool disagree with what the model was + // actually given: injection merges the overlay, so a user asking "what do + // you remember?" saw strictly less than the prompt contained. + if (ctx?.sessionID) { + await whenHydrated(ctx.sessionID) + const remote = overlayBlocks(ctx.sessionID).filter( + (b) => args.scope === "all" || b.scope === args.scope, + ) + blocks = mergeOverlay(blocks, remote) + } + // altimate_change end + if (args.tags && args.tags.length > 0) { blocks = blocks.filter((b) => args.tags!.every((tag) => b.tags.includes(tag))) } diff --git a/packages/opencode/src/memory/tools/memory-refresh.ts b/packages/opencode/src/memory/tools/memory-refresh.ts new file mode 100644 index 0000000000..60d1f77487 --- /dev/null +++ b/packages/opencode/src/memory/tools/memory-refresh.ts @@ -0,0 +1,60 @@ +// altimate_change - new file +// +// On-demand reload of this session's workspace memory. +// +// `hydrate` is idempotent for the life of a session — deliberately, so the +// per-turn injection stays cheap. The cost is that a session started before a +// teammate (or this user on another machine) wrote a block never sees it. This +// tool is the "on user's demand" half of the requirement: the user asks, the +// agent calls this, and the session picks up everything written since. +import z from "zod" +import { Tool } from "../../tool/tool" +import { refresh, isEnabled } from "@/altimate/workspace/memory-sync" + +export const MemoryRefreshTool = Tool.define("altimate_memory_refresh", { + description: [ + "Reload memory from the Altimate workspace this project is linked to.", + "", + "Use when the user asks to refresh, reload, re-read or re-sync memory, or", + "says a teammate added something they want picked up now. A session loads", + "workspace memory once at start; anything written after that is invisible", + "until this runs.", + "", + "Does nothing when the project is not linked to a workspace.", + ].join("\n"), + parameters: z.object({}), + async execute(_args, ctx) { + if (!isEnabled()) { + return { + title: "Memory: workspace sync off", + metadata: { count: 0, refreshed: false }, + output: "Workspace memory is not enabled, so there is nothing to reload.", + } + } + if (!ctx?.sessionID) { + return { + title: "Memory: no session", + metadata: { count: 0, refreshed: false }, + output: "No session context, so there is no memory overlay to reload.", + } + } + try { + const count = await refresh(ctx.sessionID) + return { + title: `Memory: reloaded ${count} workspace block(s)`, + metadata: { count, refreshed: true }, + output: + count === 0 + ? "Reloaded workspace memory. This workspace has no memory blocks visible to this project." + : `Reloaded workspace memory: ${count} block(s) now available. Use altimate_memory_read to see them.`, + } + } catch (e) { + // Never fail the turn over a refresh — the session keeps whatever it had. + return { + title: "Memory: reload failed", + metadata: { count: 0, refreshed: false }, + output: `Could not reload workspace memory: ${e instanceof Error ? e.message : String(e)}`, + } + } + }, +}) diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 20e8bd1ba6..a3be18e6a4 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -133,6 +133,7 @@ import { SampleSetupTool } from "../altimate/tools/sample-setup" // altimate_change start - import altimate persistent memory tools import { MemoryReadTool } from "../memory/tools/memory-read" +import { MemoryRefreshTool } from "../memory/tools/memory-refresh" import { MemoryWriteTool } from "../memory/tools/memory-write" import { MemoryDeleteTool } from "../memory/tools/memory-delete" import { MemoryAuditTool } from "../memory/tools/memory-audit" @@ -468,6 +469,7 @@ export namespace ToolRegistry { ...(!Flag.ALTIMATE_DISABLE_MEMORY ? [ MemoryReadTool, + MemoryRefreshTool, MemoryWriteTool, MemoryDeleteTool, MemoryAuditTool, diff --git a/packages/opencode/test/memory/overlay-merge.test.ts b/packages/opencode/test/memory/overlay-merge.test.ts index 8aa825b461..4b75a7270f 100644 --- a/packages/opencode/test/memory/overlay-merge.test.ts +++ b/packages/opencode/test/memory/overlay-merge.test.ts @@ -31,9 +31,12 @@ afterAll(() => { } }) -const { MemoryPrompt } = await import("../../src/memory/prompt") +const { MemoryPrompt, mergeOverlay } = await import("../../src/memory/prompt") +const { MemoryStore } = await import("../../src/memory/store") +const { MemoryReadTool } = await import("../../src/memory/tools/memory-read") +const { initTool } = await import("../altimate/tool-fixture") const { MIRROR_SOURCE } = await import("../../src/altimate/workspace/memory-api") -const { hydrate, resetOverlay, syncInternals } = await import( +const { hydrate, refresh, resetOverlay, overlayBlocks, syncInternals } = await import( "../../src/altimate/workspace/memory-sync" ) const { TrainingStore } = await import("../../src/altimate/training/store") @@ -127,6 +130,57 @@ const remote = (id: string, content: string, extra: Record = {} metadata: { source: MIRROR_SOURCE, block_id: id, block_scope: "global", ...extra }, }) +describe("the memory read tool", () => { + test("surfaces workspace memory, not just the local store", async () => { + // The tool read MemoryStore only, so it disagreed with what the model was + // actually given -- injection merges the overlay and the tool did not. + writeLocalBlock("local/only", "A LOCAL FACT") + listResponse = [remote("remote/only", "A WORKSPACE FACT")] + await hydrate(SES) + + const local = (await MemoryStore.listAll()).map((b) => b.id) + expect(local).toContain("local/only") + expect(local).not.toContain("remote/only") + + // Drive the real tool, not mergeOverlay directly — asserting on the helper + // would pass even if the tool stopped calling it. + const tool = await initTool(MemoryReadTool) + const res = await tool.execute({ scope: "all" }, { sessionID: SES, agent: "build" }) + const out = String((res as any).output ?? "") + expect(out).toContain("A LOCAL FACT") + expect(out).toContain("A WORKSPACE FACT") + }) +}) + +describe("on-demand reload", () => { + test("refresh picks up memory written after the session hydrated", async () => { + // `hydrate` is idempotent for the life of a session, so without `refresh` + // a live session never sees anything written after it started. + listResponse = [remote("warehouse/one", "FIRST FACT")] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["warehouse/one"]) + + // A teammate writes while this session is running. + listResponse = [remote("warehouse/one", "FIRST FACT"), remote("warehouse/two", "SECOND FACT")] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["warehouse/one"]) + + const count = await refresh(SES) + expect(count).toBe(2) + expect(overlayBlocks(SES).map((b) => b.id).sort()).toEqual(["warehouse/one", "warehouse/two"]) + }) + + test("refresh drops a block that was archived in the workspace", async () => { + listResponse = [remote("gone", "WILL BE ARCHIVED")] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["gone"]) + + listResponse = [remote("gone", "WILL BE ARCHIVED", { archived: "true" })] + await refresh(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual([]) + }) +}) + describe("workspace memory in the injected prompt", () => { test("a cloud block with no local counterpart is injected", async () => { listResponse = [remote("warehouse/sizing", "ANALYTICS_WH must not be resized without asking.")] From 6ff1ba97fb6d8b2c9c682c96b70d73682c5eee28 Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 21 Aug 2026 06:49:33 +0530 Subject: [PATCH 2/5] fix(workspace): keep memory on a failed refresh, mirror against the writing project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both defects were found by running the on-demand work end to end against a live backend, not by the unit suite. **A failed refresh destroyed the session's memory.** `doHydrate` empties the overlay when a fetch fails — correct at session start, where there was nothing to lose — but `refresh` clears the session first, so a network hiccup during a user-requested reload wiped everything the session had. The user asks to refresh, and silently ends up with less. `refresh` now snapshots the overlay, restores it when the fetch failed, and returns `{count, ok}` so the caller can tell "reloaded, workspace is empty" from "could not reach the workspace". The tool says which happened instead of reporting a count that reads like success. **The mirror resolved the wrong project.** `existsLocally` — added last round to stop a delete-during-sweep resurrecting a block — called `MemoryStore.read` without a directory, so it resolved project scope from the *ambient* instance. `mirrorBlock` is fire-and-forget, so by the time it ran the ambient instance could be a different project, the block was not found there, and a perfectly good write was skipped as "deleted". Same class as the `list`/`read` mismatch fixed earlier in this stack. The owning directory is now captured at write time, while the writing context is still current, and threaded through `mirrorBlock` → `push` → `existsLocally`; `backfill` passes the directory it swept. Reproduced directly: writing inside a nested instance context, the block reads back `false` from the outer context and `true` from the captured directory, and mirrors in 3s with the fix. E2E: 11/11 against a live backend and real mem0, including the edge cases the happy path misses — a block archived elsewhere disappearing on refresh, scope filtering not leaking project blocks into a global read, refresh on a session that never hydrated, and refresh on an unlinked project being a quiet no-op. Both fixes mutation-checked. 431 tests pass across the affected suites. --- .../src/altimate/workspace/memory-backfill.ts | 2 +- .../src/altimate/workspace/memory-sync.ts | 55 ++++++++++++++----- packages/opencode/src/memory/store.ts | 17 +++++- .../src/memory/tools/memory-refresh.ts | 11 +++- .../altimate/workspace/memory-sync.test.ts | 24 ++++++++ .../test/memory/overlay-merge.test.ts | 25 ++++++++- .../test/memory/store-directory.test.ts | 5 ++ 7 files changed, 120 insertions(+), 19 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/memory-backfill.ts b/packages/opencode/src/altimate/workspace/memory-backfill.ts index 17168234ab..7ca211f94c 100644 --- a/packages/opencode/src/altimate/workspace/memory-backfill.ts +++ b/packages/opencode/src/altimate/workspace/memory-backfill.ts @@ -33,7 +33,7 @@ export async function backfillOnBind(directory: string, binding: CachedBinding): // CLI still prints "Linked". Reading project memory was the entire point. const blocks = await MemoryStore.listAll({ directory }) if (blocks.length === 0) return true - const result = await backfill(blocks, binding) + const result = await backfill(blocks, binding, directory) log.info("workspace memory seeded after bind", result) // Only a sweep that stored everything it meant to counts as seeded. A // failure here must leave the binding eligible for a retry, or local blocks diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 48e780b050..6d6fcc7b28 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -73,6 +73,9 @@ interface SessionMemory { touchedAt: number /** Set once a bounded wait expired, so later injections do not re-wait. */ waitTimedOut?: boolean + /** Set when the last fetch for this session failed, so a caller can tell an + * empty workspace apart from an unreadable one. */ + hydrateFailed?: boolean } const sessions = new Map() @@ -107,7 +110,7 @@ export function isEnabled(): boolean { export const syncInternals: { resolveBinding?: () => Promise /** Test seam for the local-existence check. Production reads the store. */ - blockExists?: (block: MemoryBlock) => Promise + blockExists?: (block: MemoryBlock, directory?: string) => Promise } = {} /** Instance.directory throws synchronously with no instance context, so a @@ -304,11 +307,14 @@ type PushOutcome = "stored" | "unchanged" | "declined" | "skipped" * a static import would close an eval-order cycle (see ./memory-backfill.ts). * A read failure answers "yes" — refusing to mirror on a transient read error * would silently drop a memory the user does have. */ -async function existsLocally(block: MemoryBlock): Promise { - if (syncInternals.blockExists) return syncInternals.blockExists(block) +async function existsLocally(block: MemoryBlock, directory?: string): Promise { + if (syncInternals.blockExists) return syncInternals.blockExists(block, directory) try { const { MemoryStore } = await import("@/memory/store") - return !!(await MemoryStore.read(block.scope, block.id)) + // ``directory`` is the tree that owned the write. Without it this resolves + // project scope from the ambient instance, which for a fire-and-forget + // mirror can be a different project entirely. + return !!(await MemoryStore.read(block.scope, block.id, directory)) } catch (err) { log.warn("could not confirm a block still exists locally; mirroring anyway", { id: block.id, @@ -322,6 +328,7 @@ async function push( block: MemoryBlock, binding: CachedBinding | null, known?: KnownRecords, + directory?: string, ): Promise { const key = indexKey({ scope: block.scope, @@ -358,7 +365,7 @@ async function push( // dequeues it, so a delete issued mid-sweep runs first and this push would // otherwise undo it -- recreating a record the user deleted, or reviving an // archived one, and then marking it synced so no later sweep re-archives it. - if (!(await existsLocally(block))) { + if (!(await existsLocally(block, directory))) { log.warn("skipping mirror for a block that no longer exists locally", { id: block.id, scope: block.scope, @@ -457,7 +464,7 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr /** Mirror one block. Safe to call unconditionally — returns immediately when * the pilot flag is off, the project is unbound, or the workspace has memory * disabled. */ -export async function mirrorBlock(block: MemoryBlock): Promise { +export async function mirrorBlock(block: MemoryBlock, directory?: string): Promise { if (!isEnabled()) return // Queued BEFORE the binding lookup, not after. Both are async, so resolving // them first let two operations on one block reach `serialize` in the @@ -472,7 +479,7 @@ export async function mirrorBlock(block: MemoryBlock): Promise { const binding = await currentBinding() if (!binding) return if (!(await memoryEnabled(binding))) return - await push(block, binding) + await push(block, binding, undefined, directory) }) } @@ -592,6 +599,7 @@ async function runQueue( export async function backfill( blocks: MemoryBlock[], explicitBinding?: CachedBinding, + sweepDirectory?: string, ): Promise<{ ok: number; failed: number; skipped: number; declined: number; gated: boolean }> { // ``gated`` says the sweep never ran, as opposed to running and storing // nothing. A caller recording "this binding is seeded" must be able to tell @@ -604,7 +612,7 @@ export async function backfill( return { ok: 0, failed: 0, skipped: blocks.length, declined: 0, gated: true } const index = await readIndex() - const pending: { block: MemoryBlock; binding: CachedBinding | null }[] = [] + const pending: { block: MemoryBlock; binding: CachedBinding | null; directory?: string }[] = [] let skipped = 0 for (const block of blocks) { const target = block.scope === "project" ? binding : null @@ -622,7 +630,7 @@ export async function backfill( skipped++ continue } - pending.push({ block, binding: target }) + pending.push({ block, binding: target, directory: sweepDirectory }) } if (pending.length === 0) return { ok: 0, failed: 0, skipped, declined: 0, gated: false } @@ -642,7 +650,10 @@ export async function backfill( log.info("workspace memory backfill starting", { pending: pending.length, skipped }) const result = await runQueue( pending, - (item) => serialize(item.block.scope, item.block.id, () => push(item.block, item.binding, known)), + (item) => + serialize(item.block.scope, item.block.id, () => + push(item.block, item.binding, known, item.directory), + ), BACKFILL_CONCURRENCY, ) // `skipped` combines blocks filtered before the queue (already synced, or @@ -790,13 +801,17 @@ async function doHydrate(sessionID: string): Promise { blocks.push(block) } - sessionState(sessionID).overlay = blocks + const done = sessionState(sessionID) + done.overlay = blocks + done.hydrateFailed = false if (blocks.length > 0) { log.info("workspace memory hydrated", { blocks: blocks.length, workspace: binding?.datamateName }) } } catch (err) { log.warn("workspace memory hydration failed", { err: String(err) }) - sessionState(sessionID).overlay = [] + const failed = sessionState(sessionID) + failed.overlay = [] + failed.hydrateFailed = true } } @@ -813,12 +828,22 @@ export function overlayBlocks(sessionID: string): RemoteMemoryBlock[] { * (or this user on another machine) wrote a block never sees it. This is the * on-demand path: drop the session's state so the next hydrate genuinely * refetches. Returns how many blocks the session now holds. */ -export async function refresh(sessionID: string): Promise { - if (!isEnabled()) return 0 +export async function refresh(sessionID: string): Promise<{ count: number; ok: boolean }> { + if (!isEnabled()) return { count: 0, ok: false } + // Keep what the session already has. A failed fetch empties the overlay + // (hydration failure is indistinguishable from an empty workspace at session + // start, where [] is correct) -- but mid-session that would silently destroy + // working memory because the user asked to reload and the network hiccuped. + const previous = overlayBlocks(sessionID) // Clears overlay, hydration promise and the timed-out latch together. sessions.delete(sessionID) await hydrate(sessionID) - return overlayBlocks(sessionID).length + const state = sessions.get(sessionID) + if (state?.hydrateFailed) { + state.overlay = previous + return { count: previous.length, ok: false } + } + return { count: overlayBlocks(sessionID).length, ok: true } } /** Forget a session's hydration, or all of them. diff --git a/packages/opencode/src/memory/store.ts b/packages/opencode/src/memory/store.ts index da4b0bf64b..325e27629b 100644 --- a/packages/opencode/src/memory/store.ts +++ b/packages/opencode/src/memory/store.ts @@ -42,6 +42,15 @@ function projectDir(directory?: string): string { } // altimate_change end +/** The current instance directory, or undefined outside an instance context. */ +function safeDirectory(): string | undefined { + try { + return Instance.directory + } catch { + return undefined + } +} + function dirForScope(scope: "global" | "project", directory?: string): string { return scope === "global" ? globalDir() : projectDir(directory) } @@ -303,7 +312,13 @@ export namespace MemoryStore { // already durable here, so a cloud failure must not surface as a failed // memory write. No-ops unless the pilot flag is on, the project is bound, // and the workspace has memory enabled. - void mirrorBlock(block).catch((e) => { + // The directory is captured HERE, while the writing context is still + // current. `mirrorBlock` is fire-and-forget, so by the time it runs the + // ambient instance may be a different project -- and the mirror's + // local-existence check would then look for this block in the wrong tree + // and skip it as deleted. + const owningDirectory = block.scope === "project" ? safeDirectory() : undefined + void mirrorBlock(block, owningDirectory).catch((e) => { mirrorLog.warn("failed to mirror memory block to workspace", { id: block.id, scope: block.scope, diff --git a/packages/opencode/src/memory/tools/memory-refresh.ts b/packages/opencode/src/memory/tools/memory-refresh.ts index 60d1f77487..29286479fe 100644 --- a/packages/opencode/src/memory/tools/memory-refresh.ts +++ b/packages/opencode/src/memory/tools/memory-refresh.ts @@ -39,7 +39,16 @@ export const MemoryRefreshTool = Tool.define("altimate_memory_refresh", { } } try { - const count = await refresh(ctx.sessionID) + const { count, ok } = await refresh(ctx.sessionID) + if (!ok) { + // The session keeps whatever it already had -- say so rather than + // reporting a count the user might read as a successful reload. + return { + title: "Memory: could not reach the workspace", + metadata: { count, refreshed: false }, + output: `Could not reach the workspace, so memory was not reloaded. This session still has its existing ${count} block(s).`, + } + } return { title: `Memory: reloaded ${count} workspace block(s)`, metadata: { count, refreshed: true }, diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 5c80a4f6bb..6d9441945a 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -407,6 +407,30 @@ describe("mirrorBlock", () => { expect(callsTo("/datamates/memory/mem-collide", "PATCH").length).toBe(1) }) + test("the local-existence check uses the writing project, not the ambient one", async () => { + // `mirrorBlock` is fire-and-forget: by the time it runs, the ambient + // instance may be a different project. Resolving project scope from the + // ambient directory then finds nothing and skips a perfectly good write as + // "deleted". The directory captured at write time has to win. + const seen: (string | undefined)[] = [] + syncInternals.blockExists = async () => true + const b = block({ id: "owned-elsewhere", scope: "project" }) + const { MemoryStore } = await import("../../../src/memory/store") + const origRead = MemoryStore.read + ;(MemoryStore as any).read = async (_s: string, _i: string, dir?: string) => { + seen.push(dir) + return b + } + delete syncInternals.blockExists + try { + await mirrorBlock(b, "/work/the-writing-project") + expect(seen).toContain("/work/the-writing-project") + } finally { + ;(MemoryStore as any).read = origRead + syncInternals.blockExists = async () => true + } + }) + test("a block deleted mid-sweep is not recreated", async () => { // `backfill` registers a block on the serialize queue only when a worker // dequeues it, so a delete issued during the sweep runs first. Without the diff --git a/packages/opencode/test/memory/overlay-merge.test.ts b/packages/opencode/test/memory/overlay-merge.test.ts index 4b75a7270f..4567cf2319 100644 --- a/packages/opencode/test/memory/overlay-merge.test.ts +++ b/packages/opencode/test/memory/overlay-merge.test.ts @@ -165,11 +165,34 @@ describe("on-demand reload", () => { await hydrate(SES) expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["warehouse/one"]) - const count = await refresh(SES) + const { count, ok } = await refresh(SES) + expect(ok).toBe(true) expect(count).toBe(2) expect(overlayBlocks(SES).map((b) => b.id).sort()).toEqual(["warehouse/one", "warehouse/two"]) }) + test("a failed refresh keeps the memory the session already had", async () => { + // A hydration failure empties the overlay -- correct at session start, + // where there was nothing, but mid-session it would silently destroy + // working memory precisely because the user asked to reload. + listResponse = [remote("keep/me", "STILL HERE")] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["keep/me"]) + + const originalFetch = globalThis.fetch + globalThis.fetch = (async (_i?: unknown, _n?: unknown) => { + throw new Error("network down") + }) as unknown as typeof fetch + try { + const { count, ok } = await refresh(SES) + expect(ok).toBe(false) + expect(count).toBe(1) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["keep/me"]) + } finally { + globalThis.fetch = originalFetch + } + }) + test("refresh drops a block that was archived in the workspace", async () => { listResponse = [remote("gone", "WILL BE ARCHIVED")] await hydrate(SES) diff --git a/packages/opencode/test/memory/store-directory.test.ts b/packages/opencode/test/memory/store-directory.test.ts index 25b18f074f..6dba4db76d 100644 --- a/packages/opencode/test/memory/store-directory.test.ts +++ b/packages/opencode/test/memory/store-directory.test.ts @@ -1,8 +1,13 @@ +// The workspace flag is read at module load, so it must be set before the +// modules under test are imported. +process.env.ALTIMATE_WORKSPACE = "1" + import { describe, test, expect, beforeEach, afterEach } from "bun:test" import fs from "fs/promises" import path from "path" import os from "os" import { MemoryStore } from "@/memory/store" +import { Instance } from "@/project/instance" // Exercises the REAL store, unlike store.test.ts which re-implements its logic // and so cannot catch path-resolution bugs. Callers outside an Instance context From 15fda3912ab046208d22da536244680e3936f292 Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 21 Aug 2026 12:52:55 +0530 Subject: [PATCH 3/5] =?UTF-8?q?fix(workspace):=20address=20consensus=20rev?= =?UTF-8?q?iew=20=E2=80=94=20refresh=20races,=20false=20success,=20by-id?= =?UTF-8?q?=20lookup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **C1 — a stale load could publish over a newer one.** `doHydrate` resolved its write target *after* its awaits by calling `sessionState()` again, so a slow hydration that a refresh had superseded wrote its stale blocks over the fresh result — or, on failure, emptied the session. Loading is now pure (`loadWorkspaceMemory`) and the caller commits into the state that launched it, with a generation check as backstop. `refresh` no longer clears the overlay before fetching, so memory can't blink out of the prompt mid-reload. **C2 — a transient failure reported success and wiped the session.** `refresh` judged failure solely from `hydrateFailed`, which only the `catch` set — but the binding/`memory_enabled` check returns early *without throwing*, and `memoryEnabled` fails closed on any transport error. So an unreachable workspace emptied the overlay and told the user "Reloaded... no memory blocks", which is strictly worse than not reloading. Reads now use a three-way `memoryStatus` (`enabled`/`disabled`/`error`); `refresh` returns a status; the tool says "could not reach the workspace", "not linked" or "memory disabled" accordingly. The write path keeps failing closed. **M3 — concurrent refreshes could discard real memory.** Two refreshes racing let the second capture the first's not-yet-filled state as `previous` and restore emptiness over live memory. `refresh` is now serialized per session. **M1 — `memory_read` still disagreed on the by-id path.** Only the list branch merged the overlay, so looking up a workspace-only block by id — the natural follow-up to seeing it listed — answered "not found" for a block the model was holding. Both branches now share one resolved overlay, and the id path returns every match, since sibling projects may legitimately share an id. It also no longer dies when one scope is unreadable: project scope throws outside an instance, which turned an id lookup into a tool error instead of returning the global and workspace matches. **M2 — directory threading stopped short of the binding.** The mirror confirmed the block against the writing project but still resolved the *binding* from the ambient instance, so project A's memory could upload carrying project B's workspace metadata — a cross-workspace disclosure. The delete path was wholly ambient. Both now resolve `currentBinding(directory)` from the captured directory, and the capture covers global blocks too, whose upload is still gated by the writing project's binding. **M4** gates the refresh tool on `ALTIMATE_WORKSPACE`, so non-pilot users are no longer shipped a tool that can only answer "not enabled". **m2** applies expiry to overlay blocks on read. **m3** marks unsuccessful refreshes `success: false` with a reason, so telemetry stops counting them as successful tool calls. **m5/m6/n1** are cleanups. Every fix above is mutation-checked except the C1 generation check, which is defence-in-depth: the load-owned commit is the actual fix and reverting *that* fails the stale-load test. `MemoryRefreshTool` now has tests for all four branches, which it previously had none of. Tests: 11298 pass. Two failures in the full run are unrelated — `Truncate > cleanup` fails at the merge-base, and `httpapi-session` passes 17/17 alone and 436/436 beside the memory suites. --- .../src/altimate/workspace/memory-sync.ts | 169 ++++++++++++----- packages/opencode/src/memory/store.ts | 10 +- .../opencode/src/memory/tools/memory-read.ts | 82 +++++--- .../src/memory/tools/memory-refresh.ts | 32 +++- packages/opencode/src/tool/registry.ts | 6 +- .../altimate/workspace/memory-sync.test.ts | 1 - .../test/memory/overlay-merge.test.ts | 176 +++++++++++++++++- .../test/memory/store-directory.test.ts | 1 - 8 files changed, 384 insertions(+), 93 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 6d6fcc7b28..9bd2a56f58 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -127,9 +127,13 @@ export function projectKeyFor(binding: CachedBinding): string { return binding.repoRemote ?? binding.projectPath ?? "unknown" } -async function currentBinding(): Promise { +/** ``directory`` is the tree that owned the operation. Fire-and-forget mirrors + * and archives run after their writing context has gone, so resolving the + * binding from the ambient instance can attribute one project's memory to + * another project's workspace. */ +async function currentBinding(directory?: string): Promise { if (syncInternals.resolveBinding) return syncInternals.resolveBinding() - const directory = currentDirectory() + directory = directory ?? currentDirectory() ?? undefined if (!directory) return null try { return await readLocalBinding(directory) @@ -150,7 +154,8 @@ async function currentBinding(): Promise { * anyway. */ const MEMORY_ENABLED_TTL_MS = 60_000 -const memoryEnabledCache = new Map() +/** Exported for tests: the positive TTL is why a failing read can look fine. */ +export const memoryEnabledCache = new Map() /** Warn once per workspace, not once per write. */ const missingFieldWarned = new Set() @@ -160,9 +165,19 @@ const missingFieldWarned = new Set() * The workspace app exposes this as a user-facing toggle, so mirroring into a * workspace with memory disabled would contradict what the user is shown. * Fails closed: if the check cannot be made, nothing is mirrored. */ +/** Fail-closed answer for the WRITE path: an unreachable service must not let a + * mirror through. Reads need to tell "off" from "unreachable" — see + * {@link memoryStatus}. */ async function memoryEnabled(binding: CachedBinding): Promise { + return (await memoryStatus(binding)) === "enabled" +} + +/** Three-way, because a read that cannot reach the service must not be reported + * as "this workspace has no memory" — that reads as success while destroying + * whatever the session already had. */ +async function memoryStatus(binding: CachedBinding): Promise<"enabled" | "disabled" | "error"> { const cached = memoryEnabledCache.get(binding.datamateId) - if (cached && Date.now() - cached.checkedAt < MEMORY_ENABLED_TTL_MS) return true + if (cached && Date.now() - cached.checkedAt < MEMORY_ENABLED_TTL_MS) return "enabled" try { const workspaces = await WorkspaceApi.listDatamates() const match = workspaces.find((w) => w.id === binding.datamateId) @@ -177,13 +192,12 @@ async function memoryEnabled(binding: CachedBinding): Promise { const value = match?.memoryEnabled === true if (value) memoryEnabledCache.set(binding.datamateId, { checkedAt: Date.now() }) else memoryEnabledCache.delete(binding.datamateId) - return value + return value ? "enabled" : "disabled" } catch (err) { - log.warn("could not confirm workspace memory setting, skipping mirror", { err: String(err) }) + log.warn("could not confirm workspace memory setting", { err: String(err) }) // Not cached either way: a transient failure should neither disable the - // mirror for a minute nor keep it enabled. Failing closed already prevents - // this particular write. - return false + // mirror for a minute nor keep it enabled. + return "error" } } @@ -476,7 +490,7 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi // with nothing to consult and nothing to attribute it to. Global blocks still // carry no workspace themselves, so they apply everywhere on read; the // binding governs only whether we upload at all. - const binding = await currentBinding() + const binding = await currentBinding(directory) if (!binding) return if (!(await memoryEnabled(binding))) return await push(block, binding, undefined, directory) @@ -486,13 +500,19 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi /** Archive a block's cloud record rather than deleting it, so the workspace * keeps the history. Only this client filters the marker — other readers do * not — so an archived record stays visible elsewhere. */ -export async function archiveBlock(scope: "global" | "project", blockId: string): Promise { +export async function archiveBlock( + scope: "global" | "project", + blockId: string, + directory?: string, +): Promise { if (!isEnabled()) return // Queued behind any in-flight mirror for the same block, so a delete cannot // run before the create it is meant to undo. The binding lookup happens // inside the queued op for the same reason as in `mirrorBlock`. return serialize(scope, blockId, async () => { - const binding = await currentBinding() + // Same capture as the mirror: the delete's own project decides which + // workspace record is archived, not whichever instance is current now. + const binding = await currentBinding(directory) if (!binding) return if (!(await memoryEnabled(binding))) return await archiveNow(scope, blockId, binding) @@ -612,7 +632,7 @@ export async function backfill( return { ok: 0, failed: 0, skipped: blocks.length, declined: 0, gated: true } const index = await readIndex() - const pending: { block: MemoryBlock; binding: CachedBinding | null; directory?: string }[] = [] + const pending: { block: MemoryBlock; binding: CachedBinding | null }[] = [] let skipped = 0 for (const block of blocks) { const target = block.scope === "project" ? binding : null @@ -630,7 +650,7 @@ export async function backfill( skipped++ continue } - pending.push({ block, binding: target, directory: sweepDirectory }) + pending.push({ block, binding: target }) } if (pending.length === 0) return { ok: 0, failed: 0, skipped, declined: 0, gated: false } @@ -652,7 +672,7 @@ export async function backfill( pending, (item) => serialize(item.block.scope, item.block.id, () => - push(item.block, item.binding, known, item.directory), + push(item.block, item.binding, known, sweepDirectory), ), BACKFILL_CONCURRENCY, ) @@ -740,7 +760,9 @@ export async function hydrate(sessionID: string): Promise { if (!isEnabled()) return const state = sessionState(sessionID) if (state.hydration) return state.hydration - state.hydration = doHydrate(sessionID) + // The overlay is deliberately NOT cleared before loading: clearing first made + // workspace memory blink out of the prompt whenever a fetch ran long. + state.hydration = loadWorkspaceMemory().then((outcome) => commitLoad(sessionID, state, outcome)) return state.hydration } @@ -776,16 +798,29 @@ export async function whenHydrated( } } -async function doHydrate(sessionID: string): Promise { +/** What a load attempt actually concluded. + * + * "nothing to load" and "could not load" must stay distinguishable: collapsing + * them is how a transient failure gets reported as a successful reload of an + * empty workspace, taking the session's real memory with it. */ +type LoadOutcome = + | { status: "loaded"; blocks: RemoteMemoryBlock[] } + | { status: "unlinked" } + | { status: "disabled" } + | { status: "error" } + +/** Read this project's workspace memory. Pure: it publishes nothing, so a slow + * load that has been superseded cannot write over a newer result. */ +async function loadWorkspaceMemory(): Promise { try { const binding = await currentBinding() - if (!binding || !(await memoryEnabled(binding))) { - sessionState(sessionID).overlay = [] - return - } - const ownProjectKey = binding ? projectKeyFor(binding) : undefined - const ownWorkspace = binding ? String(binding.datamateId) : undefined + if (!binding) return { status: "unlinked" } + const enabled = await memoryStatus(binding) + if (enabled === "error") return { status: "error" } + if (enabled === "disabled") return { status: "disabled" } + const ownProjectKey = projectKeyFor(binding) + const ownWorkspace = String(binding.datamateId) const records = await MemoryApi.list() const blocks: RemoteMemoryBlock[] = [] @@ -800,18 +835,27 @@ async function doHydrate(sessionID: string): Promise { if (block.expires && new Date(block.expires) <= new Date()) continue blocks.push(block) } - - const done = sessionState(sessionID) - done.overlay = blocks - done.hydrateFailed = false - if (blocks.length > 0) { - log.info("workspace memory hydrated", { blocks: blocks.length, workspace: binding?.datamateName }) - } + return { status: "loaded", blocks } } catch (err) { - log.warn("workspace memory hydration failed", { err: String(err) }) - const failed = sessionState(sessionID) - failed.overlay = [] - failed.hydrateFailed = true + log.warn("workspace memory load failed", { err: String(err) }) + return { status: "error" } + } +} + +/** Publish a load result into the state that launched it. + * + * The generation check is the point: ``refresh`` replaces a session's state, and + * an older in-flight load must not write into the newer one. */ +function commitLoad(sessionID: string, state: SessionMemory, outcome: LoadOutcome): void { + if (sessions.get(sessionID) !== state) return + if (outcome.status === "error") { + state.hydrateFailed = true + return + } + state.hydrateFailed = false + state.overlay = outcome.status === "loaded" ? outcome.blocks : [] + if (outcome.status === "loaded" && outcome.blocks.length > 0) { + log.info("workspace memory hydrated", { blocks: outcome.blocks.length }) } } @@ -828,22 +872,47 @@ export function overlayBlocks(sessionID: string): RemoteMemoryBlock[] { * (or this user on another machine) wrote a block never sees it. This is the * on-demand path: drop the session's state so the next hydrate genuinely * refetches. Returns how many blocks the session now holds. */ -export async function refresh(sessionID: string): Promise<{ count: number; ok: boolean }> { - if (!isEnabled()) return { count: 0, ok: false } - // Keep what the session already has. A failed fetch empties the overlay - // (hydration failure is indistinguishable from an empty workspace at session - // start, where [] is correct) -- but mid-session that would silently destroy - // working memory because the user asked to reload and the network hiccuped. - const previous = overlayBlocks(sessionID) - // Clears overlay, hydration promise and the timed-out latch together. - sessions.delete(sessionID) - await hydrate(sessionID) - const state = sessions.get(sessionID) - if (state?.hydrateFailed) { - state.overlay = previous - return { count: previous.length, ok: false } - } - return { count: overlayBlocks(sessionID).length, ok: true } +export type RefreshResult = { + count: number + ok: boolean + status: "loaded" | "unlinked" | "disabled" | "error" | "off" +} + +/** Re-read this session's workspace memory, discarding what it already holds. + * + * ``hydrate`` is idempotent for the life of a session, which keeps the per-turn + * call cheap -- but it also means a session started before a teammate (or this + * user on another machine) wrote a block never sees it. This is the on-demand + * path. + * + * Serialized per session: two refreshes racing would otherwise let the second + * capture the first's not-yet-filled state as "previous" and, on failure, + * restore emptiness over real memory. */ +export async function refresh(sessionID: string): Promise { + if (!isEnabled()) return { count: 0, ok: false, status: "off" } + return serialize("global", `refresh:${sessionID}`, async () => { + const previous = overlayBlocks(sessionID) + const outcome = await loadWorkspaceMemory() + if (outcome.status === "error") { + // Keep what the session had. Emptying it because the network hiccuped is + // strictly worse than not reloading, and the user asked for a reload. + const state = sessionState(sessionID) + state.overlay = previous + state.hydrateFailed = true + return { count: previous.length, ok: false, status: "error" } + } + // Replace the session's state so any older in-flight hydration is orphaned + // by commitLoad's generation check rather than overwriting this result. + sessions.delete(sessionID) + const state = sessionState(sessionID) + state.hydration = Promise.resolve() + commitLoad(sessionID, state, outcome) + return { + count: state.overlay.length, + ok: outcome.status === "loaded", + status: outcome.status, + } + }) } /** Forget a session's hydration, or all of them. diff --git a/packages/opencode/src/memory/store.ts b/packages/opencode/src/memory/store.ts index 325e27629b..7f3385d2d8 100644 --- a/packages/opencode/src/memory/store.ts +++ b/packages/opencode/src/memory/store.ts @@ -317,7 +317,7 @@ export namespace MemoryStore { // ambient instance may be a different project -- and the mirror's // local-existence check would then look for this block in the wrong tree // and skip it as deleted. - const owningDirectory = block.scope === "project" ? safeDirectory() : undefined + const owningDirectory = safeDirectory() void mirrorBlock(block, owningDirectory).catch((e) => { mirrorLog.warn("failed to mirror memory block to workspace", { id: block.id, @@ -355,8 +355,12 @@ export namespace MemoryStore { tags_count: 0, }) // altimate_change start - archive rather than delete the cloud record so - // the workspace keeps the history. Fire-and-forget, as with write. - void archiveBlock(scope, id).catch((e) => { + // the workspace keeps the history. Fire-and-forget, as with write, so the + // deleting project's directory is captured here while its context is + // still current — otherwise the archive resolves another project's + // binding and can hit that workspace's same-id record. + const deletingDirectory = safeDirectory() + void archiveBlock(scope, id, deletingDirectory).catch((e) => { mirrorLog.warn("failed to archive workspace memory record", { id, scope, diff --git a/packages/opencode/src/memory/tools/memory-read.ts b/packages/opencode/src/memory/tools/memory-read.ts index e595e65eef..1791ef3557 100644 --- a/packages/opencode/src/memory/tools/memory-read.ts +++ b/packages/opencode/src/memory/tools/memory-read.ts @@ -3,8 +3,8 @@ import { Tool } from "../../tool/tool" import { MemoryStore, isExpired } from "../store" import { MemoryPrompt, mergeOverlay } from "../prompt" // altimate_change - workspace memory overlay -import { overlayBlocks, whenHydrated } from "@/altimate/workspace/memory-sync" -import { MemoryBlockSchema } from "../types" +import { overlayBlocks, whenHydrated, type RemoteMemoryBlock } from "@/altimate/workspace/memory-sync" +import { MemoryBlockSchema, type MemoryBlock } from "../types" export const MemoryReadTool = Tool.define("altimate_memory_read", { description: @@ -25,26 +25,66 @@ export const MemoryReadTool = Tool.define("altimate_memory_read", { }), async execute(args, ctx) { try { + // altimate_change start — this session's workspace memory, resolved once + // for both branches. Reading only the local store made this tool disagree + // with what the model was actually given: injection merges the overlay. + let remote: RemoteMemoryBlock[] = [] + if (ctx?.sessionID) { + await whenHydrated(ctx.sessionID) + remote = overlayBlocks(ctx.sessionID).filter((b) => { + if (args.scope !== "all" && b.scope !== args.scope) return false + // Overlay blocks are expiry-checked at hydrate time and never again, + // so honour include_expired here as the local list does. + return args.include_expired || !isExpired(b) + }) + } + // altimate_change end if (args.id) { const scopes: Array<"global" | "project"> = args.scope === "all" ? ["project", "global"] : [args.scope as "global" | "project"] + const matches: (MemoryBlock & { origin?: string })[] = [] for (const scope of scopes) { - const block = await MemoryStore.read(scope, args.id) - if (block) { - // Respect include_expired for ID reads - if (!args.include_expired && isExpired(block)) continue - return { - title: `Memory: ${block.id} (${block.scope})`, - metadata: { count: 1 }, - output: MemoryPrompt.formatBlock(block), - } + // One scope failing must not take the others down with it. Project + // scope throws outside an instance context, which would otherwise + // turn an id lookup into a tool error instead of returning the + // global and workspace matches. `listAll` already behaves this way. + let block: MemoryBlock | undefined + try { + block = await MemoryStore.read(scope, args.id) + } catch { + continue + } + if (!block) continue + // Respect include_expired for ID reads + if (!args.include_expired && isExpired(block)) continue + matches.push(block) + } + // altimate_change start — a workspace-only block is in the model's + // prompt, so it will be looked up by id; answering "not found" for a + // block the model is holding is the exact disagreement this closes. + // Sibling projects may legitimately share an id, so return every match. + const localKeys = new Set(matches.map((b) => `${b.scope}:${b.id}`)) + for (const block of remote) { + if (block.id !== args.id) continue + if (block.origin === undefined && localKeys.has(`${block.scope}:${block.id}`)) continue + matches.push(block) + } + // altimate_change end + if (matches.length === 0) { + return { + title: "Memory: not found", + metadata: { count: 0 }, + output: `No memory block found with ID "${args.id}"`, } } return { - title: "Memory: not found", - metadata: { count: 0 }, - output: `No memory block found with ID "${args.id}"`, + title: + matches.length === 1 + ? `Memory: ${matches[0].id} (${matches[0].scope})` + : `Memory: ${args.id} (${matches.length} blocks)`, + metadata: { count: matches.length }, + output: matches.map((b) => MemoryPrompt.formatBlock(b)).join("\n\n"), } } @@ -54,18 +94,8 @@ export const MemoryReadTool = Tool.define("altimate_memory_read", { ? await MemoryStore.listAll(listOpts) : await MemoryStore.list(args.scope as "global" | "project", listOpts) - // altimate_change start — fold in this session's workspace memory. Reading - // only the local store made this tool disagree with what the model was - // actually given: injection merges the overlay, so a user asking "what do - // you remember?" saw strictly less than the prompt contained. - if (ctx?.sessionID) { - await whenHydrated(ctx.sessionID) - const remote = overlayBlocks(ctx.sessionID).filter( - (b) => args.scope === "all" || b.scope === args.scope, - ) - blocks = mergeOverlay(blocks, remote) - } - // altimate_change end + // altimate_change - fold in this session's workspace memory (resolved above) + if (remote.length > 0) blocks = mergeOverlay(blocks, remote) if (args.tags && args.tags.length > 0) { blocks = blocks.filter((b) => args.tags!.every((tag) => b.tags.includes(tag))) diff --git a/packages/opencode/src/memory/tools/memory-refresh.ts b/packages/opencode/src/memory/tools/memory-refresh.ts index 29286479fe..7c6bdf52bd 100644 --- a/packages/opencode/src/memory/tools/memory-refresh.ts +++ b/packages/opencode/src/memory/tools/memory-refresh.ts @@ -27,31 +27,43 @@ export const MemoryRefreshTool = Tool.define("altimate_memory_refresh", { if (!isEnabled()) { return { title: "Memory: workspace sync off", - metadata: { count: 0, refreshed: false }, + metadata: { success: false, refreshed: false, count: 0, reason: "off" }, output: "Workspace memory is not enabled, so there is nothing to reload.", } } if (!ctx?.sessionID) { return { title: "Memory: no session", - metadata: { count: 0, refreshed: false }, + metadata: { success: false, refreshed: false, count: 0, reason: "no-session" }, output: "No session context, so there is no memory overlay to reload.", } } try { - const { count, ok } = await refresh(ctx.sessionID) + const { count, ok, status } = await refresh(ctx.sessionID) if (!ok) { - // The session keeps whatever it already had -- say so rather than - // reporting a count the user might read as a successful reload. + // Each of these is a distinct thing to tell the user. Collapsing them + // into "reloaded, nothing here" is how an unreachable workspace reads + // as an empty one. + const message = + status === "error" + ? `Could not reach the workspace, so memory was not reloaded. This session still has its existing ${count} block(s).` + : status === "unlinked" + ? "This project is not linked to a workspace, so there is no workspace memory to load." + : "This workspace has memory turned off, so there is nothing to load." return { - title: "Memory: could not reach the workspace", - metadata: { count, refreshed: false }, - output: `Could not reach the workspace, so memory was not reloaded. This session still has its existing ${count} block(s).`, + title: + status === "error" + ? "Memory: could not reach the workspace" + : status === "unlinked" + ? "Memory: project not linked" + : "Memory: workspace memory disabled", + metadata: { success: false, refreshed: false, count, reason: status }, + output: message, } } return { title: `Memory: reloaded ${count} workspace block(s)`, - metadata: { count, refreshed: true }, + metadata: { success: true, refreshed: true, count, reason: status }, output: count === 0 ? "Reloaded workspace memory. This workspace has no memory blocks visible to this project." @@ -61,7 +73,7 @@ export const MemoryRefreshTool = Tool.define("altimate_memory_refresh", { // Never fail the turn over a refresh — the session keeps whatever it had. return { title: "Memory: reload failed", - metadata: { count: 0, refreshed: false }, + metadata: { success: false, refreshed: false, count: 0, reason: "exception" }, output: `Could not reload workspace memory: ${e instanceof Error ? e.message : String(e)}`, } } diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index a3be18e6a4..876fca5512 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -134,6 +134,7 @@ import { SampleSetupTool } from "../altimate/tools/sample-setup" // altimate_change start - import altimate persistent memory tools import { MemoryReadTool } from "../memory/tools/memory-read" import { MemoryRefreshTool } from "../memory/tools/memory-refresh" +import { Flag as CoreFlag } from "@opencode-ai/core/flag/flag" import { MemoryWriteTool } from "../memory/tools/memory-write" import { MemoryDeleteTool } from "../memory/tools/memory-delete" import { MemoryAuditTool } from "../memory/tools/memory-audit" @@ -469,7 +470,10 @@ export namespace ToolRegistry { ...(!Flag.ALTIMATE_DISABLE_MEMORY ? [ MemoryReadTool, - MemoryRefreshTool, + // Workspace-only: `refresh` no-ops without the pilot flag, so + // shipping its description to every user costs a tool slot and + // invites a wasted call that can only answer "not enabled". + ...(CoreFlag.ALTIMATE_WORKSPACE ? [MemoryRefreshTool] : []), MemoryWriteTool, MemoryDeleteTool, MemoryAuditTool, diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 6d9441945a..774366449c 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -413,7 +413,6 @@ describe("mirrorBlock", () => { // ambient directory then finds nothing and skips a perfectly good write as // "deleted". The directory captured at write time has to win. const seen: (string | undefined)[] = [] - syncInternals.blockExists = async () => true const b = block({ id: "owned-elsewhere", scope: "project" }) const { MemoryStore } = await import("../../../src/memory/store") const origRead = MemoryStore.read diff --git a/packages/opencode/test/memory/overlay-merge.test.ts b/packages/opencode/test/memory/overlay-merge.test.ts index 4567cf2319..3e6482c818 100644 --- a/packages/opencode/test/memory/overlay-merge.test.ts +++ b/packages/opencode/test/memory/overlay-merge.test.ts @@ -34,9 +34,10 @@ afterAll(() => { const { MemoryPrompt, mergeOverlay } = await import("../../src/memory/prompt") const { MemoryStore } = await import("../../src/memory/store") const { MemoryReadTool } = await import("../../src/memory/tools/memory-read") +const { MemoryRefreshTool } = await import("../../src/memory/tools/memory-refresh") const { initTool } = await import("../altimate/tool-fixture") const { MIRROR_SOURCE } = await import("../../src/altimate/workspace/memory-api") -const { hydrate, refresh, resetOverlay, overlayBlocks, syncInternals } = await import( +const { hydrate, refresh, resetOverlay, overlayBlocks, syncInternals, memoryEnabledCache } = await import( "../../src/altimate/workspace/memory-sync" ) const { TrainingStore } = await import("../../src/altimate/training/store") @@ -130,7 +131,87 @@ const remote = (id: string, content: string, extra: Record = {} metadata: { source: MIRROR_SOURCE, block_id: id, block_scope: "global", ...extra }, }) +describe("the memory refresh tool", () => { + test("reports what it reloaded", async () => { + listResponse = [remote("a/one", "ONE"), remote("a/two", "TWO")] + const tool = await initTool(MemoryRefreshTool) + const res: any = await tool.execute({}, { sessionID: SES, agent: "build" }) + expect(res.metadata.success).toBe(true) + expect(res.metadata.count).toBe(2) + expect(res.output).toContain("2 block(s)") + }) + + test("says the workspace is unreachable rather than reporting an empty reload", async () => { + listResponse = [remote("keep/one", "KEEP")] + await hydrate(SES) + memoryEnabledCache.clear() + const originalFetch = globalThis.fetch + globalThis.fetch = (async (_i?: unknown, _n?: unknown) => { + throw new Error("unreachable") + }) as unknown as typeof fetch + try { + const tool = await initTool(MemoryRefreshTool) + const res: any = await tool.execute({}, { sessionID: SES, agent: "build" }) + expect(res.metadata.success).toBe(false) + expect(res.metadata.reason).toBe("error") + expect(res.output).toContain("Could not reach the workspace") + // and the memory is still there + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["keep/one"]) + } finally { + globalThis.fetch = originalFetch + } + }) + + test("says the project is not linked", async () => { + syncInternals.resolveBinding = async () => null + try { + const tool = await initTool(MemoryRefreshTool) + const res: any = await tool.execute({}, { sessionID: SES, agent: "build" }) + expect(res.metadata.success).toBe(false) + expect(res.metadata.reason).toBe("unlinked") + expect(res.output).toContain("not linked to a workspace") + } finally { + syncInternals.resolveBinding = async () => BINDING as any + } + }) + + test("without a session there is no overlay to reload", async () => { + const tool = await initTool(MemoryRefreshTool) + const res: any = await tool.execute({}, { agent: "build" }) + expect(res.metadata.success).toBe(false) + expect(res.metadata.reason).toBe("no-session") + }) +}) + describe("the memory read tool", () => { + test("finds a workspace-only block by id, instead of answering 'not found'", async () => { + // The model sees workspace blocks in its prompt, so it looks them up by id. + // Answering "not found" for a block it is holding is the exact + // disagreement this change set out to close. + listResponse = [remote("warehouse/sizing", "REMOTE ONLY BLOCK")] + await hydrate(SES) + const tool = await initTool(MemoryReadTool) + const res: any = await tool.execute({ id: "warehouse/sizing", scope: "all" }, { sessionID: SES, agent: "build" }) + expect(res.metadata.count).toBe(1) + expect(String(res.output)).toContain("REMOTE ONLY BLOCK") + }) + + test("a workspace block that expired mid-session is filtered on read", async () => { + // Overlay blocks are expiry-checked once at hydrate time and never again, + // so a block whose TTL passes mid-session would otherwise show up under + // both settings of include_expired. + listResponse = [remote("ttl/soon", "EXPIRES SOON", { block_expires: "2099-01-01T00:00:00.000Z" })] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["ttl/soon"]) + // Age it past its TTL without a refetch, exactly as wall-clock would. + ;(overlayBlocks(SES)[0] as any).expires = "2020-01-01T00:00:00.000Z" + const state = (await import("../../src/altimate/workspace/memory-sync")) as any + void state + const tool = await initTool(MemoryReadTool) + const res: any = await tool.execute({ scope: "all" }, { sessionID: SES, agent: "build" }) + expect(String(res.output ?? "")).not.toContain("EXPIRES SOON") + }) + test("surfaces workspace memory, not just the local store", async () => { // The tool read MemoryStore only, so it disagreed with what the model was // actually given -- injection merges the overlay and the tool did not. @@ -193,6 +274,99 @@ describe("on-demand reload", () => { } }) + test("a workspace that cannot be reached keeps the session's memory and reports failure", async () => { + // The killer case: `memoryEnabled` fails CLOSED on a transport error, and + // that path never threw — so refresh used to report a successful reload of + // an empty workspace while destroying what the session held. + listResponse = [remote("keep/me", "STILL HERE")] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["keep/me"]) + + memoryEnabledCache.clear() // past the 60s positive-cache TTL + const originalFetch = globalThis.fetch + globalThis.fetch = (async (_i?: unknown, _n?: unknown) => { + throw new Error("workspace unreachable") + }) as unknown as typeof fetch + try { + const res = await refresh(SES) + expect(res.ok).toBe(false) + expect(res.status).toBe("error") + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["keep/me"]) + } finally { + globalThis.fetch = originalFetch + } + }) + + test("an unlinked project reports 'unlinked', not an empty reload", async () => { + syncInternals.resolveBinding = async () => null + try { + const res = await refresh(SES) + expect(res.ok).toBe(false) + expect(res.status).toBe("unlinked") + } finally { + syncInternals.resolveBinding = async () => BINDING as any + } + }) + + test("a stale in-flight load cannot overwrite a newer refresh", async () => { + // hydrate A stalls; refresh replaces the session's state; A then resolves. + // Without the generation check A publishes its stale blocks over B's. + let release: (() => void) | undefined + const gate = new Promise((r) => (release = r)) + const originalFetch = globalThis.fetch + let firstCall = true + globalThis.fetch = (async (input: any, init?: any) => { + if (firstCall && String(input).includes("/datamates/memory/list")) { + firstCall = false + await gate + return new Response(JSON.stringify([remote("stale/one", "STALE")]), { + status: 200, headers: { "Content-Type": "application/json" }, + }) + } + return originalFetch(input, init) + }) as typeof fetch + + try { + void hydrate(SES) // A — stalls on the gate + await new Promise((r) => setTimeout(r, 20)) + listResponse = [remote("fresh/one", "FRESH")] + const res = await refresh(SES) // B — completes first + expect(res.status).toBe("loaded") + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["fresh/one"]) + + release?.() // A resolves late + await new Promise((r) => setTimeout(r, 50)) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["fresh/one"]) + } finally { + globalThis.fetch = originalFetch + } + }) + + test("two concurrent refreshes cannot discard the real overlay", async () => { + listResponse = [remote("real/one", "REAL")] + await hydrate(SES) + expect(overlayBlocks(SES).map((b) => b.id)).toEqual(["real/one"]) + + // One succeeds, one fails; the failing one must restore REAL memory, not + // the other's not-yet-filled empty state. + const originalFetch = globalThis.fetch + let n = 0 + globalThis.fetch = (async (input: any, init?: any) => { + if (String(input).includes("/datamates/memory/list")) { + n++ + if (n === 2) throw new Error("second refresh fails") + } + return originalFetch(input, init) + }) as typeof fetch + try { + const [a, b] = await Promise.all([refresh(SES), refresh(SES)]) + expect([a.ok, b.ok].filter(Boolean).length).toBeGreaterThanOrEqual(1) + expect(overlayBlocks(SES).map((x) => x.id)).toEqual(["real/one"]) + } finally { + globalThis.fetch = originalFetch + } + }) + test("refresh drops a block that was archived in the workspace", async () => { listResponse = [remote("gone", "WILL BE ARCHIVED")] await hydrate(SES) diff --git a/packages/opencode/test/memory/store-directory.test.ts b/packages/opencode/test/memory/store-directory.test.ts index 6dba4db76d..4cf6b0e529 100644 --- a/packages/opencode/test/memory/store-directory.test.ts +++ b/packages/opencode/test/memory/store-directory.test.ts @@ -7,7 +7,6 @@ import fs from "fs/promises" import path from "path" import os from "os" import { MemoryStore } from "@/memory/store" -import { Instance } from "@/project/instance" // Exercises the REAL store, unlike store.test.ts which re-implements its logic // and so cannot catch path-resolution bugs. Callers outside an Instance context From 58071c8d3d11850a87c97c238e36474608a4ec6c Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 21 Aug 2026 22:17:07 +0530 Subject: [PATCH 4/5] fix(workspace): narrow the id-lookup error suppression, stop the test flag leaking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review on the current head. **An unreadable store reported "not found".** The id path swallowed every read error so that project scope, which throws outside an instance context, could not hide a global or workspace match. But probing both scopes is our choice only when the caller passes `scope: "all"` — when they name a scope, a failure there is a real error they need to see, not missing memory. Suppression is now limited to exactly the project-scope-under-`all` case. **The test flag leaked into the process.** `store-directory.test.ts` set `ALTIMATE_WORKSPACE=1` at module load and never restored it, so unrelated suites sharing the process inherited the pilot flag. Saved and restored in teardown, deleting it when it was previously unset. Both mutation-checked. The reviewer's other two findings were raised against `de4201f7b` and are already fixed in `568b7cfae9`: the id path merges the overlay (it no longer returns before the merge), and hydration commits into the state that launched it, so a stale in-flight fetch cannot clobber a refreshed overlay. Tests: 4481 pass across memory + altimate. --- .../opencode/src/memory/tools/memory-read.ts | 14 +++++++------ .../test/memory/overlay-merge.test.ts | 21 +++++++++++++++++++ .../test/memory/store-directory.test.ts | 11 ++++++++-- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/packages/opencode/src/memory/tools/memory-read.ts b/packages/opencode/src/memory/tools/memory-read.ts index 1791ef3557..f87eb5d8a9 100644 --- a/packages/opencode/src/memory/tools/memory-read.ts +++ b/packages/opencode/src/memory/tools/memory-read.ts @@ -45,15 +45,17 @@ export const MemoryReadTool = Tool.define("altimate_memory_read", { const matches: (MemoryBlock & { origin?: string })[] = [] for (const scope of scopes) { - // One scope failing must not take the others down with it. Project - // scope throws outside an instance context, which would otherwise - // turn an id lookup into a tool error instead of returning the - // global and workspace matches. `listAll` already behaves this way. + // Narrowly suppressed: probing BOTH scopes is our choice, not the + // caller's, and project scope throws outside an instance context -- + // that must not hide a global or workspace match. A scope the caller + // asked for explicitly failing is a real error they need to see, + // rather than a misleading "not found". let block: MemoryBlock | undefined try { block = await MemoryStore.read(scope, args.id) - } catch { - continue + } catch (e) { + if (args.scope === "all" && scope === "project") continue + throw e } if (!block) continue // Respect include_expired for ID reads diff --git a/packages/opencode/test/memory/overlay-merge.test.ts b/packages/opencode/test/memory/overlay-merge.test.ts index 3e6482c818..8439b8c71e 100644 --- a/packages/opencode/test/memory/overlay-merge.test.ts +++ b/packages/opencode/test/memory/overlay-merge.test.ts @@ -212,6 +212,27 @@ describe("the memory read tool", () => { expect(String(res.output ?? "")).not.toContain("EXPIRES SOON") }) + test("an explicitly requested scope that cannot be read is an error, not 'not found'", async () => { + // Probing both scopes is our choice when scope=all, so a project-scope + // failure there is suppressed. When the caller names a scope, swallowing + // the failure would report missing memory for a store that is merely + // unreadable. + const { MemoryStore } = await import("../../src/memory/store") + const original = MemoryStore.read + ;(MemoryStore as any).read = async () => { + throw new Error("store unreadable") + } + try { + const tool = await initTool(MemoryReadTool) + const res: any = await tool.execute({ id: "x", scope: "project" }, { sessionID: SES, agent: "build" }) + const text = String(res.output ?? "") + expect(text).not.toContain("No memory block found") + expect(text).toContain("store unreadable") + } finally { + ;(MemoryStore as any).read = original + } + }) + test("surfaces workspace memory, not just the local store", async () => { // The tool read MemoryStore only, so it disagreed with what the model was // actually given -- injection merges the overlay and the tool did not. diff --git a/packages/opencode/test/memory/store-directory.test.ts b/packages/opencode/test/memory/store-directory.test.ts index 4cf6b0e529..26d19e1d71 100644 --- a/packages/opencode/test/memory/store-directory.test.ts +++ b/packages/opencode/test/memory/store-directory.test.ts @@ -1,8 +1,10 @@ // The workspace flag is read at module load, so it must be set before the -// modules under test are imported. +// modules under test are imported -- and restored afterwards so it does not +// leak into unrelated suites sharing this process. +const ORIGINAL_WORKSPACE_FLAG = process.env.ALTIMATE_WORKSPACE process.env.ALTIMATE_WORKSPACE = "1" -import { describe, test, expect, beforeEach, afterEach } from "bun:test" +import { describe, test, expect, beforeEach, afterEach, afterAll } from "bun:test" import fs from "fs/promises" import path from "path" import os from "os" @@ -12,6 +14,11 @@ import { MemoryStore } from "@/memory/store" // and so cannot catch path-resolution bugs. Callers outside an Instance context // -- the `link` subcommand is one -- pass `directory` explicitly; every step of // the read path has to honour it, not just the directory scan. +afterAll(() => { + if (ORIGINAL_WORKSPACE_FLAG === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_WORKSPACE_FLAG +}) + describe("MemoryStore project scope with an explicit directory", () => { let proj: string From 50a70dbc8885089b4a1d31654471cececdfeaf29 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 24 Aug 2026 11:26:30 +0530 Subject: [PATCH 5/5] fix(workspace): drop dead hydrateFailed field + duplicate RefreshResult doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cleanups on packages/opencode/src/altimate/workspace/memory-sync.ts: 1. Remove the `hydrateFailed` field on `SessionMemory` and its three write sites. The `refresh` restructure replaced the read with a direct `outcome.status === "error"` check, so the field is written but never read. (kilo-code-bot #1123 comment 3840696504.) 2. Remove the doc comment above `export type RefreshResult` — it duplicated the doc on `refresh` (line 867) and had drifted stale (still claimed "Returns how many blocks the session now holds", but `refresh` returns a `RefreshResult`). The doc was also attached to the wrong element (the type, not the function). (kilo-code-bot #1123 comment 3840696513.) --- .../src/altimate/workspace/memory-sync.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 9bd2a56f58..cabf56358d 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -73,9 +73,6 @@ interface SessionMemory { touchedAt: number /** Set once a bounded wait expired, so later injections do not re-wait. */ waitTimedOut?: boolean - /** Set when the last fetch for this session failed, so a caller can tell an - * empty workspace apart from an unreadable one. */ - hydrateFailed?: boolean } const sessions = new Map() @@ -848,11 +845,7 @@ async function loadWorkspaceMemory(): Promise { * an older in-flight load must not write into the newer one. */ function commitLoad(sessionID: string, state: SessionMemory, outcome: LoadOutcome): void { if (sessions.get(sessionID) !== state) return - if (outcome.status === "error") { - state.hydrateFailed = true - return - } - state.hydrateFailed = false + if (outcome.status === "error") return state.overlay = outcome.status === "loaded" ? outcome.blocks : [] if (outcome.status === "loaded" && outcome.blocks.length > 0) { log.info("workspace memory hydrated", { blocks: outcome.blocks.length }) @@ -865,13 +858,6 @@ export function overlayBlocks(sessionID: string): RemoteMemoryBlock[] { return [...(sessions.get(sessionID)?.overlay ?? [])] } -/** Re-read this session's workspace memory, discarding what it already holds. - * - * ``hydrate`` is idempotent for the life of a session, which is what keeps the - * per-turn call cheap -- but it also means a session started before a teammate - * (or this user on another machine) wrote a block never sees it. This is the - * on-demand path: drop the session's state so the next hydrate genuinely - * refetches. Returns how many blocks the session now holds. */ export type RefreshResult = { count: number ok: boolean @@ -898,7 +884,6 @@ export async function refresh(sessionID: string): Promise { // strictly worse than not reloading, and the user asked for a reload. const state = sessionState(sessionID) state.overlay = previous - state.hydrateFailed = true return { count: previous.length, ok: false, status: "error" } } // Replace the session's state so any older in-flight hydration is orphaned