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 4c0b59f4d4..cabf56358d 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -107,7 +107,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 @@ -124,9 +124,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) @@ -147,7 +151,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() @@ -157,9 +162,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) @@ -174,13 +189,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" } } @@ -304,11 +318,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 +339,7 @@ async function push( block: MemoryBlock, binding: CachedBinding | null, known?: KnownRecords, + directory?: string, ): Promise { const key = indexKey({ scope: block.scope, @@ -358,7 +376,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 +475,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 @@ -469,23 +487,29 @@ export async function mirrorBlock(block: MemoryBlock): Promise { // 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) + await push(block, binding, undefined, directory) }) } /** 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) @@ -592,6 +616,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 @@ -642,7 +667,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, sweepDirectory), + ), BACKFILL_CONCURRENCY, ) // `skipped` combines blocks filtered before the queue (already synced, or @@ -729,7 +757,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 } @@ -765,16 +795,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[] = [] @@ -789,14 +832,23 @@ async function doHydrate(sessionID: string): Promise { if (block.expires && new Date(block.expires) <= new Date()) continue blocks.push(block) } - - sessionState(sessionID).overlay = blocks - 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) }) - sessionState(sessionID).overlay = [] + 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") 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 }) } } @@ -806,6 +858,48 @@ export function overlayBlocks(sessionID: string): RemoteMemoryBlock[] { return [...(sessions.get(sessionID)?.overlay ?? [])] } +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 + 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. * * 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/store.ts b/packages/opencode/src/memory/store.ts index da4b0bf64b..7f3385d2d8 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 = safeDirectory() + void mirrorBlock(block, owningDirectory).catch((e) => { mirrorLog.warn("failed to mirror memory block to workspace", { id: block.id, scope: block.scope, @@ -340,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 e8cd063421..f87eb5d8a9 100644 --- a/packages/opencode/src/memory/tools/memory-read.ts +++ b/packages/opencode/src/memory/tools/memory-read.ts @@ -1,8 +1,10 @@ import z from "zod" import { Tool } from "../../tool/tool" import { MemoryStore, isExpired } from "../store" -import { MemoryPrompt } from "../prompt" -import { MemoryBlockSchema } from "../types" +import { MemoryPrompt, mergeOverlay } from "../prompt" +// altimate_change - workspace memory overlay +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: @@ -23,26 +25,68 @@ 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), - } + // 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 (e) { + if (args.scope === "all" && scope === "project") continue + throw e + } + 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"), } } @@ -52,6 +96,9 @@ export const MemoryReadTool = Tool.define("altimate_memory_read", { ? await MemoryStore.listAll(listOpts) : await MemoryStore.list(args.scope as "global" | "project", listOpts) + // 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 new file mode 100644 index 0000000000..7c6bdf52bd --- /dev/null +++ b/packages/opencode/src/memory/tools/memory-refresh.ts @@ -0,0 +1,81 @@ +// 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: { 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: { 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, status } = await refresh(ctx.sessionID) + if (!ok) { + // 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: + 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: { success: true, refreshed: true, count, reason: status }, + 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: { 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 20e8bd1ba6..876fca5512 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -133,6 +133,8 @@ 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" @@ -468,6 +470,10 @@ export namespace ToolRegistry { ...(!Flag.ALTIMATE_DISABLE_MEMORY ? [ MemoryReadTool, + // 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 5c80a4f6bb..774366449c 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -407,6 +407,29 @@ 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)[] = [] + 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 8aa825b461..8439b8c71e 100644 --- a/packages/opencode/test/memory/overlay-merge.test.ts +++ b/packages/opencode/test/memory/overlay-merge.test.ts @@ -31,9 +31,13 @@ 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 { 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, resetOverlay, 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") @@ -127,6 +131,274 @@ 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("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. + 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, 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("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) + 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.")] diff --git a/packages/opencode/test/memory/store-directory.test.ts b/packages/opencode/test/memory/store-directory.test.ts index 25b18f074f..26d19e1d71 100644 --- a/packages/opencode/test/memory/store-directory.test.ts +++ b/packages/opencode/test/memory/store-directory.test.ts @@ -1,4 +1,10 @@ -import { describe, test, expect, beforeEach, afterEach } from "bun:test" +// The workspace flag is read at module load, so it must be set before the +// 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, afterAll } from "bun:test" import fs from "fs/promises" import path from "path" import os from "os" @@ -8,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