From 4b2d693325d1339f93a3cecee56a3c0fece5ee48 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 21:19:57 +0530 Subject: [PATCH 1/5] fix(workspace): tell the model which memory store is the team's, and flush mirrors before `run` exits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two stores answer "remember this for the team" in a linked project: the Altimate Memory blocks (`altimate_memory_write`, mirrored to the workspace and loaded into every linked checkout) and the engine's memory hub (`datamate_add_memories`), a separate SaaS store nothing else reads. The engine's always-on workflow text tells the model to use the hub, and the block tool never said it was the shared one — so on a plain prompt the decision went to the hub and the rest of the team never saw it (#1332). - The identity section, when the project is bound and the workspace has not said memory is off, adds a "Team memory" line: save decisions and conventions with `altimate_memory_write`, they sync to the workspace and every linked checkout; the `datamate_*` memory tools are the engine's separate store; read before writing to update an existing block rather than add a duplicate. The pure formatter takes it as an option; the section derives it from the enablement memo. Cap raised to 2,000 so the longest shape still fits its name. - `altimate_memory_write`'s description says the same, so the model that reads tool descriptions before the prompt learns it there too. - `MemoryStore.write` mirrors fire-and-forget, which a one-shot `run` abandons at exit. `memory-sync` now tracks mirrors in flight and exposes `flushPendingMirrors`; `run` awaits it beside `flushPendingSyncs` before exiting. Not in this PR: hiding the engine hub when workspace memory is on, and `training_save` as a third write path — both are design decisions beyond a prompt fix. Tests: the line renders for a bound workspace with memory on and not for one with memory off (through `systemSection` and the pure formatter); the flush holds exit for an in-flight mirror and gives up at its bound; the `run` wiring is pinned. Tracking was deleted once to confirm the flush tests fail. Closes #1332 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../src/altimate/workspace/identity.ts | 48 +++++++++++++++---- .../src/altimate/workspace/memory-sync.ts | 41 +++++++++++++++- packages/opencode/src/cli/cmd/run.ts | 5 ++ .../opencode/src/memory/tools/memory-write.ts | 2 +- .../workspace/identity-section.test.ts | 26 ++++++++++ .../test/altimate/workspace/identity.test.ts | 24 ++++++++-- .../altimate/workspace/memory-sync.test.ts | 45 +++++++++++++++++ .../opencode/test/cli/run-accounting.test.ts | 7 +++ 8 files changed, 182 insertions(+), 16 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/identity.ts b/packages/opencode/src/altimate/workspace/identity.ts index 4bc016ad6c..9aec485349 100644 --- a/packages/opencode/src/altimate/workspace/identity.ts +++ b/packages/opencode/src/altimate/workspace/identity.ts @@ -29,15 +29,16 @@ import { workspaceLabel } from "./workspace-name" import { isEnabled } from "./engine-seams" import { Instance } from "../../project/instance" import { AltimateApi } from "../api/client" +import { memoryEnabledCached } from "./memory-sync" /** Independent of `awareness.ts`'s MAX_SECTION_CHARS (2,000) — this section is a short, * fixed-shape identity statement, not an open-ended list of served integrations, so a * much smaller ceiling is enough. The label is budgeted separately (`MAX_LABEL_CHARS` * in `workspace-name.ts`) so the cap here is defense in depth and never cuts the - * instruction itself: the longest fixed shape (pinned and stale) is ~1,080 characters - * before the label, and a label at its budget still leaves room. A test renders every - * shape with a budget-sized label and checks the name survives. */ -export const MAX_SECTION_CHARS = 1_500 + * instruction itself: the longest fixed shape (pinned, stale, with the team-memory line) + * is ~1,550 characters before the label, and a label at its budget still leaves room. A + * test renders every shape with a budget-sized label and checks the name survives. */ +export const MAX_SECTION_CHARS = 2_000 const HEADING = "## Altimate Workspace" @@ -65,20 +66,37 @@ const LINK_HINT = * leaving it to the caller) so the cap is part of the pure, testable surface — the * guard is against a pathological workspace name, and every branch below is built from * one, so it belongs where the name is rendered. */ -export function render(outcome: BindingOutcome, cap = MAX_SECTION_CHARS): string { - const body = renderBody(outcome) +export function render(outcome: BindingOutcome, cap = MAX_SECTION_CHARS, opts: RenderOptions = {}): string { + const body = renderBody(outcome, opts) if (body.length <= cap) return body // Fail closed rather than truncate: a cut instruction is worse than a missing // name. The name is the only variable field, so drop it and keep the id; if // even that does not fit, say nothing rather than something partial. if (outcome.status === "bound") { - const unnamed = renderBody({ ...outcome, binding: { ...outcome.binding, datamateName: "" } }) + const unnamed = renderBody({ ...outcome, binding: { ...outcome.binding, datamateName: "" } }, opts) if (unnamed.length <= cap) return unnamed } return "" } -function renderBody(outcome: BindingOutcome): string { +export type RenderOptions = { + /** The bound workspace has memory on (or has not said otherwise): tell the model + * which store is the team's. `systemSection` derives it from the enablement memo; + * the pure formatter takes it as an argument so tests stay deterministic. */ + teamMemory?: boolean +} + +/** Two stores answer "remember this". Only one is read by other linked checkouts, + * and nothing told the model which — so on a plain "save this for the team" it + * reached for the engine's hub and the decision never left the session (#1332). */ +const TEAM_MEMORY_LINE = + "Team memory: save decisions and conventions with `altimate_memory_write` (scope " + + '"project" for this project, "global" for everything); they sync to the workspace and to every ' + + "linked checkout. The `datamate_*` memory tools (`datamate_add_memories`, `datamate_search_memory`) " + + "are the engine's separate store and are not what teammates' sessions read. Before saving, check " + + "`altimate_memory_read` for an existing block on the same subject and update it rather than add a duplicate." + +function renderBody(outcome: BindingOutcome, opts: RenderOptions = {}): string { if (outcome.status === "bound") { const id = String(outcome.binding.datamateId) const name = workspaceLabel(outcome.binding.datamateName, undefined) @@ -105,6 +123,7 @@ function renderBody(outcome: BindingOutcome): string { ? " Skills and memory follow this workspace; warehouse tool routing still follows the " + "project's own link, which may name a different workspace." : ""), + ...(opts.teamMemory ? [TEAM_MEMORY_LINE] : []), `When ${TRIGGER}, the answer is this Altimate Workspace — never substitute ` + "another service's own \"workspace\" (a Databricks workspace, an IDE's " + "workspace folder, etc.) for it, and the reverse: a question about another " + @@ -287,6 +306,14 @@ async function accountScope(): Promise { return { tenant: c.altimateInstanceName, apiUrl: c.altimateUrl, account } } +/** The team-memory line is shown for a bound workspace unless it is known to have + * memory switched off; "unknown" errs toward telling the model where team memory + * goes, since the write path itself checks enablement before uploading. */ +function renderOptions(outcome: BindingOutcome): RenderOptions { + if (outcome.status !== "bound") return {} + return { teamMemory: memoryEnabledCached(outcome.binding) !== "disabled" } +} + function keyFor(scope: AccountScope, directory: string): string { return `${scope.tenant}|${scope.apiUrl}|${scope.account}|${directory}` } @@ -357,14 +384,15 @@ export async function systemSection(): Promise { if (!scope) return render({ status: "unknown" }) const key = keyFor(scope, directory) const hit = memo.get(key) - if (fresh(hit)) return render(hit!.outcome) + if (fresh(hit)) return render(hit!.outcome, MAX_SECTION_CHARS, renderOptions(hit!.outcome)) // The fallback is itself raced against a small budget, so the wait is // bounded by RESOLVE_DEADLINE_MS + FALLBACK_BUDGET_MS, not by the disk. const deadline = after(RESOLVE_DEADLINE_MS, () => Promise.race([lastKnown(key, directory), after(FALLBACK_BUDGET_MS, () => ({ status: "unknown" }))]), ) try { - return render(await Promise.race([resolve(key, directory), deadline])) + const outcome = await Promise.race([resolve(key, directory), deadline]) + return render(outcome, MAX_SECTION_CHARS, renderOptions(outcome)) } finally { for (const t of timers) clearTimeout(t) } diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index ca4240bbc3..011c199502 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -185,6 +185,11 @@ export function memoryEnabledCached(binding: CachedBinding): "enabled" | "disabl /** Test seam: both memos are process-global, and an earlier case's answer * would otherwise leak into a later one. */ +/** Test seam: record the workspace's "memory off" answer without a request. */ +export function noteMemoryDisabledForTests(datamateId: number): void { + memoryDisabledMemo.set(datamateId, Date.now()) +} + export function resetEnablementMemoForTests(): void { memoryEnabledCache.clear() memoryDisabledMemo.clear() @@ -530,6 +535,34 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr return next } +/** Mirrors still in flight. `MemoryStore.write` fires the mirror and forgets + * it (the local file is already durable, and a cloud failure must not fail + * the write), which is right for the TUI and wrong for a one-shot `run`: the + * process exits the moment the turn ends, routinely before the upload lands, + * and the block a teammate was meant to see never leaves the machine (#1332). + * Tracked here so `flushPendingMirrors` can hold the exit for them, the way + * `skill-sync.flushPendingSyncs` holds it for a cold skill sync. */ +const mirrorsInFlight = new Set>() + +/** Await every mirror still in flight, bounded, so a short-lived process does + * not exit with an upload half-done. Failures are already logged by the + * caller; this only waits. */ +export async function flushPendingMirrors(timeoutMs = 30_000): Promise { + const pending = [...mirrorsInFlight] + if (pending.length === 0) return + let timer: ReturnType | undefined + try { + await Promise.race([ + Promise.allSettled(pending), + new Promise((resolve) => { + timer = setTimeout(resolve, timeoutMs) + }), + ]) + } finally { + if (timer) clearTimeout(timer) + } +} + /** 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. */ @@ -538,7 +571,7 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi // Queued BEFORE the binding lookup, not after. Both are async, so resolving // them first let two operations on one block reach `serialize` in the // opposite order to the writes that triggered them. - await serialize(block.scope, block.id, async () => { + const task = serialize(block.scope, block.id, async () => { // A binding is required for EVERY scope, not just project. Memories are // associated with a workspace, and the workspace is what carries the // memory_enabled setting — mirroring from an unbound directory would upload @@ -550,6 +583,12 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi if (!(await memoryEnabled(binding))) return await push(block, binding, undefined, directory) }) + mirrorsInFlight.add(task) + try { + await task + } finally { + mirrorsInFlight.delete(task) + } } /** Archive a block's cloud record rather than deleting it, so the workspace diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 3ea9ddb91b..64516adb85 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -1448,6 +1448,11 @@ You are speaking to a non-technical business executive. Follow these rules stric await import("../../altimate/workspace/skill-sync") .then((m) => m.flushPendingSyncs()) .catch(() => {}) + // And the memory mirrors: a block saved on the last turn was uploaded + // fire-and-forget and lost the same race (#1332). + await import("../../altimate/workspace/memory-sync") + .then((m) => m.flushPendingMirrors()) + .catch(() => {}) } // altimate_change end diff --git a/packages/opencode/src/memory/tools/memory-write.ts b/packages/opencode/src/memory/tools/memory-write.ts index 3969f1cf45..6d35518ff8 100644 --- a/packages/opencode/src/memory/tools/memory-write.ts +++ b/packages/opencode/src/memory/tools/memory-write.ts @@ -6,7 +6,7 @@ import { MEMORY_MAX_BLOCK_SIZE, MEMORY_MAX_BLOCKS_PER_SCOPE, CitationSchema, Mem const idSchema = MemoryBlockSchema.shape.id export const MemoryWriteTool = Tool.define("altimate_memory_write", { - description: `Save an Altimate Memory block for cross-session persistence. Use this to store information worth remembering across sessions — warehouse configurations, naming conventions, team preferences, data model notes, or past analysis decisions. Each block is a Markdown file persisted to disk. Max ${MEMORY_MAX_BLOCK_SIZE} chars per block, ${MEMORY_MAX_BLOCKS_PER_SCOPE} blocks per scope. Supports hierarchical IDs with slashes (e.g., 'warehouse/snowflake-config'), optional TTL expiration, and citation-backed memories.`, + description: `Save an Altimate Memory block for cross-session persistence. Use this to store information worth remembering across sessions — warehouse configurations, naming conventions, team preferences, data model notes, or past analysis decisions. When the project is linked to an Altimate Workspace this is the TEAM's memory: blocks sync to the workspace and to every linked checkout, so a decision saved here is what teammates' sessions read. The datamate_* memory tools (datamate_add_memories, datamate_search_memory) are the engine's separate store and are not what linked checkouts read. Each block is a Markdown file persisted to disk. Max ${MEMORY_MAX_BLOCK_SIZE} chars per block, ${MEMORY_MAX_BLOCKS_PER_SCOPE} blocks per scope. Supports hierarchical IDs with slashes (e.g., 'warehouse/snowflake-config'), optional TTL expiration, and citation-backed memories.`, parameters: z.object({ id: idSchema .describe( diff --git a/packages/opencode/test/altimate/workspace/identity-section.test.ts b/packages/opencode/test/altimate/workspace/identity-section.test.ts index 31e770be75..71f6cd6c51 100644 --- a/packages/opencode/test/altimate/workspace/identity-section.test.ts +++ b/packages/opencode/test/altimate/workspace/identity-section.test.ts @@ -104,6 +104,32 @@ describe("systemSection", () => { expect(out).toContain("never substitute") }) + test("a bound project whose workspace has memory on gets the team-memory line; one with memory off does not", async () => { + const { memoryEnabledCache, resetEnablementMemoForTests, noteMemoryDisabledForTests } = await import( + "../../../src/altimate/workspace/memory-sync" + ) + await recordApprovedBinding(projectDir, { + datamateId: 77, + datamateName: "Team", + repoRemote: null, + projectPath: projectDir, + linkedAt: Date.now(), + }) + resetEnablementMemoForTests() + memoryEnabledCache.set(77, { checkedAt: Date.now() }) + try { + expect(await inProject(systemSection)).toContain("Team memory:") + resetOutcomeMemoForTests() + resetEnablementMemoForTests() + // A remembered "no" from the workspace switches the line off. + noteMemoryDisabledForTests(77) + expect(await inProject(systemSection)).not.toContain("Team memory:") + expect(await inProject(systemSection)).toContain('is "Team"') + } finally { + resetEnablementMemoForTests() + } + }) + test("renders nothing when the workspace pilot is off", async () => { // A user outside the pilot has no Altimate Workspace to be linked to, and // must not be told every turn that none is linked and how to link one. diff --git a/packages/opencode/test/altimate/workspace/identity.test.ts b/packages/opencode/test/altimate/workspace/identity.test.ts index b981fc0829..4d73db0453 100644 --- a/packages/opencode/test/altimate/workspace/identity.test.ts +++ b/packages/opencode/test/altimate/workspace/identity.test.ts @@ -25,6 +25,20 @@ describe("bound — a specific Altimate Workspace is linked", () => { } const boundOut = render(boundOutcome) + test("with team memory on, the section names the team's store and the engine's hub as separate (#1332)", () => { + const out = render(boundOutcome, undefined, { teamMemory: true }) + expect(out).toContain("Team memory: save decisions and conventions with `altimate_memory_write`") + expect(out).toContain("sync to the workspace and to every linked checkout") + expect(out).toContain("`datamate_add_memories`") + expect(out).toContain("are the engine's separate store") + expect(out).toContain("check `altimate_memory_read` for an existing block") + expect(out.split("\n")).toHaveLength(5) + // Off (workspace memory disabled) and by default (pure formatter): no line. + expect(render(boundOutcome, undefined, { teamMemory: false })).not.toContain("Team memory") + expect(boundOut).not.toContain("Team memory") + expect(render({ status: "unbound" }, undefined, { teamMemory: true })).not.toContain("Team memory") + }) + test("names the workspace and forbids substituting another service's 'workspace' for an identity question", () => { expect(boundOut).toContain("## Altimate Workspace") expect(boundOut).toContain('"Foo Corp Data Team"') @@ -267,10 +281,12 @@ describe("the section cap fails closed", () => { { status: "bound", binding: b(true), stale: true }, ] for (const shape of shapes) { - const out = render(shape) - expect(out.length).toBeLessThanOrEqual(MAX_SECTION_CHARS) - expect(out).toContain('\\"\\"\\"') // the name is there, not "(unnamed)" - expect(out).not.toContain("(unnamed)") + for (const teamMemory of [false, true]) { + const out = render(shape, MAX_SECTION_CHARS, { teamMemory }) + expect(out.length).toBeLessThanOrEqual(MAX_SECTION_CHARS) + expect(out).toContain('\\"\\"\\"') // the name is there, not "(unnamed)" + expect(out).not.toContain("(unnamed)") + } } }) diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 84ba0844a0..2bb1f14d4e 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -44,6 +44,7 @@ const { isEnabled, memoryEnabledCached, mirrorBlock, + flushPendingMirrors, overlayBlocks, resetOverlay, syncInternals, @@ -340,6 +341,50 @@ describe("buildMetadata", () => { // ── write path ────────────────────────────────────────────────────────────── describe("mirrorBlock", () => { + test("flushPendingMirrors waits for a mirror a short-lived process would abandon (#1332)", async () => { + // `MemoryStore.write` fires the mirror and forgets it; a one-shot `run` exits + // when the turn ends, routinely before the upload lands. The flush holds the + // exit for it, the way `skill-sync.flushPendingSyncs` holds it for a skill sync. + let release!: () => void + const gate = new Promise((r) => (release = r)) + const original = globalThis.fetch + let requests = 0 + globalThis.fetch = (async (input: any, init?: any) => { + requests++ + await gate // the server is slow: nothing completes until we say so + return original(input, init) + }) as unknown as typeof fetch + createResult = [{ id: "mem-slow" }] + let settled = false + void mirrorBlock(block({ id: "slow" })).then(() => (settled = true)) + await Bun.sleep(20) + expect(settled).toBe(false) + expect(requests).toBeGreaterThan(0) // it is genuinely on the wire + const flush = flushPendingMirrors() + let flushed = false + void flush.then(() => (flushed = true)) + await Bun.sleep(20) + expect(flushed).toBe(false) // the flush is holding for the mirror + release() + await flush + expect(settled).toBe(true) + // Nothing in flight: an immediate return. + const started = Date.now() + await flushPendingMirrors() + expect(Date.now() - started).toBeLessThan(50) + }) + + test("flushPendingMirrors gives up after its bound rather than hanging exit forever", async () => { + globalThis.fetch = (() => new Promise(() => {})) as unknown as typeof fetch + void mirrorBlock(block({ id: "hung" })) + await Bun.sleep(20) + const started = Date.now() + await flushPendingMirrors(100) + const waited = Date.now() - started + expect(waited).toBeGreaterThanOrEqual(90) + expect(waited).toBeLessThan(1000) + }) + test("a create is repaired with a verbatim update", async () => { // A create runs an extractor that rewrites the text; update() is verbatim, // so every create is followed by one. diff --git a/packages/opencode/test/cli/run-accounting.test.ts b/packages/opencode/test/cli/run-accounting.test.ts index eee8b8888b..09cc9e9b60 100644 --- a/packages/opencode/test/cli/run-accounting.test.ts +++ b/packages/opencode/test/cli/run-accounting.test.ts @@ -509,6 +509,13 @@ describe("run command request/stream lifecycle contracts", () => { expect(source).toContain('"IdleDoneContinuationUnconfirmed"') }) + test("the exit path flushes pending skill syncs AND memory mirrors when the workspace pilot is on (#1332)", async () => { + const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url).pathname).text() + expect(source).toMatch( + /if \(CoreFlag\.ALTIMATE_WORKSPACE\) \{[\s\S]{0,400}?skill-sync"\)[\s\S]{0,200}?flushPendingSyncs\(\)[\s\S]{0,600}?memory-sync"\)[\s\S]{0,200}?flushPendingMirrors\(\)/, + ) + }) + test("an SSE-triggered request abort preserves the original stream failure", async () => { const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url).pathname).text() expect(source).toMatch( From 841a59e74e7e31b1835868144d16ea5d6cf0dca4 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 22:16:57 +0530 Subject: [PATCH 2/5] fix(workspace): team-memory line only on confirmed enablement; track archives in the exit flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review on #1344. - The identity section's team-memory line renders only when the workspace's memory is cached as enabled — not merely "not disabled". The write path uploads only on a confirmed yes, so an unconfirmed or failed check must not promise a sync that would stay local. The memo is filled by the first enablement check of the session, so the line appears from the next turn - `archiveBlock` is tracked in `mirrorsInFlight` like a mirror, so a one-shot `run` that deletes a block does not exit before the workspace record is archived; `flushPendingMirrors` covers both - `altimate_memory_write` description: the team-store claim is qualified with "with workspace memory enabled", and the read-before-write instruction from the prompt line is repeated here - Tests: the "unknown" enablement state hides the line; an archive holds the flush; the hung-mirror test is gated and released so `mirrorsInFlight` is clean afterwards; the `run.ts` source guard matches the exact awaited `import(...).then(flush)` shape so a dropped `await` fails it Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../src/altimate/workspace/identity.ts | 11 +-- .../src/altimate/workspace/memory-sync.ts | 45 +++++++----- .../opencode/src/memory/tools/memory-write.ts | 2 +- .../workspace/identity-section.test.ts | 8 ++- .../altimate/workspace/memory-sync.test.ts | 69 ++++++++++++++++--- .../opencode/test/cli/run-accounting.test.ts | 4 +- 6 files changed, 106 insertions(+), 33 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/identity.ts b/packages/opencode/src/altimate/workspace/identity.ts index 9aec485349..9e2f05833a 100644 --- a/packages/opencode/src/altimate/workspace/identity.ts +++ b/packages/opencode/src/altimate/workspace/identity.ts @@ -306,12 +306,15 @@ async function accountScope(): Promise { return { tenant: c.altimateInstanceName, apiUrl: c.altimateUrl, account } } -/** The team-memory line is shown for a bound workspace unless it is known to have - * memory switched off; "unknown" errs toward telling the model where team memory - * goes, since the write path itself checks enablement before uploading. */ +/** The team-memory line is shown only for a bound workspace known to have memory + * enabled. "Unknown" is not enough: the write path uploads only on a confirmed + * "enabled", so advertising sync on an unconfirmed or failed check would tell the + * model teammates will read a block that stays on this machine. The memo is + * populated by the first enablement check of the session (the backfill sweep on + * bind, or the first mirror), so the line appears from the next turn on. */ function renderOptions(outcome: BindingOutcome): RenderOptions { if (outcome.status !== "bound") return {} - return { teamMemory: memoryEnabledCached(outcome.binding) !== "disabled" } + return { teamMemory: memoryEnabledCached(outcome.binding) === "enabled" } } function keyFor(scope: AccountScope, directory: string): string { diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 011c199502..3ed82e9bf0 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -544,9 +544,19 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr * `skill-sync.flushPendingSyncs` holds it for a cold skill sync. */ const mirrorsInFlight = new Set>() -/** Await every mirror still in flight, bounded, so a short-lived process does - * not exit with an upload half-done. Failures are already logged by the - * caller; this only waits. */ +/** Hold a task in `mirrorsInFlight` for its lifetime. */ +async function tracked(task: Promise): Promise { + mirrorsInFlight.add(task) + try { + await task + } finally { + mirrorsInFlight.delete(task) + } +} + +/** Await every mirror and archive still in flight, bounded, so a short-lived + * process does not exit with an upload or an archive half-done. Failures are + * already logged by the caller; this only waits. */ export async function flushPendingMirrors(timeoutMs = 30_000): Promise { const pending = [...mirrorsInFlight] if (pending.length === 0) return @@ -583,12 +593,7 @@ export async function mirrorBlock(block: MemoryBlock, directory?: string): Promi if (!(await memoryEnabled(binding))) return await push(block, binding, undefined, directory) }) - mirrorsInFlight.add(task) - try { - await task - } finally { - mirrorsInFlight.delete(task) - } + return tracked(task) } /** Archive a block's cloud record rather than deleting it, so the workspace @@ -602,15 +607,19 @@ export async function archiveBlock( 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 () => { - // 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) - }) + // inside the queued op for the same reason as in `mirrorBlock`. Tracked like a + // mirror: a one-shot `run` that deletes a block must not exit before the + // workspace record is archived, or teammates keep a memory the author removed. + return tracked( + serialize(scope, blockId, async () => { + // 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) + }), + ) } async function archiveNow( diff --git a/packages/opencode/src/memory/tools/memory-write.ts b/packages/opencode/src/memory/tools/memory-write.ts index 6d35518ff8..e1c6550d8d 100644 --- a/packages/opencode/src/memory/tools/memory-write.ts +++ b/packages/opencode/src/memory/tools/memory-write.ts @@ -6,7 +6,7 @@ import { MEMORY_MAX_BLOCK_SIZE, MEMORY_MAX_BLOCKS_PER_SCOPE, CitationSchema, Mem const idSchema = MemoryBlockSchema.shape.id export const MemoryWriteTool = Tool.define("altimate_memory_write", { - description: `Save an Altimate Memory block for cross-session persistence. Use this to store information worth remembering across sessions — warehouse configurations, naming conventions, team preferences, data model notes, or past analysis decisions. When the project is linked to an Altimate Workspace this is the TEAM's memory: blocks sync to the workspace and to every linked checkout, so a decision saved here is what teammates' sessions read. The datamate_* memory tools (datamate_add_memories, datamate_search_memory) are the engine's separate store and are not what linked checkouts read. Each block is a Markdown file persisted to disk. Max ${MEMORY_MAX_BLOCK_SIZE} chars per block, ${MEMORY_MAX_BLOCKS_PER_SCOPE} blocks per scope. Supports hierarchical IDs with slashes (e.g., 'warehouse/snowflake-config'), optional TTL expiration, and citation-backed memories.`, + description: `Save an Altimate Memory block for cross-session persistence. Use this to store information worth remembering across sessions — warehouse configurations, naming conventions, team preferences, data model notes, or past analysis decisions. When the project is linked to an Altimate Workspace with workspace memory enabled this is the TEAM's memory: blocks sync to the workspace and to every linked checkout, so a decision saved here is what teammates' sessions read. Before saving, call \`altimate_memory_read\` for an existing block on the same subject and update it rather than add a duplicate. The datamate_* memory tools (datamate_add_memories, datamate_search_memory) are the engine's separate store and are not what linked checkouts read. Each block is a Markdown file persisted to disk. Max ${MEMORY_MAX_BLOCK_SIZE} chars per block, ${MEMORY_MAX_BLOCKS_PER_SCOPE} blocks per scope. Supports hierarchical IDs with slashes (e.g., 'warehouse/snowflake-config'), optional TTL expiration, and citation-backed memories.`, parameters: z.object({ id: idSchema .describe( diff --git a/packages/opencode/test/altimate/workspace/identity-section.test.ts b/packages/opencode/test/altimate/workspace/identity-section.test.ts index 71f6cd6c51..4b374de57a 100644 --- a/packages/opencode/test/altimate/workspace/identity-section.test.ts +++ b/packages/opencode/test/altimate/workspace/identity-section.test.ts @@ -104,7 +104,7 @@ describe("systemSection", () => { expect(out).toContain("never substitute") }) - test("a bound project whose workspace has memory on gets the team-memory line; one with memory off does not", async () => { + test("a bound project gets the team-memory line only once the workspace's memory is confirmed on", async () => { const { memoryEnabledCache, resetEnablementMemoForTests, noteMemoryDisabledForTests } = await import( "../../../src/altimate/workspace/memory-sync" ) @@ -125,6 +125,12 @@ describe("systemSection", () => { noteMemoryDisabledForTests(77) expect(await inProject(systemSection)).not.toContain("Team memory:") expect(await inProject(systemSection)).toContain('is "Team"') + // Not yet checked is not "on": the write path uploads only on a confirmed + // yes, so the prompt must not promise a sync that would stay local. (bot review) + resetOutcomeMemoForTests() + resetEnablementMemoForTests() + expect(await inProject(systemSection)).not.toContain("Team memory:") + expect(await inProject(systemSection)).toContain('is "Team"') } finally { resetEnablementMemoForTests() } diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 2bb1f14d4e..55e2dc0708 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -375,14 +375,67 @@ describe("mirrorBlock", () => { }) test("flushPendingMirrors gives up after its bound rather than hanging exit forever", async () => { - globalThis.fetch = (() => new Promise(() => {})) as unknown as typeof fetch - void mirrorBlock(block({ id: "hung" })) - await Bun.sleep(20) - const started = Date.now() - await flushPendingMirrors(100) - const waited = Date.now() - started - expect(waited).toBeGreaterThanOrEqual(90) - expect(waited).toBeLessThan(1000) + // Gated, not hung forever: `mirrorsInFlight` is module-level, and a mirror that + // never settles would make every later default-bound flush in this process wait + // the full 30s. (bot review) + let release!: () => void + const gate = new Promise((r) => (release = r)) + const original = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + await gate + return original(input, init) + }) as unknown as typeof fetch + createResult = [{ id: "mem-hung" }] + const mirror = mirrorBlock(block({ id: "hung" })) + try { + await Bun.sleep(20) + const started = Date.now() + await flushPendingMirrors(100) + const waited = Date.now() - started + expect(waited).toBeGreaterThanOrEqual(90) + expect(waited).toBeLessThan(1000) + } finally { + release() + await mirror + globalThis.fetch = original + } + }) + + test("flushPendingMirrors holds the exit for an archive too, not only for a mirror", async () => { + // A one-shot `run` that deletes a block fires `archiveBlock` and forgets it; + // without tracking, the process could exit with the workspace record still + // live and teammates keeping a memory the author removed. (bot review) + const b = block({ id: "to-archive-late" }) + createResult = [{ id: "mem-archive-late" }] + await mirrorBlock(b) + listResponse = [ + { id: "mem-archive-late", memory: b.content, metadata: { source: MIRROR_SOURCE, block_id: "to-archive-late", block_scope: "global" } }, + ] + let release!: () => void + const gate = new Promise((r) => (release = r)) + const original = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + await gate + return original(input, init) + }) as unknown as typeof fetch + let settled = false + const archive = archiveBlock("global", "to-archive-late").then(() => (settled = true)) + try { + await Bun.sleep(20) + expect(settled).toBe(false) + const flush = flushPendingMirrors() + let flushed = false + void flush.then(() => (flushed = true)) + await Bun.sleep(20) + expect(flushed).toBe(false) // the flush is holding for the archive + release() + await flush + expect(settled).toBe(true) + } finally { + release() + await archive + globalThis.fetch = original + } }) test("a create is repaired with a verbatim update", async () => { diff --git a/packages/opencode/test/cli/run-accounting.test.ts b/packages/opencode/test/cli/run-accounting.test.ts index 09cc9e9b60..f2ce54e0d2 100644 --- a/packages/opencode/test/cli/run-accounting.test.ts +++ b/packages/opencode/test/cli/run-accounting.test.ts @@ -512,7 +512,9 @@ describe("run command request/stream lifecycle contracts", () => { test("the exit path flushes pending skill syncs AND memory mirrors when the workspace pilot is on (#1332)", async () => { const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url).pathname).text() expect(source).toMatch( - /if \(CoreFlag\.ALTIMATE_WORKSPACE\) \{[\s\S]{0,400}?skill-sync"\)[\s\S]{0,200}?flushPendingSyncs\(\)[\s\S]{0,600}?memory-sync"\)[\s\S]{0,200}?flushPendingMirrors\(\)/, + // Each flush must be AWAITED: a `void import(...)` keeps the same tokens and + // brings back the lost-upload race. (bot review) + /if \(CoreFlag\.ALTIMATE_WORKSPACE\) \{[\s\S]{0,400}?await import\("\.\.\/\.\.\/altimate\/workspace\/skill-sync"\)\s*\.then\(\(m\) => m\.flushPendingSyncs\(\)\)[\s\S]{0,600}?await import\("\.\.\/\.\.\/altimate\/workspace\/memory-sync"\)\s*\.then\(\(m\) => m\.flushPendingMirrors\(\)\)/, ) }) From 8da3abef177c65fa2153f7ec93b6c2d6388fef3a Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 22:57:36 +0530 Subject: [PATCH 3/5] fix(workspace): mirror state on globalThis, bounded flush on signals, no sync promise for a stale link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of #1344 (gpt-5.6-sol). - `mirrorsInFlight` and the per-block queues were plain module state, while this module is reached through two specifiers (`@/…` from `MemoryStore`, relative from the `run` exit path) — the situation skill-sync already anchors its tables on `globalThis` for. Same anchor here, so the set the flush reads is the set the writer filled whatever the runtime does with module records; test mirrors through one specifier and flushes through the other - SIGINT/SIGTERM exited at once, killing an upload of a block saved that turn. The signal handlers now hold for pending mirrors up to 2s (a second signal is not delayed) before exiting with the same code - The team-memory line is not rendered on a stale outcome: "last known … may since have changed" followed by "syncs to the workspace" contradicted itself - Test: the production enablement check (the same `memoryStatus` the write path and backfill run) fills the memo the line reads — skipping the cache write fails it (checked by mutation); a confirmed-on memo on a stale link shows no line Not changed: `serve`, the TUI and ACP exits still do not drain mirrors (pre-existing, separate entry points); the first turn of a cold one-shot `run` can miss the line while the enablement request is in flight — the `altimate_memory_write` description carries the same guidance for that turn. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../src/altimate/workspace/identity.ts | 3 ++ .../src/altimate/workspace/memory-sync.ts | 13 +++++- packages/opencode/src/cli/cmd/run.ts | 19 +++++++- .../workspace/identity-section.test.ts | 45 ++++++++++++++++--- .../altimate/workspace/memory-sync.test.ts | 32 +++++++++++++ 5 files changed, 103 insertions(+), 9 deletions(-) diff --git a/packages/opencode/src/altimate/workspace/identity.ts b/packages/opencode/src/altimate/workspace/identity.ts index 9e2f05833a..5075bd2f2e 100644 --- a/packages/opencode/src/altimate/workspace/identity.ts +++ b/packages/opencode/src/altimate/workspace/identity.ts @@ -314,6 +314,9 @@ async function accountScope(): Promise { * bind, or the first mirror), so the line appears from the next turn on. */ function renderOptions(outcome: BindingOutcome): RenderOptions { if (outcome.status !== "bound") return {} + // A stale outcome is "last known … may since have changed": promising that a + // save syncs to that workspace would contradict the line above it. + if (outcome.stale) return {} return { teamMemory: memoryEnabledCached(outcome.binding) === "enabled" } } diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 3ed82e9bf0..8e7b9c0b3c 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -520,7 +520,16 @@ async function push( * memory the user deleted. Two rapid saves race the same way and create * duplicates. Keyed by scope+id, so unrelated blocks still mirror in parallel. */ -const blockQueues = new Map>() +// Anchored on a process-global, as skill-sync's tables are: this module is reached +// through two module graphs in one process — `MemoryStore` via `@/…`, the `run` +// exit path via a relative specifier — and a runtime that keeps a record per +// specifier would fork plain module state. A flush that saw an empty set while +// the writer's copy held the upload would defeat the #1332 fix silently. +const SYNC_STATE = Symbol.for("altimate.memory-sync.state") +const syncState: { blockQueues: Map>; mirrorsInFlight: Set> } = (( + globalThis as unknown as Record +)[SYNC_STATE] ??= { blockQueues: new Map(), mirrorsInFlight: new Set() }) +const blockQueues = syncState.blockQueues function serialize(scope: "global" | "project", blockId: string, op: () => Promise): Promise { const key = `${scope}:${blockId}` @@ -542,7 +551,7 @@ function serialize(scope: "global" | "project", blockId: string, op: () => Pr * and the block a teammate was meant to see never leaves the machine (#1332). * Tracked here so `flushPendingMirrors` can hold the exit for them, the way * `skill-sync.flushPendingSyncs` holds it for a cold skill sync. */ -const mirrorsInFlight = new Set>() +const mirrorsInFlight = syncState.mirrorsInFlight /** Hold a task in `mirrorsInFlight` for its lifetime. */ async function tracked(task: Promise): Promise { diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 64516adb85..ce3ff4ad36 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -1075,14 +1075,29 @@ You are speaking to a non-technical business executive. Follow these rules stric // altimate_change end // Register crash handlers to flush the trace on unexpected exit + // altimate_change start — and hold a signal exit, briefly, for a memory + // mirror still on the wire: Ctrl-C while the last response streams used to + // kill the upload of a block saved that turn (#1332). Bounded well below + // the normal-exit flush, and a second signal is not delayed by the first. + let signalled = false + const exitAfterMirrors = (code: number) => { + if (signalled) return process.exit(code) + signalled = true + if (!CoreFlag.ALTIMATE_WORKSPACE) return process.exit(code) + void import("../../altimate/workspace/memory-sync") + .then((m) => m.flushPendingMirrors(2_000)) + .catch(() => {}) + .finally(() => process.exit(code)) + } const onSigint = () => { tracer?.flushSync("Process interrupted") - process.exit(130) + exitAfterMirrors(130) } const onSigterm = () => { tracer?.flushSync("Process interrupted") - process.exit(143) + exitAfterMirrors(143) } + // altimate_change end // altimate_change start — honest rc on fatal abort. beforeExit firing // before the run finishes means the event loop drained before the run // completed — the prompt/event stream was abandoned (observed: a diff --git a/packages/opencode/test/altimate/workspace/identity-section.test.ts b/packages/opencode/test/altimate/workspace/identity-section.test.ts index 4b374de57a..e26b0d4846 100644 --- a/packages/opencode/test/altimate/workspace/identity-section.test.ts +++ b/packages/opencode/test/altimate/workspace/identity-section.test.ts @@ -136,6 +136,32 @@ describe("systemSection", () => { } }) + test("the production enablement check fills the memo the line reads (codex on #1344)", async () => { + // Not a hand-written cache entry: the sidebar poller's check (`memoryStatus`, the + // same one the write path and the backfill sweep run) asks GET /datamates/ and + // caches a yes; the section reads that. Deleting the cache write would hide the + // line for good, and the previous test would not notice. + const { memoryEnabledForPoller, resetEnablementMemoForTests } = await import("../../../src/altimate/workspace/memory-sync") + globalThis.fetch = (async (input: any) => + new Response( + JSON.stringify( + String(input).includes("/datamates/") ? { datamates: [{ id: 91, name: "Team", memory_enabled: true }] } : {}, + ), + { status: 200, headers: { "content-type": "application/json" } }, + )) as unknown as typeof fetch + const binding = { datamateId: 91, datamateName: "Team", repoRemote: null, projectPath: projectDir, linkedAt: Date.now() } + await recordApprovedBinding(projectDir, binding) + resetEnablementMemoForTests() + try { + expect(await inProject(systemSection)).not.toContain("Team memory:") + expect(await memoryEnabledForPoller(binding as never)).toBe("enabled") + resetOutcomeMemoForTests() + expect(await inProject(systemSection)).toContain("Team memory:") + } finally { + resetEnablementMemoForTests() + } + }) + test("renders nothing when the workspace pilot is off", async () => { // A user outside the pilot has no Altimate Workspace to be linked to, and // must not be told every turn that none is linked and how to link one. @@ -294,11 +320,20 @@ describe("systemSection", () => { globalThis.fetch = (async () => { throw new Error("offline") }) as unknown as typeof fetch - const out = await inProject(systemSection) - expect(out).toContain("was last known to be linked to Altimate Workspace id 9") - expect(out).toContain('is "Finance"') - expect(out).toContain("could not be re-verified just now") - expect(out).not.toContain("This project is linked to Altimate Workspace id 9") + // Memory was confirmed on earlier; that does not make a sync promise true of a + // binding the server can no longer vouch for. (codex on #1344) + const { memoryEnabledCache, resetEnablementMemoForTests } = await import("../../../src/altimate/workspace/memory-sync") + memoryEnabledCache.set(9, { checkedAt: Date.now() }) + try { + const out = await inProject(systemSection) + expect(out).toContain("was last known to be linked to Altimate Workspace id 9") + expect(out).toContain('is "Finance"') + expect(out).toContain("could not be re-verified just now") + expect(out).not.toContain("This project is linked to Altimate Workspace id 9") + expect(out).not.toContain("Team memory:") + } finally { + resetEnablementMemoForTests() + } }) test("concurrent steps share one resolve (single-flight)", async () => { diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 55e2dc0708..1d28983222 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -374,6 +374,38 @@ describe("mirrorBlock", () => { expect(Date.now() - started).toBeLessThan(50) }) + test("a mirror tracked through one module specifier is flushed through another (codex on #1344)", async () => { + // `MemoryStore` reaches this module via `@/…`, the `run` exit path via a relative + // path. Whether or not the runtime keeps one record per specifier, the set the + // flush reads must be the set the writer filled — hence the `globalThis` anchor. + const viaAlias = await import("@/altimate/workspace/memory-sync") + let release!: () => void + const gate = new Promise((r) => (release = r)) + const original = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + await gate + return original(input, init) + }) as unknown as typeof fetch + createResult = [{ id: "mem-alias" }] + let settled = false + const mirror = viaAlias.mirrorBlock(block({ id: "via-alias" })).then(() => (settled = true)) + try { + await Bun.sleep(20) + const flush = flushPendingMirrors() // the relative-import copy + let flushed = false + void flush.then(() => (flushed = true)) + await Bun.sleep(20) + expect(flushed).toBe(false) // it is holding for the alias copy's mirror + release() + await flush + expect(settled).toBe(true) + } finally { + release() + await mirror + globalThis.fetch = original + } + }) + test("flushPendingMirrors gives up after its bound rather than hanging exit forever", async () => { // Gated, not hung forever: `mirrorsInFlight` is module-level, and a mirror that // never settles would make every later default-bound flush in this process wait From 55c9fcb91d0acfde344685c66741750242f13881 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 23:24:33 +0530 Subject: [PATCH 4/5] fix(workspace): flush syncs and mirrors together, abort before the signal flush, steadier tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review of 8da3abe on #1344. - The two exit flushes ran sequentially under independent 30s bounds — two stalled backends cost a minute. They run together now - On the first signal the event stream is aborted before the mirror snapshot is taken, so the run cannot enqueue a mirror behind the flush - The `altimate_memory_write` description names the workspace-sync gate too - Tests: the gated cases wait on a "request arrived" signal instead of a 20ms sleep, and every one releases its mirror in `finally`; the two-specifier test — which proves nothing under Bun's single record — asserts the `globalThis` anchor directly; the production-path enablement test waits for the bind's backfill and resets the poller memo it fills Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- packages/opencode/src/cli/cmd/run.ts | 20 ++- .../opencode/src/memory/tools/memory-write.ts | 2 +- .../workspace/identity-section.test.ts | 10 +- .../altimate/workspace/memory-sync.test.ts | 140 +++++++++--------- .../opencode/test/cli/run-accounting.test.ts | 2 +- 5 files changed, 93 insertions(+), 81 deletions(-) diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index ce3ff4ad36..3651824c3a 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -1084,6 +1084,9 @@ You are speaking to a non-technical business executive. Follow these rules stric if (signalled) return process.exit(code) signalled = true if (!CoreFlag.ALTIMATE_WORKSPACE) return process.exit(code) + // Stop the run first so no new mirror is enqueued behind the snapshot the + // flush takes; what is already on the wire is what gets the 2s. + eventAbort.abort() void import("../../altimate/workspace/memory-sync") .then((m) => m.flushPendingMirrors(2_000)) .catch(() => {}) @@ -1460,14 +1463,17 @@ You are speaking to a non-technical business executive. Follow these rules stric // received its skills at all. Imported lazily and only when the feature is // on, so an opted-out run does not load the module. if (CoreFlag.ALTIMATE_WORKSPACE) { - await import("../../altimate/workspace/skill-sync") - .then((m) => m.flushPendingSyncs()) - .catch(() => {}) // And the memory mirrors: a block saved on the last turn was uploaded - // fire-and-forget and lost the same race (#1332). - await import("../../altimate/workspace/memory-sync") - .then((m) => m.flushPendingMirrors()) - .catch(() => {}) + // fire-and-forget and lost the same race (#1332). Both flushes run together + // under their own bounds, so two stalled backends cost one wait, not two. + await Promise.all([ + import("../../altimate/workspace/skill-sync") + .then((m) => m.flushPendingSyncs()) + .catch(() => {}), + import("../../altimate/workspace/memory-sync") + .then((m) => m.flushPendingMirrors()) + .catch(() => {}), + ]) } // altimate_change end diff --git a/packages/opencode/src/memory/tools/memory-write.ts b/packages/opencode/src/memory/tools/memory-write.ts index e1c6550d8d..4613d9756d 100644 --- a/packages/opencode/src/memory/tools/memory-write.ts +++ b/packages/opencode/src/memory/tools/memory-write.ts @@ -6,7 +6,7 @@ import { MEMORY_MAX_BLOCK_SIZE, MEMORY_MAX_BLOCKS_PER_SCOPE, CitationSchema, Mem const idSchema = MemoryBlockSchema.shape.id export const MemoryWriteTool = Tool.define("altimate_memory_write", { - description: `Save an Altimate Memory block for cross-session persistence. Use this to store information worth remembering across sessions — warehouse configurations, naming conventions, team preferences, data model notes, or past analysis decisions. When the project is linked to an Altimate Workspace with workspace memory enabled this is the TEAM's memory: blocks sync to the workspace and to every linked checkout, so a decision saved here is what teammates' sessions read. Before saving, call \`altimate_memory_read\` for an existing block on the same subject and update it rather than add a duplicate. The datamate_* memory tools (datamate_add_memories, datamate_search_memory) are the engine's separate store and are not what linked checkouts read. Each block is a Markdown file persisted to disk. Max ${MEMORY_MAX_BLOCK_SIZE} chars per block, ${MEMORY_MAX_BLOCKS_PER_SCOPE} blocks per scope. Supports hierarchical IDs with slashes (e.g., 'warehouse/snowflake-config'), optional TTL expiration, and citation-backed memories.`, + description: `Save an Altimate Memory block for cross-session persistence. Use this to store information worth remembering across sessions — warehouse configurations, naming conventions, team preferences, data model notes, or past analysis decisions. When the project is linked to an Altimate Workspace with workspace sync on and workspace memory enabled this is the TEAM's memory: blocks sync to the workspace and to every linked checkout, so a decision saved here is what teammates' sessions read. Before saving, call \`altimate_memory_read\` for an existing block on the same subject and update it rather than add a duplicate. The datamate_* memory tools (datamate_add_memories, datamate_search_memory) are the engine's separate store and are not what linked checkouts read. Each block is a Markdown file persisted to disk. Max ${MEMORY_MAX_BLOCK_SIZE} chars per block, ${MEMORY_MAX_BLOCKS_PER_SCOPE} blocks per scope. Supports hierarchical IDs with slashes (e.g., 'warehouse/snowflake-config'), optional TTL expiration, and citation-backed memories.`, parameters: z.object({ id: idSchema .describe( diff --git a/packages/opencode/test/altimate/workspace/identity-section.test.ts b/packages/opencode/test/altimate/workspace/identity-section.test.ts index e26b0d4846..4e77e3d300 100644 --- a/packages/opencode/test/altimate/workspace/identity-section.test.ts +++ b/packages/opencode/test/altimate/workspace/identity-section.test.ts @@ -141,7 +141,9 @@ describe("systemSection", () => { // same one the write path and the backfill sweep run) asks GET /datamates/ and // caches a yes; the section reads that. Deleting the cache write would hide the // line for good, and the previous test would not notice. - const { memoryEnabledForPoller, resetEnablementMemoForTests } = await import("../../../src/altimate/workspace/memory-sync") + const { memoryEnabledForPoller, resetEnablementMemoForTests, resetPollMemoForTests } = await import( + "../../../src/altimate/workspace/memory-sync" + ) globalThis.fetch = (async (input: any) => new Response( JSON.stringify( @@ -150,8 +152,11 @@ describe("systemSection", () => { { status: 200, headers: { "content-type": "application/json" } }, )) as unknown as typeof fetch const binding = { datamateId: 91, datamateName: "Team", repoRemote: null, projectPath: projectDir, linkedAt: Date.now() } - await recordApprovedBinding(projectDir, binding) + // The bind's own backfill sweep runs the same check; wait for it so it cannot + // refill the memo between the reset and the first assertion. (bot review) + await recordApprovedBinding(projectDir, binding, { awaitBackfill: true }) resetEnablementMemoForTests() + resetPollMemoForTests() try { expect(await inProject(systemSection)).not.toContain("Team memory:") expect(await memoryEnabledForPoller(binding as never)).toBe("enabled") @@ -159,6 +164,7 @@ describe("systemSection", () => { expect(await inProject(systemSection)).toContain("Team memory:") } finally { resetEnablementMemoForTests() + resetPollMemoForTests() } }) diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 1d28983222..f401895aa0 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -339,97 +339,103 @@ describe("buildMetadata", () => { }) }) +/** A fetch that parks every request behind a gate and reports when the first one + * has arrived — the readiness signal the gated tests wait on instead of a sleep. */ +function gatedFetch() { + let release!: () => void + const gate = new Promise((r) => (release = r)) + let entered!: () => void + const firstRequest = new Promise((r) => (entered = r)) + const original = globalThis.fetch + let requests = 0 + globalThis.fetch = (async (input: any, init?: any) => { + requests++ + entered() + await gate + return original(input, init) + }) as unknown as typeof fetch + return { + release, + firstRequest, + requests: () => requests, + restore: () => { + globalThis.fetch = original + }, + } +} + // ── write path ────────────────────────────────────────────────────────────── describe("mirrorBlock", () => { test("flushPendingMirrors waits for a mirror a short-lived process would abandon (#1332)", async () => { // `MemoryStore.write` fires the mirror and forgets it; a one-shot `run` exits // when the turn ends, routinely before the upload lands. The flush holds the // exit for it, the way `skill-sync.flushPendingSyncs` holds it for a skill sync. - let release!: () => void - const gate = new Promise((r) => (release = r)) - const original = globalThis.fetch - let requests = 0 - globalThis.fetch = (async (input: any, init?: any) => { - requests++ - await gate // the server is slow: nothing completes until we say so - return original(input, init) - }) as unknown as typeof fetch + const net = gatedFetch() createResult = [{ id: "mem-slow" }] let settled = false - void mirrorBlock(block({ id: "slow" })).then(() => (settled = true)) - await Bun.sleep(20) - expect(settled).toBe(false) - expect(requests).toBeGreaterThan(0) // it is genuinely on the wire - const flush = flushPendingMirrors() - let flushed = false - void flush.then(() => (flushed = true)) - await Bun.sleep(20) - expect(flushed).toBe(false) // the flush is holding for the mirror - release() - await flush - expect(settled).toBe(true) - // Nothing in flight: an immediate return. - const started = Date.now() - await flushPendingMirrors() - expect(Date.now() - started).toBeLessThan(50) - }) - - test("a mirror tracked through one module specifier is flushed through another (codex on #1344)", async () => { - // `MemoryStore` reaches this module via `@/…`, the `run` exit path via a relative - // path. Whether or not the runtime keeps one record per specifier, the set the - // flush reads must be the set the writer filled — hence the `globalThis` anchor. - const viaAlias = await import("@/altimate/workspace/memory-sync") - let release!: () => void - const gate = new Promise((r) => (release = r)) - const original = globalThis.fetch - globalThis.fetch = (async (input: any, init?: any) => { - await gate - return original(input, init) - }) as unknown as typeof fetch - createResult = [{ id: "mem-alias" }] - let settled = false - const mirror = viaAlias.mirrorBlock(block({ id: "via-alias" })).then(() => (settled = true)) + const mirror = mirrorBlock(block({ id: "slow" })).then(() => (settled = true)) try { - await Bun.sleep(20) - const flush = flushPendingMirrors() // the relative-import copy + await net.firstRequest // it is genuinely on the wire + expect(settled).toBe(false) + const flush = flushPendingMirrors() let flushed = false void flush.then(() => (flushed = true)) await Bun.sleep(20) - expect(flushed).toBe(false) // it is holding for the alias copy's mirror - release() + expect(flushed).toBe(false) // the flush is holding for the mirror + net.release() await flush expect(settled).toBe(true) + // Nothing in flight: an immediate return. + const started = Date.now() + await flushPendingMirrors() + expect(Date.now() - started).toBeLessThan(50) } finally { - release() + net.release() await mirror - globalThis.fetch = original + net.restore() + } + }) + + test("the in-flight set lives on globalThis, so every copy of this module shares it (codex on #1344)", async () => { + // `MemoryStore` reaches this module via `@/…`, the `run` exit path via a relative + // path. Bun hands both the same record today, so a two-specifier test proves + // nothing; what is asserted is the anchor itself: a tracked mirror is visible on + // the process-global state, which is what a second module record would read. + const net = gatedFetch() + createResult = [{ id: "mem-anchor" }] + const mirror = mirrorBlock(block({ id: "anchored" })) + try { + await net.firstRequest + const state = (globalThis as any)[Symbol.for("altimate.memory-sync.state")] + expect(state?.mirrorsInFlight?.size).toBe(1) + net.release() + await mirror + expect(state.mirrorsInFlight.size).toBe(0) + } finally { + net.release() + await mirror + net.restore() } }) test("flushPendingMirrors gives up after its bound rather than hanging exit forever", async () => { - // Gated, not hung forever: `mirrorsInFlight` is module-level, and a mirror that + // Gated, not hung forever: `mirrorsInFlight` is process-global, and a mirror that // never settles would make every later default-bound flush in this process wait // the full 30s. (bot review) - let release!: () => void - const gate = new Promise((r) => (release = r)) - const original = globalThis.fetch - globalThis.fetch = (async (input: any, init?: any) => { - await gate - return original(input, init) - }) as unknown as typeof fetch + const net = gatedFetch() createResult = [{ id: "mem-hung" }] const mirror = mirrorBlock(block({ id: "hung" })) try { - await Bun.sleep(20) + await net.firstRequest const started = Date.now() await flushPendingMirrors(100) const waited = Date.now() - started expect(waited).toBeGreaterThanOrEqual(90) expect(waited).toBeLessThan(1000) } finally { - release() + net.release() await mirror - globalThis.fetch = original + net.restore() } }) @@ -443,30 +449,24 @@ describe("mirrorBlock", () => { listResponse = [ { id: "mem-archive-late", memory: b.content, metadata: { source: MIRROR_SOURCE, block_id: "to-archive-late", block_scope: "global" } }, ] - let release!: () => void - const gate = new Promise((r) => (release = r)) - const original = globalThis.fetch - globalThis.fetch = (async (input: any, init?: any) => { - await gate - return original(input, init) - }) as unknown as typeof fetch + const net = gatedFetch() let settled = false const archive = archiveBlock("global", "to-archive-late").then(() => (settled = true)) try { - await Bun.sleep(20) + await net.firstRequest expect(settled).toBe(false) const flush = flushPendingMirrors() let flushed = false void flush.then(() => (flushed = true)) await Bun.sleep(20) expect(flushed).toBe(false) // the flush is holding for the archive - release() + net.release() await flush expect(settled).toBe(true) } finally { - release() + net.release() await archive - globalThis.fetch = original + net.restore() } }) diff --git a/packages/opencode/test/cli/run-accounting.test.ts b/packages/opencode/test/cli/run-accounting.test.ts index f2ce54e0d2..10893a0e99 100644 --- a/packages/opencode/test/cli/run-accounting.test.ts +++ b/packages/opencode/test/cli/run-accounting.test.ts @@ -514,7 +514,7 @@ describe("run command request/stream lifecycle contracts", () => { expect(source).toMatch( // Each flush must be AWAITED: a `void import(...)` keeps the same tokens and // brings back the lost-upload race. (bot review) - /if \(CoreFlag\.ALTIMATE_WORKSPACE\) \{[\s\S]{0,400}?await import\("\.\.\/\.\.\/altimate\/workspace\/skill-sync"\)\s*\.then\(\(m\) => m\.flushPendingSyncs\(\)\)[\s\S]{0,600}?await import\("\.\.\/\.\.\/altimate\/workspace\/memory-sync"\)\s*\.then\(\(m\) => m\.flushPendingMirrors\(\)\)/, + /if \(CoreFlag\.ALTIMATE_WORKSPACE\) \{[\s\S]{0,400}?await Promise\.all\(\[\s*import\("\.\.\/\.\.\/altimate\/workspace\/skill-sync"\)\s*\.then\(\(m\) => m\.flushPendingSyncs\(\)\)[\s\S]{0,200}?import\("\.\.\/\.\.\/altimate\/workspace\/memory-sync"\)\s*\.then\(\(m\) => m\.flushPendingMirrors\(\)\)[\s\S]{0,100}?\]\)/, ) }) From 91013cb6a822907b74d060bc89d9dcd396c68704 Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 22 Sep 2026 03:14:17 +0530 Subject: [PATCH 5/5] test(workspace): drop the unused request counter from gatedFetch (bot review) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- packages/opencode/test/altimate/workspace/memory-sync.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index f401895aa0..a81510ae17 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -347,9 +347,7 @@ function gatedFetch() { let entered!: () => void const firstRequest = new Promise((r) => (entered = r)) const original = globalThis.fetch - let requests = 0 globalThis.fetch = (async (input: any, init?: any) => { - requests++ entered() await gate return original(input, init) @@ -357,7 +355,6 @@ function gatedFetch() { return { release, firstRequest, - requests: () => requests, restore: () => { globalThis.fetch = original },