diff --git a/packages/opencode/src/altimate/workspace/manage.ts b/packages/opencode/src/altimate/workspace/manage.ts index 1ad3792f3..32b09358b 100644 --- a/packages/opencode/src/altimate/workspace/manage.ts +++ b/packages/opencode/src/altimate/workspace/manage.ts @@ -44,11 +44,17 @@ export interface StatusReport { /** Blocks held locally for this project, and how many have not reached the * workspace. `null` when memory is off — "not synced" and "not applicable" are * different answers and a status line must not conflate them. */ - /** `unsynced: null` means the workspace's memory setting is not known from - * cache and status did not go to the network to find out. Rendering that as + /** `unsynced: null` means the workspace's memory setting is not known — from + * cache for the menu, which never asks; from the bounded poller path for the + * sidebar, when the service could not be reached. Rendering that as * 0 would tell the user their memory is current when nobody knows. */ memory: { local: number; unsynced: number | null } | null skillsEnabled: boolean + /** When workspace skills last synced successfully, or null if they have not in + * this process. Null is genuinely "unknown", not "never" — the store is + * per-process, so a fresh session has not synced yet even for a project whose + * snapshot is current on disk. Callers must not render it as "never synced". */ + skillsSyncedAt: number | null } export interface RefreshReport { @@ -92,25 +98,59 @@ export interface SyncReport { /** What the project is linked to and how far its local state has drifted. * * Cheap enough for a status line: one binding read from the local cache and, when - * memory is on, one index read. Network only when there is no cached row. */ -export async function status(directory: string): Promise { - // The cached row first, and the resolver only when there is none. A fresh - // clone, or a new machine, whose project is still bound server-side has no - // cached row, and reading only the cache answered "this project is not - // linked" with a lone Done — so that case asks. But the resolver revalidates - // a cached row too, and on the first call of a process nothing has been - // validated yet: the menu then sat on the API's full timeout when the - // service was unreachable. A cached row is taken as it is here; the poll and - // the operations behind the menu are what revalidate it. + * memory is on, one index read. Network only when there is no cached row, or + * on the poller path. */ +export async function status( + directory: string, + opts: { + /** Set for the sidebar poller. Resolves the workspace's memory setting + * through the poller path — the service is asked at most once every few + * minutes when the answer is "no" and not at all once it is "yes" — because + * on a cold cache nothing else would ever warm it, and the counts would + * simply never appear. The binding goes through the resolver too: the + * poll runs in the background, and it is what revalidates a cached row. + * + * Unset for the `/workspace` menu, which is awaited before the dialog can + * open and must NOT wait on the network: there the setting is read from + * cache alone and reported as unknown (`null`) if not held, and a cached + * row is taken as it is. `sync` does the live check. */ + poll?: boolean + /** A binding the caller has already resolved this pass. The sidebar + * resolves before it asks for status; resolving again here doubled the + * requests during an outage, when neither answer is memoized. */ + binding?: CachedBinding | null + } = {}, +): Promise { + // The cached row first, and the resolver only when there is none — for the + // menu. A fresh clone, or a new machine, whose project is still bound + // server-side has no cached row, and reading only the cache answered "this + // project is not linked" with a lone Done — so that case asks. But the + // resolver revalidates a cached row too, and on the first call of a process + // nothing has been validated yet: the menu then sat on the API's full + // timeout when the service was unreachable. const binding = - (await readLocalBinding(directory).catch(() => null)) ?? (await resolveBinding(directory).catch(() => null)) + opts.binding !== undefined + ? opts.binding + : opts.poll + ? await resolveBinding(directory).catch(() => null) + : ((await readLocalBinding(directory).catch(() => null)) ?? + (await resolveBinding(directory).catch(() => null))) return { binding, - memory: await memoryCounts(directory, binding), + memory: await memoryCounts(directory, binding, opts.poll === true), skillsEnabled: SkillSync.isEnabled(), + skillsSyncedAt: await skillsSyncedAt(directory, binding), } } +/** The age of the last clean skill sync FOR THIS BINDING, or null. */ +async function skillsSyncedAt(directory: string, binding: CachedBinding | null): Promise { + if (!binding) return null + const scope = await currentScope() + if (!scope) return null + return SkillSync.lastSuccessfulSyncAt(directory, { datamateId: binding.datamateId, ...scope }) +} + /** Pull: bring local state in line with the workspace. * * Both halves are attempted even if one fails — they are independent, and a @@ -221,18 +261,35 @@ export async function sync(directory: string): Promise { * accurate claim — nothing is pending against a workspace that accepts nothing. * Best-effort: a status line must not fail because an index read did. * - * Cache-only. `status` is awaited before the `/workspace` dialog can appear, - * so it must not sit on the network: the enablement check behind `pendingCount` - * is a GET with a 15s budget, and on a slow or dead link the menu looked like it - * did nothing. When the setting is not known from cache the count is `null` — - * unknown — and `sync` does the live check. */ + * The enablement policy differs by caller, and both are deliberate. + * + * The `/workspace` menu (`poll` false) is awaited before the dialog can appear, + * so it reads the setting from cache alone: the check behind `pendingCount` is a + * GET with a 15s budget, and on a slow or dead link the menu looked like it did + * nothing. Not known from cache is `null`, and `sync` does the live check. + * + * The sidebar poller (`poll` true) may ask, on the rate-limited poller path. + * It runs in the background, so a wait costs nothing visible — and on a cold + * cache it is the only thing that will ever warm the answer the menu then + * reads. Either way, "unknown" is reported as unknown, never as zero. */ async function memoryCounts( directory: string, binding: CachedBinding | null, + poll: boolean, ): Promise<{ local: number; unsynced: number | null } | null> { if (!MemorySync.isEnabled()) return null try { const blocks = await MemoryStore.listAll({ directory }) + if (poll && binding) { + const status = await MemorySync.memoryEnabledForPoller(binding) + // "disabled" is a real answer: memory is off, so nothing is outstanding + // and 0 is the truth. "unknown" is not — the service could not be + // reached, and reporting 0 there claims the workspace is up to date on + // the strength of a failed request. + if (status !== "enabled") return { local: blocks.length, unsynced: status === "disabled" ? 0 : null } + // Enablement is settled; `pendingCount`'s own gate must not re-ask. + return { local: blocks.length, unsynced: await MemorySync.pendingCount(blocks, binding, { trustEnabled: true }) } + } return { local: blocks.length, unsynced: await MemorySync.pendingCount(blocks, binding, { network: false }) } } catch (err) { log.warn("could not count local memory for the workspace status", { err: String(err) }) diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index 7866aec84..ca4240bbc 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -27,6 +27,7 @@ import { TRAINING_META_COMMENT } from "@/altimate/training/types" import { resolveBinding as resolveProjectBinding, type CachedBinding } from "./state" import { indexKey, readIndex, readIndexEntry, recordIndexEntry } from "./memory-index" import { WorkspaceApi } from "./api-client" +import { AltimateApi } from "@/altimate/api/client" import { LIST_LIMIT, MemoryApi, @@ -207,8 +208,17 @@ async function memoryEnabled(binding: CachedBinding): Promise { /** 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) +async function memoryStatus( + binding: CachedBinding, + opts: { + /** Skip the positive cache and ask. The cache is keyed by bare workspace + * id, which is safe for the write path (its credentials are fixed) but not + * for a caller whose own memo is tenant-scoped: on a memo miss it must not + * inherit a positive written under a previous account. */ + fresh?: boolean + } = {}, +): Promise<"enabled" | "disabled" | "error"> { + const cached = opts.fresh ? undefined : memoryEnabledCache.get(binding.datamateId) if (cached && Date.now() - cached.checkedAt < MEMORY_ENABLED_TTL_MS) return "enabled" try { const workspaces = await WorkspaceApi.listDatamates() @@ -660,6 +670,96 @@ async function runQueue( return { ok, failed, declined, skipped, deferred } } +/** How long a poller trusts either answer. Deliberately separate from + * `MEMORY_ENABLED_TTL_MS` and from the main cache: `memoryEnabled` stays + * positive-only with a 60s TTL so the WRITE path picks up a newly enabled + * workspace almost at once, which is the property that matters for not losing + * memory. A poller can afford to be a few minutes behind; what it cannot afford + * is a request every tick. + * + * Both answers are memoized, not just the "no". Reusing the write path's 60s + * positive meant that a minute after the first check an ENABLED workspace went + * back to the network on every other tick — a steady drip of `/datamates` + * requests for the life of the session. (cubic P2 on #1279.) */ +const POLL_TTL_MS = 5 * 60 * 1000 +/** How long the poller sits on an "unknown" before asking again. Not + * memoizing it at all meant every tick during an outage asked — and a queued + * re-run after a self-adoption asked twice in one tick. One poll interval: + * the tile shows no counts until the next tick either way. */ +const POLL_UNKNOWN_TTL_MS = 30 * 1000 + +/** Keyed by tenant and API URL as well as workspace id. Workspace ids are + * tenant-local, so a bare id let a same-numbered workspace in a NEWLY switched + * account inherit the previous tenant's answer and hide its unsynced count for + * the whole TTL. (cubic P2 on #1279.) */ +const pollMemo = new Map() +const pollInFlight = new Map>() + +async function pollMemoKey(binding: CachedBinding): Promise { + try { + const creds = await AltimateApi.getCredentials() + return `${creds.altimateInstanceName}|${creds.altimateUrl}|${binding.datamateId}` + } catch { + // No credentials resolved: fall back to an id-only key. The caller is about + // to fail its lookup anyway, and a wrong-tenant hit is impossible when + // there is no tenant. + return `?|?|${binding.datamateId}` + } +} + +/** Whether this workspace has memory on, for a caller that polls. + * + * Three-way on purpose. `memoryEnabled` folds "the service could not be + * reached" into `false`, which is right for the write path — it fails closed so + * an outage cannot leak a mirror — but wrong for a status line: rendering an + * unreachable service as "0 not synced" tells the user their memory is current + * when nobody knows. `memoryStatus` already draws that distinction; this used + * to throw it away and then memoize the result for five minutes. (cubic P2 on + * #1279.) + * + * An "unknown" is never memoized: the next tick should ask again rather than + * inherit a network blip. */ +export async function memoryEnabledForPoller( + binding: CachedBinding, +): Promise<"enabled" | "disabled" | "unknown"> { + // The scoped memo is the poller's only cache. `memoryEnabledCache` is keyed by + // bare workspace id — right for the write path, which is tenant-bound by its + // credentials — but consulting it here reopened a 60s cross-tenant window the + // scoped memo had closed: a positive written under the previous account was + // served to a same-numbered workspace in the next. One extra request per five + // minutes per tenant is the price, and `memoryStatus` still warms both. + const key = await pollMemoKey(binding) + const memo = pollMemo.get(key) + if (memo && Date.now() - memo.at < (memo.status === "unknown" ? POLL_UNKNOWN_TTL_MS : POLL_TTL_MS)) + return memo.status + // The in-flight ask is memoized too, not only the settled answer. Two + // refreshes overlapping on a cold memo — a remount while a slow one is + // still out — both saw it empty and both put a request on the wire. + const pending = pollInFlight.get(key) + if (pending) return pending + const ask = (async () => { + try { + const status = await memoryStatus(binding, { fresh: true }) + const answer = status === "error" ? ("unknown" as const) : status + pollMemo.set(key, { at: Date.now(), status: answer }) + return answer + } finally { + pollInFlight.delete(key) + } + })() + pollInFlight.set(key, ask) + return ask +} + +/** Test seam: the poller memo is process-global and would otherwise leak between + * cases in the same file. */ +export function resetPollMemoForTests(): void { + pollMemo.clear() + pollInFlight.clear() + memoryEnabledCache.clear() + memoryDisabledMemo.clear() +} + /** Split blocks into those the workspace still needs and those already there at * their current payload. * @@ -718,10 +818,17 @@ export async function pendingCount( * known"), never wait on the network. The default asks, and is what a * sweep wants. */ network?: boolean + /** The caller has already resolved the workspace's setting as enabled — + * the poller does, on its own rate-limited path — so the gate here must + * not ask again. Without this the poll dripped a `/datamates` request every + * 60s once the write path's positive expired, which is the exact drip the + * poller memo exists to stop. */ + trustEnabled?: boolean } = {}, ): Promise { if (blocks.length === 0) return 0 if (!isEnabled()) return 0 + if (opts.trustEnabled && binding) return partitionPending(blocks, binding, await readIndex()).pending.length if (opts.network === false && binding) { const cached = memoryEnabledCached(binding) if (cached === "unknown") return null diff --git a/packages/opencode/src/altimate/workspace/skill-sync.ts b/packages/opencode/src/altimate/workspace/skill-sync.ts index 2fc157a6e..d2ec146d6 100644 --- a/packages/opencode/src/altimate/workspace/skill-sync.ts +++ b/packages/opencode/src/altimate/workspace/skill-sync.ts @@ -68,6 +68,10 @@ const MANAGED_DIR = path.join(".altimate-code", "skill", "_workspace") * discovery scans. See the swap in `syncSkills`. */ const STAGING_DIR = path.join(".altimate-code", "skill-staging") const MANIFEST_NAME = ".manifest.json" +/** Written at the managed root after every CLEAN run, holding the epoch ms. + * The manifest cannot serve: a partial run publishes one too, and a clean run + * that finds the snapshot up to date publishes nothing. */ +const SYNCED_MARKER = ".synced-at" export interface ManifestSkill { /** Server's ``updated_at``, verbatim. The only change signal the API offers. */ @@ -251,6 +255,79 @@ export async function flushPendingSyncs(timeoutMs = 30_000): Promise { } } +/** When this project's workspace skills were last brought up to date by a + * CLEAN sync, or null when there is no snapshot, no marker, or the snapshot is + * another binding's. + * + * Exposed for the sidebar. `recentlySynced` answers a boolean against the poll + * interval, which cannot say "6 minutes ago" — and a status line whose whole job + * is to make staleness visible needs the age, not a threshold. Read from the + * marker on disk, which every thread and module realm sees alike. */ +export async function lastSuccessfulSyncAt( + directory: string, + /** The binding the age is being reported under. A marker for another + * workspace or account is not this binding's sync: after a rebind the + * sidebar can refresh before the detached sync has replaced the previous + * workspace's snapshot, and would otherwise show A's age under B's name. + * The marker carries its own identity — checking the manifest beside it was + * a second read, and another process could swap the tree between the two. */ + binding?: { datamateId: number; tenant: string; apiUrl: string }, +): Promise { + // From disk, not from the map. The map is on `globalThis`, which is shared + // across module realms but NOT across threads — and the per-message sync + // that does most of the stamping runs in the server worker, while the TUI + // and its sidebar render on the main thread. Read from the map alone, the + // "skills synced Xm ago" line never saw the syncs that actually happened. + // The marker is written on exactly the runs that stamp the map, so the two + // agree; the manifest's mtime did not — a partial run publishes one, and a + // clean up-to-date run publishes nothing. + try { + // Validated, not coerced: a truncated or hand-edited marker reads as + // unknown, never as a sync from 1970 or as another workspace's. + const raw = await fs.readFile(path.join(managedRoot(directory), SYNCED_MARKER), "utf8") + const marker = parseMarker(raw) + if (!marker) return null + if ( + binding && + (marker.datamateId !== binding.datamateId || marker.tenant !== binding.tenant || marker.apiUrl !== binding.apiUrl) + ) + return null + return marker.at + } catch (err) { + // No snapshot is no sync: after an unlink, a rebind, or an empty + // workspace, an in-memory stamp from before would report a sync that no + // longer describes what is on disk. + const code = (err as NodeJS.ErrnoException)?.code + if (code === "ENOENT" || code === "ENOTDIR") return null + return lastSyncedAt.get(path.resolve(directory)) ?? null + } +} + +/** What a clean sync leaves at the managed root: when, and for which binding. */ +interface SyncMarker { + at: number + datamateId: number + tenant: string + apiUrl: string +} + +function parseMarker(raw: string): SyncMarker | null { + try { + const m = JSON.parse(raw) as Partial | null + if (!m || typeof m !== "object") return null + if (typeof m.at !== "number" || !Number.isSafeInteger(m.at) || m.at <= 0) return null + if (typeof m.datamateId !== "number") return null + if (typeof m.tenant !== "string" || typeof m.apiUrl !== "string") return null + return { at: m.at, datamateId: m.datamateId, tenant: m.tenant, apiUrl: m.apiUrl } + } catch { + return null + } +} + +function markerFor(manifest: Pick, at: number): string { + return JSON.stringify({ at, datamateId: manifest.datamateId, tenant: manifest.tenant, apiUrl: manifest.apiUrl }) +} + /** Has this project's snapshot been checked within the poll interval? Callers * on a per-message path use this to skip the network entirely. */ export async function recentlySynced(directory: string): Promise { @@ -323,10 +400,10 @@ function safePathComponent(p: unknown): p is string { if (typeof p !== "string" || !p) return false if (p === "." || p === "..") return false if (path.isAbsolute(p)) return false - // These two are written as FILES at the staged root. An id of either name - // becomes a directory there, the write fails EISDIR, and that workspace can - // never sync again. - if (p === MANIFEST_NAME || p === ".gitignore") return false + // These are written as FILES at the managed root. An id of any of these + // names becomes a directory there, the write fails EISDIR, and that + // workspace can never sync again. + if (p === MANIFEST_NAME || p === ".gitignore" || p === SYNCED_MARKER) return false return !/[\\/\0]/.test(p) } @@ -934,6 +1011,18 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean // Written into staging so it lands atomically with the snapshot. await fs.writeFile(path.join(staging, ".gitignore"), "*\n") await fs.writeFile(path.join(staging, MANIFEST_NAME), JSON.stringify(next, null, 2)) + // The clean-sync marker lands in the same rename as the manifest, so the + // two can never describe different workspaces: written afterwards, at + // the root, there was a window in which B's manifest sat beside A's + // marker and B was reported with A's sync age. `failed` is settled by + // now — every skill has been fetched or skipped — so a partial publish + // carries no marker, and the previous one went with the retired tree. + // Best-effort: the marker is status metadata, and a failure to write it + // must not cost a complete staged snapshot its publish. + if (!failed) + await fs.writeFile(path.join(staging, SYNCED_MARKER), markerFor(next, Date.now())).catch((err) => { + log.warn("could not write the workspace skill sync marker", { err: String(err) }) + }) // Move the live tree aside rather than deleting it first. `rm` then // `rename` leaves a window with no snapshot at all — a crash or a reader // inside it sees the skills vanish. The retired tree is removed only @@ -977,7 +1066,20 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean } // Only a clean run earns the poll interval. `failed` is set by the inner // catch, which swallows so that skills can never block a turn. - if (ok && !failed && sawRemote) lastSyncedAt.set(canon, Date.now()) + if (ok && !failed && sawRemote) { + const now = Date.now() + lastSyncedAt.set(canon, now) + // A clean run that published carried its marker in the swap. This is + // the clean run that found the snapshot up to date and published + // nothing: the manifest on disk is unchanged, so stamping beside it + // cannot pair it with another workspace. Only where a snapshot exists — + // a clean run against an empty workspace removed the root. + if (!changed) { + const current = await readManifest(canon) + if (current) + await fs.writeFile(path.join(managedRoot(canon), SYNCED_MARKER), markerFor(current, now)).catch(() => {}) + } + } return { changed } })() inFlight.set(canon, settled) diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index ed3e82541..2b320aae3 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -389,18 +389,73 @@ export async function resolveBindingOutcome(directory: string): Promise void>() + +export function onBindingChanged(listener: () => void): () => void { + bindingChangeListeners.add(listener) + return () => { + bindingChangeListeners.delete(listener) + } +} + +/** Never throws: a listener is a UI refresh, and one bad subscriber must not + * fail the link or unlink that notified it. Iterates a copy so a listener that + * unsubscribes itself mid-notify cannot skip the next one. */ +function notifyBindingChanged(): void { + // Snapshot first: a listener may subscribe or unsubscribe while being + // notified, and iterating the live Set would then walk a collection that + // changed underneath us. + const listeners = Array.from(bindingChangeListeners) + for (const listener of listeners) { + try { + listener() + } catch (err) { + log.warn("a binding-change listener threw", { err: String(err) }) + } + } +} + /** Every key in the file that names this directory. Normally one — the * canonical path — but a file written before keys were canonicalised, or whose * migration could not be written back, can still hold a raw alias @@ -503,6 +558,7 @@ function forgetBinding( expect?: ExpectedRow, before?: UnscopedRow | null, ): boolean { + let dropped = false try { const cache = readCache() if (!cache) return true @@ -534,9 +590,24 @@ function forgetBinding( } for (const k of keys) delete cache.bindings[k] writeCache(cache) + dropped = true } catch (err) { log.warn("could not drop a binding the server no longer recognises", { err: String(err) }) } + // Outside the try on purpose. A listener is a UI refresh; its failure is not + // a failed cache drop, and notifying from inside would log a throwing + // subscriber as "could not drop a binding" — a misleading line about a write + // that had already succeeded. + // + // Only when something on disk changed. Notifying on a failed write too was + // tried, so the tile would not name an unlinked workspace until the next + // poll — and it made a hot loop: the sidebar answers a notification with a + // resolve, the resolve hears the memoized miss and re-enters here, the + // write fails again, and it notifies again, forty times in as many + // milliseconds for as long as the state directory stays unwritable. A + // read-only state directory now costs one poll interval of staleness + // instead, which is the right trade. (Ralph, review of #1279.) + if (dropped) notifyBindingChanged() return true } @@ -587,6 +658,7 @@ async function lookupBinding( projectPath: row.project_path ?? null, linkedAt: Date.now(), } + let adoptedNow = false try { const existing = readCache() const cache: CacheFile = @@ -602,6 +674,15 @@ async function lookupBinding( ? { ...adopted, adopted: prior.adopted, seededAt: prior.seededAt, linkedAt: prior.linkedAt } : adopted writeCache(cache) + // Anything the tile renders or the delete identifies by: a rename, or a + // remote/path the server now holds differently, is a change worth waking + // the sidebar for, even under the same id. + adoptedNow = + !prior || + prior.datamateId !== adopted.datamateId || + prior.datamateName !== adopted.datamateName || + prior.repoRemote !== adopted.repoRemote || + prior.projectPath !== adopted.projectPath } catch (err) { // The binding still stands for this call; only the cache write failed, so // the next process looks it up again. Same reasoning as recordApprovedBinding. @@ -611,6 +692,11 @@ async function lookupBinding( log.info("adopted the workspace binding this project already has on the server", { datamateId: adopted.datamateId, }) + // An adoption is a binding change this process made to its cache, and the + // sidebar is not always the caller — a `/workspace` open that adopts left + // the tile to the next poll. Stamped as validated above, so the sidebar's + // answering resolve trusts the row and does not come back here. + if (adoptedNow) notifyBindingChanged() return { status: "bound", binding: adopted } } @@ -704,6 +790,9 @@ export async function recordApprovedBinding( // synchronously on the `link` path, which awaits the seed. let bindingChanged = true let alreadySeeded = false + /** The name as it was on disk, so a rename can be detected even when the + * binding's identity is unchanged. `undefined` when there was no prior row. */ + let priorName: string | undefined try { const existing = readCache() const cache: CacheFile = @@ -711,6 +800,7 @@ export async function recordApprovedBinding( ? existing : { version: CACHE_VERSION, tenant: key.tenant, apiUrl: key.apiUrl, bindings: {} } const prior = cache.bindings[canonicalizeKey(directory)] + priorName = prior?.datamateName bindingChanged = !prior || !sameBinding(prior, binding) alreadySeeded = !bindingChanged && !!prior?.seededAt // Carry the seed marker across a warm so a completed seed is not repeated. @@ -727,6 +817,17 @@ export async function recordApprovedBinding( }) } + // Only when something a subscriber could render actually changed. A warm + // cache re-read must not wake the tile on every resolve. + // + // `bindingChanged` alone is not the right test: `sameBinding` compares + // identity (id, remote, path) because it also gates the memory seed, and + // widening it would re-seed a whole workspace every time someone renamed one. + // But the sidebar renders `datamateName`, so a rename is a visible change + // with an unchanged identity. Checked separately for that reason. (cubic P2 + // on #1279.) + if (bindingChanged || priorName !== binding.datamateName) notifyBindingChanged() + // altimate_change start - seed the workspace with the memory this machine // already holds. Deliberately OUTSIDE the try above: a failed cache write // must not skip the backfill, and a failed backfill must not read as a failed diff --git a/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx b/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx index f00d8ab06..21cfcf4a7 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace-sidebar.tsx @@ -9,7 +9,10 @@ import type { TuiPlugin, TuiPluginApi } from "@opencode-ai/plugin/tui" import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import { createSignal, onCleanup, onMount, Show } from "solid-js" -import { readLocalBinding, type CachedBinding } from "@/altimate/workspace/state" +import { onBindingChanged, resolveBindingOutcome, type CachedBinding } from "@/altimate/workspace/state" +// altimate_change start - status lines +import * as Manage from "@/altimate/workspace/manage" +// altimate_change end import { buildManageUrl, resolveWorkspaceWebUrl } from "@/altimate/workspace/browser-handoff" import { getResolvedWorkspaceId } from "@/altimate/workspace/session-context" import { AltimateApi } from "@/altimate/api/client" @@ -50,27 +53,172 @@ async function resolveManageBase(): Promise { } } +// altimate_change start - status lines +/** Coarse relative age. Deliberately not a timestamp: the point is "is this + * stale?", and a clock time makes the reader do the subtraction. */ +function describeAge(at: number): string { + // Floored from the raw elapsed time, so every label owns a full window: + // "1m ago" is 60–119s. Rounding — and rounding twice, seconds then minutes + // — had squeezed it into ~30s (89.5s → 90s → "2m ago"). + const ms = Math.max(0, Date.now() - at) + if (ms < 60_000) return "just now" + const minutes = Math.floor(ms / 60_000) + if (minutes < 60) return `${minutes}m ago` + return `${Math.floor(ms / 3_600_000)}h ago` +} +// altimate_change end + function View(props: { api: TuiPluginApi }) { const theme = () => props.api.theme.current - const [binding, setBinding] = createSignal(null) + // Three states, not two. `undefined` is "the first read has not come back + // yet"; `null` is "read, and this project is not linked". Starting at `null` + // made the pane assert "Not linked — run altimate-code link" for the first + // moments of every session, including projects that ARE linked — a false + // statement plus an instruction to run a command the user does not need. + const [binding, setBinding] = createSignal(undefined) const [manageUrl, setManageUrl] = createSignal(null) + // altimate_change start - status lines + const [detail, setDetail] = createSignal(null) + // altimate_change end let refreshInFlight = false - const refresh = async () => { - if (refreshInFlight) return + let refreshQueued = false + let disposed = false + /** `tenant|apiUrl` the current binding was resolved under. */ + let boundScope: string | null = null + const currentScope = async (): Promise => { + try { + const creds = await AltimateApi.getCredentials() + return `${creds.altimateInstanceName}|${creds.altimateUrl}` + } catch { + return null + } + } + const refresh = async (why: "poll" | "notify" = "poll") => { + // A notification that lands mid-refresh is queued, not dropped: that pass + // may already have read the old binding, and returning early would leave + // the tile stale until the next tick — the lag the listener exists to + // remove. A TICK that lands mid-refresh is dropped: it carries no news, + // and queuing it meant that while the service was unreachable — three + // calls on a 15s budget each, longer than the 30s tick — the next refresh + // started the moment the last one ended, back to back for as long as the + // outage lasted. (Ralph, review of #1279.) + if (refreshInFlight) { + if (why === "notify") refreshQueued = true + return + } refreshInFlight = true try { const dir = props.api.state.path.directory - const b = await readLocalBinding(dir).catch(() => null) - setBinding(b) - if (!b) { + // Resolve, don't just read the cache. `readLocalBinding` never touches the + // network, so on a cold cache it returns null and this tile asserted + // "Not linked — run altimate-code link" about a project that IS linked, + // until some unrelated code path happened to warm the cache. Same shape as + // the counts bug directly above. + // + // `resolveBindingOutcome` is already safe to poll: a confirmed "unbound" + // is memoized for MISS_TTL_MS and a known binding is trusted for + // REVALIDATE_MS, so the worst case is one request per five minutes. + // "unknown" (unreachable, 5xx) deliberately leaves the last answer + // standing — a network blip must not downgrade a working tile to + // "Not linked", which is the one state that tells the user to go and run + // a command. + // The account scope first, independently of the outcome. An "unknown" + // answer leaves a working tile standing over a blip — but only within + // the same account. After a credential switch the tile would otherwise + // keep showing the previous tenant's workspace, counts and manage URL + // while the process is using the next, for as long as the new lookup + // failed. A scope change clears what was rendered and leaves the tile + // undecided until the new account answers. + // `null` is "could not read the credentials this instant", not "a + // different account": a transient read failure must not blank a tile + // the resolver would have preserved. Only a scope that READS as another + // one clears. + const clearRendered = () => { + setDetail(null) setManageUrl(null) + setBinding(undefined) + boundScope = null + } + const scopeBefore = await currentScope() + if (boundScope !== null && scopeBefore !== null && scopeBefore !== boundScope) clearRendered() + const outcome = await resolveBindingOutcome(dir).catch(() => ({ status: "unknown" }) as const) + // Read again after the resolve. The credentials can change between the + // two reads, and the resolver runs under whatever they were when it + // ran; a scope that moved underneath it means this outcome cannot be + // trusted against the scope read first. What WAS rendered belonged to + // the old scope and goes now; the outcome is dropped, and the next tick + // reads a settled pair. + const scope = await currentScope() + if (scope !== scopeBefore) { + if (scope !== null) clearRendered() return } + // Every later commit in this pass checks the scope again first: the + // manage base and the status each take a credentials read of their own, + // and a switch during either would otherwise pair one account's binding + // with another's URL or counts. + const stillThisScope = async () => (await currentScope()) === scope + if (outcome.status === "bound") { + // Counts and the manage URL belong to a SPECIFIC workspace. On a rebind + // they would otherwise keep describing the old one until the new status + // resolved — the wrong numbers under the right name. Cleared only on a + // real change; an "unknown" outcome deliberately leaves everything + // standing rather than blanking a working tile over a blip. (cubic P2 + // on #1279.) + // Compared with the account scope, not the id alone. Workspace ids are + // tenant-local, so after an account switch a same-numbered workspace + // in the new tenant would otherwise be treated as unchanged and keep + // the old counts under the new name. + if (binding()?.datamateId !== outcome.binding.datamateId || scope !== boundScope) { + setDetail(null) + setManageUrl(null) + } + boundScope = scope + setBinding(outcome.binding) + } else if (outcome.status === "unbound") { + setDetail(null) + setManageUrl(null) + setBinding(null) + } + const b = binding() + // No clear here: every path that reaches this with no binding has already + // cleared the manage URL, or never set one. + if (!b) return const base = await resolveManageBase() + if (!(await stillThisScope())) { + clearRendered() + return + } setManageUrl(base ? buildManageUrl(base, b.datamateId) : null) + // altimate_change start - status lines + // `poll: true` marks this as the POLLER path: `status` then resolves the + // memory setting through a rate-limited resolver that asks at most once + // every few minutes on a "no" and never once it is "yes", instead of once + // per POLL_MS. It is a bound, not a ban — reading it as "never ask" is + // what left these counts blank until something else happened to warm the + // cache. The `/workspace` menu, by contrast, is cache-only, because it is + // awaited before the dialog can open. See `Manage.status`. + // Handed the binding this pass resolved, so `status` does not resolve it + // again — during an outage neither answer is memoized, and that was two + // requests where one was already too many. + const detail = await Manage.status(dir, { poll: true, binding: b }).catch(() => null) + if (!(await stillThisScope())) { + clearRendered() + return + } + setDetail(detail) + // altimate_change end } finally { refreshInFlight = false + // Not after disposal. A notification can land mid-refresh and unmount can + // follow before it settles, and the queued run would then do network and + // status work for a view nobody is looking at, writing to signals that no + // longer render. (cubic P3 on #1279.) + if (refreshQueued && !disposed) { + refreshQueued = false + void refresh() + } } } @@ -79,7 +227,18 @@ function View(props: { api: TuiPluginApi }) { const timer = setInterval(() => void refresh(), POLL_MS) // eslint-disable-next-line @typescript-eslint/no-explicit-any ;(timer as any)?.unref?.() - onCleanup(() => clearInterval(timer)) + // Link and unlink happen in THIS process, so the tile can hear about them + // directly instead of waiting out the poll. Without this, Unlink shows a + // success toast while the pane beside it keeps naming the workspace for up + // to POLL_MS — the UI contradicting itself, with the stale half looking + // authoritative. The interval stays: it is what catches a change made by + // another process, which no in-process listener can see. + const unsubscribe = onBindingChanged(() => void refresh("notify")) + onCleanup(() => { + disposed = true + clearInterval(timer) + unsubscribe() + }) }) return ( @@ -90,9 +249,14 @@ function View(props: { api: TuiPluginApi }) { - Not linked — run altimate-code link - + // Only once a read has actually returned `null`. While the answer is + // still unknown the tile shows its heading and nothing under it, + // which reads as "loading" rather than as a claim. + + + Not linked — run altimate-code link + + } > {(b) => ( @@ -135,6 +299,20 @@ function View(props: { api: TuiPluginApi }) { (pinned via --workspace) + {/* altimate_change start - status lines: what has drifted, so the + * reason to run `/workspace` is visible before you need it. */} + + {(m) => ( + + {m().local} {m().local === 1 ? "memory" : "memories"} + {m().unsynced !== null && m().unsynced! > 0 ? ` · ${m().unsynced} not synced` : ""} + + )} + + + {(at) => {`skills synced ${describeAge(at())}`}} + + {/* altimate_change end */} {(u) => ( openManageUrl(props.api, u())}> diff --git a/packages/opencode/src/plugin/tui/altimate/workspace.tsx b/packages/opencode/src/plugin/tui/altimate/workspace.tsx index 8c81c11b7..228a279aa 100644 --- a/packages/opencode/src/plugin/tui/altimate/workspace.tsx +++ b/packages/opencode/src/plugin/tui/altimate/workspace.tsx @@ -1733,6 +1733,16 @@ export { syncMessage as syncMessageForTests } * sweep that sent nothing because everything was refused or deferred read as a * clean all-clear. `skipped` alone (present at its current payload) is the * healthy case, and is deliberately not surfaced as a number. */ +/** Paired with `syncMessage`. A gated sweep has every count at zero, which the + * count-based rule read as a green success — with "Could not read this + * project's local memory" as the text. A failed read is a warning; the other + * gates are states, not outcomes, and are told as information. */ +function syncVariant(result: Manage.SyncReport): "info" | "success" | "warning" { + if (result.gated) return result.gatedBecause === "read-failed" ? "warning" : "info" + return result.failed > 0 || result.declined > 0 || result.deferred > 0 ? "warning" : "success" +} +export { syncVariant as syncVariantForTests } + function syncMessage(result: Manage.SyncReport): string { if (result.gated) { switch (result.gatedBecause) { @@ -1830,7 +1840,7 @@ async function runWorkspaceManage(api: TuiPluginApi, directory: string): Promise Manage.sync(directory) .then((result) => { api.ui.toast({ - variant: result.failed > 0 || result.declined > 0 || result.deferred > 0 ? "warning" : "success", + variant: syncVariant(result), message: syncMessage(result), duration: 8_000, }) diff --git a/packages/opencode/test/altimate/plugin/workspace-sync-message.test.ts b/packages/opencode/test/altimate/plugin/workspace-sync-message.test.ts index ac1edd254..edf0d8643 100644 --- a/packages/opencode/test/altimate/plugin/workspace-sync-message.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace-sync-message.test.ts @@ -6,7 +6,10 @@ // already in the workspace", and one folded deferrals into "already present" so // a sweep that sent nothing read as a clean all-clear. import { describe, expect, test } from "bun:test" -import { syncMessageForTests as message } from "../../../src/plugin/tui/altimate/workspace" +import { + syncMessageForTests as message, + syncVariantForTests as variant, +} from "../../../src/plugin/tui/altimate/workspace" const report = (over: Partial[0]> = {}) => ({ gated: false, @@ -48,6 +51,17 @@ describe("the sync toast", () => { expect(message(report({ gated: true }))).toContain("memory is off") }) + test("a gated sweep is never a green success", () => { + // Every count is zero when a sweep never ran, and the count-based rule + // rendered "Could not read this project's local memory" in the success + // colour. + expect(variant(report({ gated: true, gatedBecause: "read-failed" }))).toBe("warning") + expect(variant(report({ gated: true, gatedBecause: "memory-off" }))).toBe("info") + expect(variant(report({ gated: true, gatedBecause: "no-binding" }))).toBe("info") + expect(variant(report({ skipped: 3 }))).toBe("success") + expect(variant(report({ deferred: 1 }))).toBe("warning") + }) + test("names the actual reason a sweep never ran", () => { // Four things gate a sweep and only one is the workspace's memory toggle. // Told "memory is off" for a failed local read, the user went to a setting @@ -72,4 +86,23 @@ describe("the sync toast", () => { expect(out).toContain("3 deferred") expect(out).not.toContain("all") }) + + test("does not hide transport failures behind a refusal", () => { + // Second regression, found the same way as the first: six blocks, five + // refused and one failed, reported as "The workspace refused all 5 + // memories". The failure was dropped and "all" was false. A transport + // failure is the retryable outcome — it is the one that must survive. + const out = message(report({ sent: 0, failed: 1, declined: 5 })) + expect(out).toContain("1 failed") + expect(out).toContain("5 refused") + expect(out).not.toContain("all 5") + }) + + test("still claims 'all' only when the refusal really was all of it", () => { + expect(message(report({ declined: 4 }))).toContain("refused all 4") + }) + + test("leads with what happened, not a count of zero", () => { + expect(message(report({ sent: 0, failed: 2, declined: 1 }))).toContain("Nothing was sent") + }) }) diff --git a/packages/opencode/test/altimate/workspace/manage.test.ts b/packages/opencode/test/altimate/workspace/manage.test.ts index 425713bc0..49eb74e41 100644 --- a/packages/opencode/test/altimate/workspace/manage.test.ts +++ b/packages/opencode/test/altimate/workspace/manage.test.ts @@ -14,7 +14,7 @@ // issued — method, path, query — and the binding cache is a real file in a real // sandbox directory. import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test" -import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs" +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs" import { execFileSync } from "node:child_process" import path from "node:path" import os from "node:os" @@ -41,11 +41,17 @@ afterAll(() => { const { AltimateApi } = await import("../../../src/altimate/api/client") const { unlink, sync, status, refresh } = await import("../../../src/altimate/workspace/manage") -const { resetEnablementMemoForTests } = await import("../../../src/altimate/workspace/memory-sync") -const { readLocalBinding, recordApprovedBinding, resolveBindingOutcome, expireValidationForTests, cachePath } = - await import("../../../src/altimate/workspace/state") +const { + readLocalBinding, + recordApprovedBinding, + onBindingChanged, + resolveBindingOutcome, + expireValidationForTests, + cachePath, +} = await import("../../../src/altimate/workspace/state") const { resolveProjectIdentifier } = await import("../../../src/altimate/workspace/detect") -const { pendingCount } = await import("../../../src/altimate/workspace/memory-sync") +const { resetPollMemoForTests, resetEnablementMemoForTests, pendingCount, memoryEnabledCache, memoryEnabledForPoller } = + await import("../../../src/altimate/workspace/memory-sync") type Creds = Awaited> const originalIsConfigured = AltimateApi.isConfigured @@ -190,6 +196,181 @@ describe("status", () => { expect(report.binding).toBeNull() }) + + test("a poller resolves the workspace setting once, not on every tick", async () => { + // The sidebar calls this every 30 seconds. The shared enablement cache is + // positive-only — a workspace with memory switched OFF is never memoized — + // so asking it directly on each tick would put a request on the wire every + // 30 seconds, forever, for exactly the workspaces whose answer is "no". + // + // The fix is a bound, NOT a ban. An earlier version refused the network + // outright and the counts then never appeared at all on a session where + // nothing else warmed the cache — the very drift the line exists to surface. + await bind(projectDir) + resetPollMemoForTests() + // Counted against the workspace-list endpoint specifically, not every + // request: `recordApprovedBinding` starts a fire-and-forget backfill whose + // traffic lands at an unpredictable moment, so a total-request assertion + // passes alone and fails in a full run. + const listCalls = () => requests.filter((r) => r.url.includes("/datamates")).length + const before = listCalls() + + await status(projectDir, { poll: true }) + const afterFirst = listCalls() + await status(projectDir, { poll: true }) + await status(projectDir, { poll: true }) + + // The first poll asks. + expect(afterFirst).toBeGreaterThan(before) + // The next two do not. + expect(listCalls()).toBe(afterFirst) + }) + + test("a poller still reports the local block count when memory is off", async () => { + // "How many memories do I have" is answerable without the service; only + // "how many are outstanding" depends on the workspace setting. Reporting + // nothing at all would hide the first fact to protect the second. + await bind(projectDir) + resetPollMemoForTests() + + const report = await status(projectDir, { poll: true }) + + expect(report.memory).not.toBeNull() + // The stub workspace has memory off, so nothing is outstanding — a sweep + // would refuse to send any of it. + expect(report.memory?.unsynced).toBe(0) + expect(report.binding?.datamateName).toBe("Growth") + }) +}) + +describe("binding-change notifications", () => { + // The sidebar tile polls every 30s. Without these, Unlink shows a success + // toast while the pane beside it keeps naming the workspace until the next + // tick — the UI contradicting itself, with the stale half looking + // authoritative. Found by watching the real TUI, like the rest of this file. + test("unlink wakes subscribers so the tile does not wait out the poll", async () => { + await bind(projectDir) + let fired = 0 + const stop = onBindingChanged(() => { + fired++ + }) + try { + await unlink(projectDir) + expect(fired).toBeGreaterThan(0) + } finally { + stop() + } + }) + + test("a new bind wakes subscribers", async () => { + let fired = 0 + const stop = onBindingChanged(() => { + fired++ + }) + try { + await bind(projectDir) + expect(fired).toBeGreaterThan(0) + } finally { + stop() + } + }) + + test("re-recording the SAME binding does not", async () => { + // A warm cache re-read is not a change. Waking the tile on every resolve + // would undo the point of the poll interval. + await bind(projectDir) + let fired = 0 + const stop = onBindingChanged(() => { + fired++ + }) + try { + await bind(projectDir) + expect(fired).toBe(0) + } finally { + stop() + } + }) + + test("unsubscribing stops them", async () => { + let fired = 0 + const stop = onBindingChanged(() => { + fired++ + }) + stop() + await bind(projectDir) + expect(fired).toBe(0) + }) + + test("a listener that throws does not fail the unlink", async () => { + await bind(projectDir) + const stop = onBindingChanged(() => { + throw new Error("subscriber blew up") + }) + try { + const report = await unlink(projectDir) + expect(report.removedServerSide).toBe(true) + } finally { + stop() + } + }) +}) + +describe("what the status line is allowed to claim", () => { + test("does not report '0 not synced' when the workspace setting cannot be resolved", async () => { + // The write path folds "unreachable" into "disabled" on purpose — it fails + // closed so an outage cannot leak a mirror. A status line must not inherit + // that: rendering an unreachable service as "nothing outstanding" tells the + // user their memory is current on the strength of a failed request. + await bind(projectDir) + resetPollMemoForTests() + const failing = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + if (url.includes("/datamates")) throw new Error("network down") + return failing(input, init) + }) as typeof fetch + + const report = await status(projectDir, { poll: true }) + expect(report.memory).not.toBeNull() + // Local blocks are still countable without a service; how many are + // outstanding is genuinely unknown, and null is how that is said. + expect(report.memory?.unsynced).toBeNull() + }) + + test("still reports 0 outstanding when memory is genuinely off", async () => { + // The contrast that gives the test above its meaning: "disabled" IS an + // answer, and 0 is the truth for it. + await bind(projectDir) + resetPollMemoForTests() + const report = await status(projectDir, { poll: true }) + expect(report.memory?.unsynced).toBe(0) + }) +}) + +describe("renames", () => { + test("wake the sidebar even though the binding identity is unchanged", async () => { + // `sameBinding` compares id/remote/path because it also gates the memory + // seed — widening it would re-seed a workspace on every rename. But the + // tile renders the NAME, so a rename is a visible change that the identity + // check alone would swallow. + await bind(projectDir) + let fired = 0 + const stop = onBindingChanged(() => { + fired++ + }) + try { + await recordApprovedBinding(projectDir, { + datamateId: 42, + datamateName: "Growth Renamed", + repoRemote: "git@github.com:acme/app.git", + projectPath: projectDir, + linkedAt: Date.now(), + } as any) + expect(fired).toBeGreaterThan(0) + } finally { + stop() + } + }) }) describe("what unlink leaves on disk", () => { @@ -757,3 +938,303 @@ describe("what /workspace status may cost and claim (review round 2)", () => { expect(report.skillsLeftBehind).toBe(false) }) }) + +const manifestFor = (datamateId: number) => + JSON.stringify({ version: 1, tenant: "acme", apiUrl: "https://api.example.com", datamateId, skills: {} }) +const markerFor = (datamateId: number, at: number) => + JSON.stringify({ at, datamateId, tenant: "acme", apiUrl: "https://api.example.com" }) + +describe("the sidebar's skills-synced age", () => { + test("comes from the snapshot on disk, not a per-thread map", async () => { + // The per-message sync stamps its map in the server worker; the sidebar + // reads on the main thread, which has its own `globalThis` and so its own + // map. The marker the sync writes on every clean run is the answer both + // threads can see. + await bind(projectDir) + const managed = path.join(projectDir, ".altimate-code", "skill", "_workspace") + mkdirSync(managed, { recursive: true }) + writeFileSync(path.join(managed, ".manifest.json"), manifestFor(42)) + const before = Date.now() - 5 * 60_000 + writeFileSync(path.join(managed, ".synced-at"), markerFor(42, before)) + + const report = await status(projectDir) + + expect(report.skillsSyncedAt).toBe(before) + }) + + test("is nothing when the marker belongs to another workspace", async () => { + // After a rebind the sidebar can refresh before the detached sync has + // replaced the previous workspace's snapshot, and would otherwise render + // A's age under B's name. The marker carries its own identity, so this + // holds without a second read of the manifest beside it. + await bind(projectDir) + const managed = path.join(projectDir, ".altimate-code", "skill", "_workspace") + mkdirSync(managed, { recursive: true }) + writeFileSync(path.join(managed, ".manifest.json"), manifestFor(42)) + writeFileSync(path.join(managed, ".synced-at"), markerFor(7, Date.now() - 60_000)) + + expect((await status(projectDir)).skillsSyncedAt).toBeNull() + }) + + test("is nothing when the marker is empty or garbage", async () => { + // A truncated marker must read as unknown, not as a sync from 1970. + await bind(projectDir) + const managed = path.join(projectDir, ".altimate-code", "skill", "_workspace") + mkdirSync(managed, { recursive: true }) + writeFileSync(path.join(managed, ".manifest.json"), manifestFor(42)) + for (const junk of ["", " \n", "soon", "12abc", "1700000000000", '{"at":0,"datamateId":42}', "{not json"]) { + writeFileSync(path.join(managed, ".synced-at"), junk) + expect((await status(projectDir)).skillsSyncedAt).toBeNull() + } + }) + + test("is nothing when there is no snapshot", async () => { + // After an unlink, a rebind or an empty workspace the root is gone, and an + // in-memory stamp from before would describe a sync that no longer is. + await bind(projectDir) + const report = await status(projectDir) + expect(report.skillsSyncedAt).toBeNull() + }) +}) + +describe("the poller does not drip", () => { + test("once enablement is memoized as enabled, computing the count asks nothing", async () => { + await bind(projectDir) + resetPollMemoForTests() + // A real block, or `pendingCount` returns before its gate and the test + // cannot see whether the gate asks. + const memDir = path.join(projectDir, ".altimate-code", "memory") + mkdirSync(memDir, { recursive: true }) + writeFileSync( + path.join(memDir, "one.md"), + "---\nid: one\nscope: project\ncreated: 2026-09-01T00:00:00Z\nupdated: 2026-09-01T00:00:00Z\n---\n\nA block.\n", + ) + // Warm the poller memo: the stub answers a workspace with memory ON. + const originalFetch3 = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + const method = (init?.method ?? "GET").toUpperCase() + requests.push({ method, url }) + if (method === "GET" && url.endsWith("/datamates/")) + return new Response(JSON.stringify({ datamates: [{ id: 42, name: "Growth", memory_enabled: true }] }), { + status: 200, + headers: { "content-type": "application/json" }, + }) + return originalFetch3(input, init) + }) as typeof fetch + try { + await status(projectDir, { poll: true }) // asks once, memoizes "enabled" + // The drip only starts once the write path's 60s positive has expired + // while the poller's 5-minute memo has not. Expire it explicitly — this + // is the state every poll after the first minute is in. + memoryEnabledCache.clear() + requests = [] + await status(projectDir, { poll: true }) + await status(projectDir, { poll: true }) + expect(requests.filter((r) => r.method === "GET" && r.url.endsWith("/datamates/"))).toHaveLength(0) + } finally { + globalThis.fetch = originalFetch3 + } + }) +}) + +describe("a state directory that cannot be written", () => { + test("does not turn the notify → resolve chain into a hot loop", async () => { + // Ralph's reproduction on #1279: a row on disk, the server answering + // unbound, the state directory read-only, and a subscriber that resolves + // the way the sidebar's queued refresh does. Notifying on a failed write + // made every resolve re-enter `forgetBinding`, which notified, which the + // subscriber answered with another resolve — forty times in 24ms. + await bind(projectDir) + expireValidationForTests(projectDir) + const originalFetch3 = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + const method = (init?.method ?? "GET").toUpperCase() + requests.push({ method, url }) + if (method === "GET" && url.includes("/datamate-project-bindings/by-")) + return new Response(JSON.stringify({ detail: "gone" }), { + status: 404, + headers: { "content-type": "application/json" }, + }) + return originalFetch3(input, init) + }) as typeof fetch + let notifications = 0 + let resolves = 0 + const unsubscribe = onBindingChanged(() => { + notifications++ + if (notifications < 50) void resolveBindingOutcome(projectDir).then(() => resolves++) + }) + const stateDir = path.dirname(cachePath()) + chmodSync(stateDir, 0o555) + try { + const outcome = await resolveBindingOutcome(projectDir) + expect(outcome.status).toBe("unbound") + await new Promise((r) => setTimeout(r, 100)) + } finally { + chmodSync(stateDir, 0o755) + unsubscribe() + globalThis.fetch = originalFetch3 + } + // Nothing on disk changed, so nobody was told; the row stays until the + // next poll, which is one interval of staleness rather than a loop. + expect(notifications).toBe(0) + expect(resolves).toBe(0) + }) +}) + +describe("status reuses a binding the caller resolved", () => { + test("makes no binding request of its own when handed one", async () => { + await bind(projectDir) + expireValidationForTests(projectDir) + const b = (await readLocalBinding(projectDir))! + requests = [] + const report = await status(projectDir, { poll: true, binding: b }) + expect(report.binding?.datamateId).toBe(42) + expect(requests.filter((r) => r.url.includes("/datamate-project-bindings/"))).toHaveLength(0) + }) +}) + +describe("the poller during an outage", () => { + test("sits on an unknown for a tick rather than asking again at once", async () => { + // Not memoizing "unknown" at all meant every tick during an outage asked, + // and a queued re-run after a self-adoption asked twice in one tick. + await bind(projectDir) + resetPollMemoForTests() + const originalFetch3 = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + const method = (init?.method ?? "GET").toUpperCase() + requests.push({ method, url }) + if (method === "GET" && url.endsWith("/datamates/")) + return new Response(JSON.stringify({ detail: "down" }), { + status: 503, + headers: { "content-type": "application/json" }, + }) + return originalFetch3(input, init) + }) as typeof fetch + try { + requests = [] + const b = (await readLocalBinding(projectDir))! + expect(await memoryEnabledForPoller(b)).toBe("unknown") + expect(await memoryEnabledForPoller(b)).toBe("unknown") + expect(requests.filter((r) => r.method === "GET" && r.url.endsWith("/datamates/"))).toHaveLength(1) + } finally { + globalThis.fetch = originalFetch3 + } + }) +}) + +describe("a rename on the server", () => { + test("reaches the resolver's answer and wakes the sidebar", async () => { + // The same workspace, renamed in the SaaS. `lookupBinding` wrote the new + // name to the cache but the resolver handed back the row it had read + // before, and nothing woke the tile — so the old name stood. + await bind(projectDir) + expireValidationForTests(projectDir) + const originalFetch3 = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + const method = (init?.method ?? "GET").toUpperCase() + requests.push({ method, url }) + if (method === "GET" && url.includes("/datamate-project-bindings/by-")) + return new Response( + JSON.stringify({ + // Only the name differs; the identifiers match the recorded row, so + // the notification can be attributed to the rename alone. + binding: { + id: 1, + datamate_id: 42, + datamate_name: "Growth (renamed)", + repo_remote: "git@github.com:acme/app.git", + project_path: projectDir, + }, + datamate: { id: 42, name: "Growth (renamed)" }, + }), + { status: 200, headers: { "content-type": "application/json" } }, + ) + return originalFetch3(input, init) + }) as typeof fetch + let notified = 0 + const unsubscribe = onBindingChanged(() => notified++) + try { + const outcome = await resolveBindingOutcome(projectDir) + expect(outcome.status).toBe("bound") + if (outcome.status === "bound") expect(outcome.binding.datamateName).toBe("Growth (renamed)") + expect(notified).toBe(1) + } finally { + unsubscribe() + globalThis.fetch = originalFetch3 + } + }) +}) + +describe("the poller coalesces overlapping misses", () => { + test("two refreshes on a cold memo put one request on the wire", async () => { + // A remount while a slow refresh is still out started a second one; both + // saw the memo empty and both asked. The in-flight ask is memoized too. + await bind(projectDir) + resetPollMemoForTests() + const originalFetch3 = globalThis.fetch + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + const method = (init?.method ?? "GET").toUpperCase() + requests.push({ method, url }) + if (method === "GET" && url.endsWith("/datamates/")) { + await new Promise((r) => setTimeout(r, 20)) + return new Response(JSON.stringify({ datamates: [{ id: 42, name: "Growth", memory_enabled: true }] }), { + status: 200, + headers: { "content-type": "application/json" }, + }) + } + return originalFetch3(input, init) + }) as typeof fetch + try { + requests = [] + const b = (await readLocalBinding(projectDir))! + const [a, c] = await Promise.all([memoryEnabledForPoller(b), memoryEnabledForPoller(b)]) + expect(a).toBe("enabled") + expect(c).toBe("enabled") + expect(requests.filter((r) => r.method === "GET" && r.url.endsWith("/datamates/"))).toHaveLength(1) + } finally { + globalThis.fetch = originalFetch3 + } + }) +}) + +describe("the poller after an account switch", () => { + test("does not serve the previous tenant's positive to a same-numbered workspace", async () => { + // Workspace ids are tenant-local. The write path's positive cache is keyed + // by bare id — fine there, its credentials are fixed — but the poller + // consulting it first reopened a 60s window in which tenant A's "enabled" + // was served to tenant B's workspace 42. + await bind(projectDir) + resetPollMemoForTests() + const binding = (await readLocalBinding(projectDir))! + const originalFetch4 = globalThis.fetch + let tenantMemory = true + globalThis.fetch = (async (input: any, init?: any) => { + const url = typeof input === "string" ? input : input.url + const method = (init?.method ?? "GET").toUpperCase() + requests.push({ method, url }) + if (method === "GET" && url.endsWith("/datamates/")) + return new Response(JSON.stringify({ datamates: [{ id: 42, name: "W", memory_enabled: tenantMemory }] }), { + status: 200, + headers: { "content-type": "application/json" }, + }) + return originalFetch4(input, init) + }) as typeof fetch + try { + expect(await memoryEnabledForPoller(binding)).toBe("enabled") // tenant A, warms the bare-id positive + // Switch accounts: same workspace id, memory OFF there. + ;(AltimateApi as unknown as { getCredentials: () => Promise }).getCredentials = async () => + ({ altimateInstanceName: "other", altimateUrl: "https://api.example.com", altimateApiKey: "k" }) as Creds + tenantMemory = false + expect(await memoryEnabledForPoller(binding)).toBe("disabled") + } finally { + globalThis.fetch = originalFetch4 + ;(AltimateApi as unknown as { getCredentials: () => Promise }).getCredentials = async () => + ({ altimateInstanceName: "acme", altimateUrl: "https://api.example.com", altimateApiKey: "key-a" }) as Creds + } + }) +}) diff --git a/packages/opencode/test/altimate/workspace/skill-sync.test.ts b/packages/opencode/test/altimate/workspace/skill-sync.test.ts index 9fa866967..2f7560df5 100644 --- a/packages/opencode/test/altimate/workspace/skill-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/skill-sync.test.ts @@ -49,6 +49,7 @@ writeFileSync( const { syncSkills, recentlySynced, + lastSuccessfulSyncAt, registryStale, markRegistryApplied, flushPendingSyncs, @@ -560,6 +561,52 @@ describe("workspace skill sync", () => { expect(existsSync(skillFile("pub-1", "references/g.md"))).toBe(true) }) + test("a partial sync is not reported as the last successful one", async () => { + // A run that could not fetch one skill still publishes a snapshot of the + // rest — so the manifest's mtime moved on a run the sync itself marked + // failed, and the sidebar showed a fresh "synced" for a snapshot with a + // hole in it. + serve({ "pub-1": { "SKILL.md": "one" }, "pub-2": { "SKILL.md": "two" } }) + const inner = globalThis.fetch + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + if (String(input).includes("/pub-2/files/")) throw new Error("offline") + return inner(input as never, init as never) + }) as unknown as typeof fetch + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + expect(await lastSuccessfulSyncAt(project)).toBeNull() + + // The next clean run is one. + serve({ "pub-1": { "SKILL.md": "one" }, "pub-2": { "SKILL.md": "two" } }, "2026-01-02T00:00:00Z") + await syncSkills(project) + expect(await lastSuccessfulSyncAt(project)).not.toBeNull() + }) + + test("a clean run that finds the snapshot up to date still advances the age", async () => { + // Publishing nothing is still a successful sync. Without a stamp here + // the age grew stale for as long as the workspace did not change. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + const first = await lastSuccessfulSyncAt(project) + expect(first).not.toBeNull() + await new Promise((r) => setTimeout(r, 5)) + await syncSkills(project) + expect((await lastSuccessfulSyncAt(project)) as number).toBeGreaterThan(first as number) + }) + + test("a removed snapshot has no last sync, whatever the process remembers", async () => { + // The in-memory stamp survives the purge; the answer must not. After an + // unlink or a rebind the root is gone, and "synced 2m ago" would describe + // a snapshot that no longer exists. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(await lastSuccessfulSyncAt(project)).not.toBeNull() + + rmSync(path.join(project, MANAGED), { recursive: true, force: true }) + + expect(await lastSuccessfulSyncAt(project)).toBeNull() + }) + test("a failed sync does not consume the poll window", async () => { globalThis.fetch = (async () => { throw new Error("offline")