diff --git a/packages/opencode/src/altimate/workspace/api-client.ts b/packages/opencode/src/altimate/workspace/api-client.ts index 99c2681e58..0ad47b147c 100644 --- a/packages/opencode/src/altimate/workspace/api-client.ts +++ b/packages/opencode/src/altimate/workspace/api-client.ts @@ -138,6 +138,12 @@ async function req( opts: { body?: unknown query?: Record + /** Cap the response body. Off by default because this helper is shared and + * some endpoints legitimately return large payloads (memory ``/list`` + * embeds block content and is not capped server-side). Set it where the + * body size is attacker- or accident-controlled, as skill file downloads + * are. */ + boundResponse?: boolean /** Override the base path prefix. Defaults to * ``/datamate-project-bindings`` (this module's namespace). Pass e.g. * ``/datamates`` to hit the sibling datamates_router through the same @@ -176,7 +182,21 @@ async function req( // swallows the AbortError from the timeout firing during the body read // and turns a stalled response into a false "empty body". Rejection // rethrows into the outer catch and is classified there. (cubic round 3.) - text = await res.text() + // Bound the body before buffering it. `res.text()` reads to completion, so + // a response far larger than advertised is an out-of-memory crash before + // any size check downstream can reject it. Content-Length is a hint, not a + // guarantee, so the stream is also cut off at the cap. + if (opts.boundResponse) { + const declared = Number(res.headers.get("content-length") ?? Number.NaN) + if (Number.isFinite(declared) && declared > MAX_RESPONSE_BYTES) { + throw new WorkspaceApiError( + `Response from ${target} declares ${declared} bytes, over the ${MAX_RESPONSE_BYTES} limit`, + ) + } + text = await readBounded(res, target) + } else { + text = await res.text() + } } catch (err) { // Distinguish "we hit our 15s abort" from "network stack failed" so the // caller can decide differently (retry, longer timeout, offline banner). @@ -246,6 +266,51 @@ async function req( * not duplicate any of it — see ./memory-api.ts, which drives * ``/datamates/memory/*`` through this exact path. Always pass an explicit * ``base``; the default is this module's own namespace. */ +/** Ceiling on a single response body, applied ONLY where a caller opts in. + * + * Nothing upstream bounds what a workspace can hold and the body is buffered + * whole, so an oversized response is a process crash rather than a failed + * request. But this helper is shared: memory `/list` embeds block content and is + * deliberately not capped server-side, so a blanket limit would fail requests + * that work today. Skill file downloads opt in; everything else is unchanged. */ +const MAX_RESPONSE_BYTES = 8 * 1024 * 1024 + +/** Read a response body, refusing to buffer past the cap. */ +async function readBounded(res: Response, target: string): Promise { + // No stream to meter (a mocked or bodyless response): fall back to the + // unbounded read, then enforce the cap on what actually arrived so this + // branch cannot be used to bypass it. + if (!res.body) { + const whole = await res.text() + if (Buffer.byteLength(whole, "utf8") > MAX_RESPONSE_BYTES) { + throw new WorkspaceApiError( + `Response from ${target} exceeded the ${MAX_RESPONSE_BYTES} byte limit`, + ) + } + return whole + } + const reader = res.body.getReader() + const chunks: Uint8Array[] = [] + let total = 0 + try { + for (;;) { + const { done, value } = await reader.read() + if (done) break + if (!value) continue + total += value.byteLength + if (total > MAX_RESPONSE_BYTES) { + throw new WorkspaceApiError( + `Response from ${target} exceeded the ${MAX_RESPONSE_BYTES} byte limit`, + ) + } + chunks.push(value) + } + } finally { + reader.cancel().catch(() => {}) + } + return new TextDecoder().decode(Buffer.concat(chunks)) +} + export { req as altimateRequest } export namespace WorkspaceApi { diff --git a/packages/opencode/src/altimate/workspace/memory-sync.ts b/packages/opencode/src/altimate/workspace/memory-sync.ts index cabf56358d..5b8314aea8 100644 --- a/packages/opencode/src/altimate/workspace/memory-sync.ts +++ b/packages/opencode/src/altimate/workspace/memory-sync.ts @@ -23,7 +23,8 @@ import { Instance } from "@/project/instance" import { Log } from "@/altimate/util/log" import type { MemoryBlock } from "@/memory/types" import { TRAINING_META_COMMENT } from "@/altimate/training/types" -import { readLocalBinding, type CachedBinding } from "./state" +// Aliased: `syncInternals.resolveBinding` below is an unrelated test seam. +import { resolveBinding as resolveProjectBinding, type CachedBinding } from "./state" import { indexKey, readIndex, readIndexEntry, recordIndexEntry } from "./memory-index" import { WorkspaceApi } from "./api-client" import { @@ -133,7 +134,12 @@ async function currentBinding(directory?: string): Promise directory = directory ?? currentDirectory() ?? undefined if (!directory) return null try { - return await readLocalBinding(directory) + // Server fallback, not just the local cache: that cache is written only by + // an explicit link, so a directory holding a repo that IS bound — a git + // worktree, a second clone, a teammate's checkout, a new machine — would + // mirror nothing at all, silently. See `resolveBinding` for why adopting a + // binding here does not also seed the workspace. + return await resolveProjectBinding(directory) } catch (err) { log.warn("could not resolve binding for memory mirror", { err: String(err) }) return null diff --git a/packages/opencode/src/altimate/workspace/skill-sync.ts b/packages/opencode/src/altimate/workspace/skill-sync.ts new file mode 100644 index 0000000000..d1d8e2b357 --- /dev/null +++ b/packages/opencode/src/altimate/workspace/skill-sync.ts @@ -0,0 +1,867 @@ +// altimate_change - new file +// +// Mirror a bound workspace's custom skill bundles onto disk, where the existing +// skill discovery in ``@/skill`` finds them with no other change. +// +// Scope (v0): custom uploaded bundles only. The 21 skills that ship with the +// binary already live in ``~/.altimate/builtin``, and the datamate-computed +// skills document MCP tool names rather than this CLI's tools — neither is +// synced here. +// +// Activation is not handled HERE, but that is not the same as "cannot happen". +// A synced skill is discovered like any other: normally it is listed in +// ```` by name + description and loaded only when the model +// invokes the Skill tool. Whatever frontmatter an author put in the bundle +// flows through untouched — including ``alwaysApply`` and ``applyPaths``, which +// discovery carries into ``Info`` (skill/index.ts) and which +// ``collectAutoLoadedSkills`` (session/system.ts) injects into every applicable +// system prompt with no Skill-tool call and no permission prompt. +// +// That is a real consequence worth stating plainly: anyone who can upload a +// skill to a workspace can put standing instructions into the prompts of every +// member bound to it. The backend has no activation field, so this can only +// arrive through the uploaded SKILL.md. Whether workspace skills should be +// allowed to auto-activate is a product decision, not one this module should +// make silently by stripping frontmatter an author wrote. +// +// Server contract (app/api/datamates/custom_skills.py, mounted at ``/skills``): +// GET "" -> Page[CustomSkillSummary] (paginated) +// GET "/{public_id}" -> CustomSkillDetail (adds files[] + content) +// GET "/{public_id}/files/{p}" -> {path, content} JSON (NOT raw bytes) +// The summary carries ``file_count``, not an inventory, and nothing in the API +// exposes a checksum — ``CustomSkillFileMeta`` is ``{path, size}``. So change +// detection is per-skill ``updated_at`` and the only integrity check available +// is byte length. +import fs from "fs/promises" +import { statSync } from "node:fs" +import path from "path" +import { Flag as CoreFlag } from "@opencode-ai/core/flag/flag" +import { Log } from "@/altimate/util/log" +import { AltimateApi } from "@/altimate/api/client" +import { resolveBindingOutcome, type CachedBinding } from "./state" +import { altimateRequest, WorkspaceApiError } from "./api-client" + +const log = Log.create({ service: "altimate-workspace-skill-sync" }) + +/** Base path for the skills API on the backend (``custom_skills_router`` is + * mounted at ``/skills`` in ``app/main.py``). */ +const SKILLS_BASE = "/skills" + +/** The list endpoint is paginated by ``add_pagination(app)``; walk every page + * rather than trusting the first. A bound is kept so a server that never + * advances ``page`` cannot spin forever. */ +const MAX_PAGES = 50 + +/** Managed subdirectory. Everything inside is ours and may be replaced + * wholesale; nothing outside it is ever written or removed. The directory + * boundary is the ownership marker — we deliberately do NOT stamp a marker + * into the files themselves, because unlike the VS Code extension (which + * generates rule files) we mirror author-written content verbatim, and editing + * it would alter what the model reads. */ +const MANAGED_DIR = path.join(".altimate-code", "skill", "_workspace") +/** Staging lives here, deliberately NOT under `.altimate-code/skill/`, which + * discovery scans. See the swap in `syncSkills`. */ +const STAGING_DIR = path.join(".altimate-code", "skill-staging") +const MANIFEST_NAME = ".manifest.json" + +export interface ManifestSkill { + /** Server's ``updated_at``, verbatim. The only change signal the API offers. */ + updatedAt: string + files: Record +} + +export interface Manifest { + version: 1 + tenant: string + apiUrl: string + datamateId: number + skills: Record +} + +/** A row of ``Page[CustomSkillSummary]``, narrowed to what sync needs. */ +interface RemoteSummary { + publicId: string + updatedAt: string +} + +/** ``CustomSkillDetail.files`` — ``CustomSkillFileMeta`` is ``{path, size}``. */ +interface RemoteFile { + path: string + size: number +} + +export function isEnabled(): boolean { + return CoreFlag.ALTIMATE_WORKSPACE +} + +function managedRoot(directory: string): string { + return path.join(directory, MANAGED_DIR) +} + +/** Every mutable table below is anchored on a process-global rather than being + * plain module state. This file is reached through two different module graphs + * in the same process — the bind path resolves it via one specifier and the + * per-turn hook via another — and the runtime keeps a separate module record + * for each, so module-level `Map`s silently fork — `inFlight` included, which + * would let a bind and a turn stage and swap the same project concurrently. + * A symbol-keyed global gives every copy of this module the same tables. + * + * Note this only reaches copies sharing a realm. Threads do NOT share + * `globalThis`, so anything a bind must hand to a later turn goes through disk + * instead — see `snapshotFingerprint`. */ +const STORE_KEY = Symbol.for("altimate.workspace.skill-sync.store") + +interface SyncStore { + inFlight: Map> + lastSyncedAt: Map + registryAppliedAt: Map + syncedFor: Map +} + +const globals = globalThis as unknown as Record +const store: SyncStore = (globals[STORE_KEY] ??= { + inFlight: new Map(), + lastSyncedAt: new Map(), + registryAppliedAt: new Map(), + syncedFor: new Map(), +}) + +/** In-flight sync per canonical project directory, so a bind and a session + * start racing on the same project do not both stage and swap. */ +const inFlight = store.inFlight + +/** How long a snapshot is trusted before the next turn re-checks the workspace. + * + * `prompt` runs per message, so syncing on every turn would put an HTTP round + * trip in the one path whose first-answer latency is measured. Syncing once per + * process is the other extreme: a skill added in the SaaS never reaches a + * session that is already open. This bounds the staleness instead — at most one + * list call per project per interval, and the list is Postgres-only server-side + * (no S3 reads), so the check is cheap when nothing changed. */ +const POLL_INTERVAL_MS = 5 * 60 * 1000 + +/** Ceilings on one snapshot. Nothing upstream bounds a workspace's size, and + * every file is read fully into memory before it reaches disk, so without these + * a single oversized bundle is an out-of-memory crash rather than a failed + * sync. Exceeding either abandons the snapshot the same way any other error + * does — the previous one is kept. */ +const MAX_TOTAL_BYTES = 32 * 1024 * 1024 +const MAX_TOTAL_FILES = 2000 + +/** Last SUCCESSFUL sync per canonical project, for the interval above. A failed + * attempt must not stamp: doing so suppresses retry for a full interval on a + * transient error, which is the opposite of what a failure should cause. */ +const lastSyncedAt = store.lastSyncedAt + +/** Which snapshot this caller has already refreshed its skill registry for. + * + * The comparison is against a fingerprint taken from DISK, not from a sibling + * in-memory table. A bind and a turn do not share memory: the runtime loads this + * module once per thread, so each gets its own module record AND its own + * `globalThis`. A bind stamping an in-process map is invisible to the thread + * that serves the next turn, which is exactly why a workspace linked + * mid-session stayed invisible to the agent until the process restarted. The + * manifest is swapped into place by every sync that changes files and removed + * by a deactivate, so its mtime moves on exactly the events a refresh must + * follow — and every thread reads the same number. */ +const registryAppliedAt = store.registryAppliedAt + +/** mtime of the snapshot manifest, or 0 when there is no snapshot. Both + * directions matter: a sync that adds one moves this off 0, and a deactivate + * that removes it moves it back, so a purge refreshes the registry too. */ +function snapshotFingerprint(canon: string): number { + try { + return statSync(path.join(managedRoot(canon), MANIFEST_NAME)).mtimeMs + } catch { + return 0 + } +} + +/** Does the skill registry still reflect a different snapshot than the one on + * disk? True until `markRegistryApplied` records the current fingerprint. */ +export function registryStale(directory: string): boolean { + const canon = path.resolve(directory) + const current = snapshotFingerprint(canon) + const applied = registryAppliedAt.get(canon) + // Nothing applied yet: stale only if there IS a snapshot. A project that has + // never synced must not pay a config invalidation on the first turn of every + // session. A bound project does pay one — this cannot tell a snapshot that + // predates boot from one a bind just wrote, and refreshing a registry that + // was already current is a cheap rescan, while missing a new one is the bug + // this whole path exists to prevent. + if (applied === undefined) return current !== 0 + return applied !== current +} + +/** Record that the caller has refreshed the registry for the current snapshot. */ +export function markRegistryApplied(directory: string): void { + const canon = path.resolve(directory) + registryAppliedAt.set(canon, snapshotFingerprint(canon)) +} + +/** Await every sync still in flight, so a short-lived process does not exit + * with one half-finished. + * + * A one-shot `run` ends as soon as its turn does, which is routinely sooner + * than a cold sync completes — measured at ~7.6s against a local backend + * against a 2s wait bound. The staged tree was then discarded on exit and, + * because nothing had been persisted, the next `run` started cold and lost the + * same race: such a project never received its skills at all, however many + * times it was run. The TUI never showed this because it outlives the sync. + * Same reasoning as `awaitBackfill` on the bind path. */ +export async function flushPendingSyncs(timeoutMs = 30_000): Promise { + const pending = [...inFlight.values()] + if (pending.length === 0) return + let timer: ReturnType | undefined + try { + await Promise.race([ + Promise.allSettled(pending), + new Promise((resolve) => { + timer = setTimeout(resolve, timeoutMs) + }), + ]) + } finally { + // An armed timer keeps the event loop alive — the very thing this is + // called to avoid depending on. + if (timer) clearTimeout(timer) + } +} + +/** 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 { + // Checked first, for two reasons. It avoids a credentials read on every + // message when the feature is off — this sits on the latency-measured path. + // And it closes a correctness hole in the other direction: turning the flag + // off AFTER a successful sync left `lastSyncedAt` inside the interval with a + // matching account, so `syncSkills` was never called, `deactivate` never ran, + // and the snapshot stayed live for up to a full interval. + if (!isEnabled()) return false + const canon = path.resolve(directory) + const at = lastSyncedAt.get(canon) + if (at === undefined || Date.now() - at >= POLL_INTERVAL_MS) return false + // Scoped to the account in play RIGHT NOW, not the one the snapshot was + // fetched for. Without this an account switch inside the interval keeps + // serving the previous tenant's skills, because the poll that would notice + // the change is the thing being skipped. + let now: string | null = null + try { + const creds = await AltimateApi.getCredentials() + now = accountKeyOf(creds.altimateInstanceName, creds.altimateUrl) + } catch { + // Unreadable OR absent — both fall through and let the sync decide, which + // is the only place that distinguishes disconnected from corrupt. + now = null + } + return now !== null && syncedFor.get(canon) === now +} + +/** Which account each project's snapshot was last fetched for. */ +const syncedFor = store.syncedFor + +function accountKeyOf(tenant: string, apiUrl: string): string { + return `${tenant}\u0000${apiUrl}` +} + +async function readManifest(directory: string): Promise { + try { + const raw = await fs.readFile(path.join(managedRoot(directory), MANIFEST_NAME), "utf8") + const parsed = JSON.parse(raw) as unknown + if (!parsed || typeof parsed !== "object") return null + const m = parsed as Partial + // A manifest we cannot validate is treated as absent, never as ownership: + // the tree it describes gets rebuilt rather than trusted. + if (m.version !== 1) return null + if (typeof m.datamateId !== "number") return null + if (typeof m.tenant !== "string" || typeof m.apiUrl !== "string") return null + if (!m.skills || typeof m.skills !== "object") return null + return m as Manifest + } catch { + return null + } +} + +/** Reject a bundle path that would escape the skill's own directory. */ +function safeRelativePath(p: unknown): p is string { + if (typeof p !== "string" || !p) return false + if (path.isAbsolute(p)) return false + if (p.includes("\0")) return false // symmetrical with safePathComponent + return !p.split(/[\\/]/).includes("..") +} + +/** Reject a ``public_id`` that is not usable as a single directory name. + * + * The id is server-generated, but it is still remote input concatenated into a + * filesystem path. Without this a malformed or compromised response could place + * bundle files anywhere the process can write — the per-file guard above does + * not help, because the escape happens one component earlier. */ +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 + return !/[\\/\0]/.test(p) +} + +/** Read one page of the list endpoint, refusing anything unrecognised. + * + * Returning null means "error", never "empty" — the distinction is what stops a + * malformed 200 from reading as an empty workspace and deleting the user's + * tree. ``api-client``'s helpers coerce unknown envelopes to ``[]``, so an + * empty result is only trustworthy when the envelope itself parsed. */ +function parsePage(payload: unknown, expectedPage: number): { rows: RemoteSummary[]; pages: number } | null { + if (!payload || typeof payload !== "object") return null + const p = payload as { items?: unknown; pages?: unknown } + if (!Array.isArray(p.items)) return null + // `pages` decides when to stop paginating, so a missing or nonsense value + // must be an error, not a default of 1 — defaulting turns a partial first + // page into "the whole workspace" and prunes everything on later pages. + const rawPages = (payload as { pages?: unknown }).pages + if (typeof rawPages !== "number" || !Number.isInteger(rawPages) || rawPages < 1) return null + const pages = rawPages + // An empty page while the envelope claims rows exist is a proxy or backend + // inconsistency, not an empty workspace — and "empty workspace" is the one + // answer that deletes the user's snapshot. Refuse it. + const total = (payload as { total?: unknown }).total + if (p.items.length === 0 && typeof total === "number" && total > 0) return null + // A page that is not the one requested means the accumulation below would be + // wrong; treat it as unrecognised rather than merging it. + const echoed = (payload as { page?: unknown }).page + if (typeof echoed === "number" && echoed !== expectedPage) return null + const rows: RemoteSummary[] = [] + for (const row of p.items) { + if (!row || typeof row !== "object") return null + const r = row as { public_id?: unknown; updated_at?: unknown } + if (typeof r.public_id !== "string" || !r.public_id) return null + if (typeof r.updated_at !== "string" || !r.updated_at) return null + rows.push({ publicId: r.public_id, updatedAt: r.updated_at }) + } + return { rows, pages } +} + +/** ``GET /skills/{id}/files/{path}`` answers ``{path, content}``. Anything else + * is an error, not an empty file — see ``parsePage`` for why that matters. */ +function parseFileContent(body: unknown, expectedPath: string): string | null { + if (!body || typeof body !== "object") return null + const b = body as { content?: unknown; path?: unknown } + // The echoed path must be the one requested. Without checking it, a + // mis-routed or cached response of the same length is written under the + // filename we asked for — and no checksum exists to catch it later. + // Required, not "checked when present": a mis-routed or cached response is + // exactly the case where the field may be absent, which is what this guard + // was written for. + if (b.path !== expectedPath) return null + return typeof b.content === "string" ? b.content : null +} + +function parseDetailFiles(payload: unknown): RemoteFile[] | null { + if (!payload || typeof payload !== "object") return null + // The detail view wraps its body in ``{skill: {...}}`` while the list view + // does not wrap at all. Verified against a local backend on `development`; + // the inconsistency is the contract, so accept the wrapper and also the bare + // object in case the envelope is ever dropped. + const inner = (payload as { skill?: unknown }).skill + const body = inner && typeof inner === "object" ? inner : payload + const files = (body as { files?: unknown }).files + if (!Array.isArray(files)) return null + const out: RemoteFile[] = [] + for (const f of files) { + if (!f || typeof f !== "object") return null + const e = f as { path?: unknown; size?: unknown } + if (!safeRelativePath(e.path)) return null + if (typeof e.size !== "number" || e.size < 0) return null + out.push({ path: e.path, size: e.size }) + } + return out +} + +/** Is the managed directory ours to replace? + * + * Ours means: absent, or present with the manifest this module writes. Anything + * else is a directory that happens to sit at our path — a hand-written skill, a + * checkout from an older tool — and we must not delete it. The name is ours by + * convention only, and convention is not an ownership check. */ +async function ownsManagedDir(directory: string): Promise { + const root = managedRoot(directory) + let entries: string[] + try { + entries = await fs.readdir(root) + } catch (err) { + // ONLY "absent" means the first sync may create it. `readdir` also throws + // ENOTDIR for a plain file at this path and EACCES for a directory we + // cannot read — answering "ours" to those hands a user's own file to + // `fs.rm`, which is the exact outcome this guard exists to prevent. + return (err as NodeJS.ErrnoException)?.code === "ENOENT" + } + if (entries.length === 0) return true + if (!entries.includes(MANIFEST_NAME)) return false + // The filename alone is not proof. A directory holding an unrelated or + // corrupt `.manifest.json` is someone else's; require one we can actually + // read as ours. + return (await readManifest(directory)) !== null +} + +/** Is every component this module writes through a real directory? + * + * `readdir` follows symlinks, so a repository shipping + * ``.altimate-code/skill-staging -> ../..`` (git tracks symlinks, so it + * survives a clone) would have the sweep below enumerate and recursively delete + * the link's target. The same applies to symlinked ANCESTORS, which every + * mkdir, rename and write resolves through. Nothing here should ever traverse a + * link, so refuse rather than try to make traversal safe. */ +async function pathsAreReal(directory: string): Promise { + const candidates = [ + path.join(directory, ".altimate-code"), + path.join(directory, ".altimate-code", "skill"), + path.join(directory, STAGING_DIR), + managedRoot(directory), + ] + for (const candidate of candidates) { + try { + const st = await fs.lstat(candidate) + if (!st.isDirectory()) return false // a symlink lstats as a link, not a dir + } catch (err) { + // Absent is fine — it will be created as a real directory. + if ((err as NodeJS.ErrnoException)?.code !== "ENOENT") return false + } + } + return true +} + +/** Remove staging trees this project abandoned — a SIGKILL mid-sync leaves one + * behind, and nothing else would ever collect it. */ +async function sweepStaging(directory: string): Promise { + const dir = path.join(directory, STAGING_DIR) + let entries: string[] + try { + entries = await fs.readdir(dir) + } catch { + return // nothing staged + } + for (const entry of entries) { + // Leave another process's work alone. These are named `-`, and + // deleting a live owner's staging makes it publish a snapshot missing every + // file written before the sweep, with a manifest that claims them. + const owner = /-(\d+)$/.exec(entry)?.[1] + if (owner && owner !== String(process.pid) && processAlive(Number(owner))) continue + const target = path.join(dir, entry) + try { + const st = await fs.lstat(target) + if (!st.isDirectory()) { + // A symlink here would have `rm -r` follow into its target. + await fs.unlink(target).catch(() => {}) + continue + } + } catch { + continue + } + await fs.rm(target, { recursive: true, force: true }).catch(() => {}) + } +} + +/** Is a PID still running? Used only to avoid sweeping a live sibling's staging; + * a wrong answer costs a stale directory, never a deletion of live work. */ +function processAlive(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) return false + try { + process.kill(pid, 0) + return true + } catch (err) { + return (err as NodeJS.ErrnoException)?.code === "EPERM" + } +} + +/** Take the snapshot out of service when this client is no longer entitled to + * serve it — the account was disconnected, or the feature was switched off. + * + * Leaving it is not neutral. Discovery loads whatever is on disk without + * consulting the manifest, so a disconnected user keeps getting the workspace's + * skills, and any of them carrying ``alwaysApply`` keeps being injected into + * every prompt. Returns whether anything was actually removed, so the caller + * knows to refresh the registry. + * + * Only removes a tree this client owns, for the same reason the sync does. */ +async function deactivate(directory: string, why: string): Promise { + const root = managedRoot(directory) + try { + await fs.stat(root) + } catch { + return false // nothing published here + } + if (!(await ownsManagedDir(directory))) return false + await removeManaged(directory) + await sweepStaging(directory) + log.info("removed the workspace skill snapshot", { why, path: root }) + return true +} + +async function removeManaged(directory: string): Promise { + await fs.rm(managedRoot(directory), { recursive: true, force: true }) +} + +/** Sync the bound workspace's custom skills into ``directory``. + * + * Never throws: skills must not be able to block a bind or a turn. Every + * failure path leaves whatever is already on disk in place, except the + * deliberate purge described below. */ +export async function syncSkills(directory: string): Promise<{ changed: boolean }> { + const canon = path.resolve(directory) + // Joined BEFORE the flag is read, so the opt-out purge is serialised against + // a sync too. Both paths write the same tree; with the purge outside this + // gate, an enabled run already past its own flag check could publish + // `_workspace` moments after a disabled run deleted it, leaving a snapshot on + // disk for a feature that is off. (bot review) + const existing = inFlight.get(canon) + if (existing) { + // Report the joined run's real outcome. Returning a hard-coded `false` is a + // false answer waiting for the next caller to trust it. + return await existing.catch(() => ({ changed: false })) + } + if (!isEnabled()) { + // Opting out has to actually take effect: a snapshot left behind keeps + // loading into every session. Still gated on the symlink check — this path + // deletes, and it runs before the one inside `run`, so without it a + // symlinked `.altimate-code` would have the purge follow the link. + const purge = (async () => { + const dropped = (await pathsAreReal(canon).catch(() => false)) + ? await deactivate(canon, "the workspace feature is off").catch(() => false) + : false + return { changed: dropped } + })() + inFlight.set(canon, purge) + try { + return await purge + } finally { + inFlight.delete(canon) + } + } + let changed = false + let failed = false + // Set once the workspace's list has actually been read. Only then has this + // project been "checked", and only then should the poll interval start. + let sawRemote = false + const run = (async () => { + // Checked BEFORE the binding: `resolveBinding` needs credentials too, so a + // disconnected client would otherwise return on a null binding and never + // reach this. Disconnected is different from "could not read the + // credentials" — the first is a decision the user made and must take + // effect, the second is unknown, and unknown never destroys a snapshot. + if (!(await AltimateApi.isConfigured())) { + // Symlink-guarded like every other deletion here. This branch runs BEFORE + // the `pathsAreReal` check below, so without its own guard a symlinked + // `.altimate-code` has `removeManaged`/`sweepStaging` resolve through the + // link and delete the target's tree. (bot review) + if ((await pathsAreReal(canon).catch(() => false)) && (await deactivate(canon, "no altimate credentials"))) + changed = true + return + } + + // Nothing below should ever traverse a symlink. Checked before any write, + // rename or sweep, all of which resolve through these components. + if (!(await pathsAreReal(canon))) { + log.warn("refusing to sync: a workspace skill path is not a real directory", { + path: managedRoot(canon), + }) + failed = true + return + } + + // Read credentials and the manifest BEFORE resolving the binding, so an + // account switch can be acted on. `resolveBinding` returns null for both + // "confirmed unbound" and "lookup failed", and the old code returned on + // that null before ever reaching the foreign-manifest purge — so switching + // to an account with no binding here left the previous tenant's skills on + // disk and loading into prompts, with every retry hitting the same return. + const credsForPurge = await AltimateApi.getCredentials().catch(() => null) + if (credsForPurge) { + const priorManifest = await readManifest(canon) + if ( + priorManifest && + (priorManifest.tenant !== credsForPurge.altimateInstanceName || + priorManifest.apiUrl !== credsForPurge.altimateUrl) + ) { + if (await deactivate(canon, "the snapshot belongs to another account")) changed = true + } + } + + // `resolveBinding`, not `readLocalBinding`: the local cache is written only + // by an explicit link, so a project bound server-side (fresh clone, new + // machine, cleared state) would otherwise never get its workspace's skills. + const outcome = await resolveBindingOutcome(canon) + if (outcome.status !== "bound") { + // A CONFIRMED unbind must take the snapshot out of service — discovery + // does not consult the manifest, so leaving it keeps serving a workspace + // this project is no longer attached to. "Unknown" must not: a lookup + // failure is not evidence of anything, and deleting on it would wipe a + // snapshot on a network blip. + if (outcome.status === "unbound") { + if (await deactivate(canon, "this project is no longer bound to a workspace")) changed = true + } + return + } + const binding = outcome.binding + + // Refuse to touch a directory we did not create. Everything below either + // deletes this tree or replaces it wholesale, so without this a user's own + // files at our path are destroyed by a routine sync. + if (!(await ownsManagedDir(canon))) { + log.warn( + "refusing to manage the workspace skill directory: it has contents this client did not write", + { path: managedRoot(canon) }, + ) + return + } + await sweepStaging(canon) + + let creds: { altimateUrl: string; altimateInstanceName: string } + try { + creds = await AltimateApi.getCredentials() + } catch (err) { + log.warn("could not read altimate credentials; keeping the existing snapshot", { + err: String(err), + }) + return + } + + const manifest = await readManifest(canon) + + // Purge on rebind or account change. ``recordApprovedBinding`` persists the + // new binding before any sync runs, and skill discovery loads whatever is + // on disk without consulting this manifest — so leaving a previous + // workspace's tree in place through a failed pull would silently feed the + // model another workspace's skills. Empty is correct; wrong-workspace is not. + const foreign = + manifest !== null && + (manifest.datamateId !== binding.datamateId || + manifest.tenant !== creds.altimateInstanceName || + manifest.apiUrl !== creds.altimateUrl) + if (foreign) { + log.info("this project's snapshot belongs to another workspace or account; dropping it", { + was: manifest.datamateId, + now: binding.datamateId, + }) + await removeManaged(canon) + changed = true + } + + const remote = await listAll(binding) + if (!remote) return // error, not empty — keep what is on disk + sawRemote = true + syncedFor.set(canon, accountKeyOf(creds.altimateInstanceName, creds.altimateUrl)) + + if (!foreign && (await upToDate(canon, manifest, remote))) return + + if (remote.length === 0) { + await removeManaged(canon) + changed = true + log.info("workspace has no custom skills; removed the local snapshot") + return + } + + // Stage a complete snapshot, then swap. A partial bundle is never + // published: any failure abandons the staging directory and leaves the + // previous snapshot untouched. + const root = managedRoot(canon) + // Staged OUTSIDE `.altimate-code/skill/`, because discovery globs + // `{skill,skills}/**/SKILL.md` from the config dir — a staging tree that + // lived beside `_workspace` would be scanned, so a half-downloaded snapshot + // (or one abandoned by a SIGKILL) would be loaded as real skills. + const staging = path.join(canon, STAGING_DIR, `pending-${process.pid}`) + await fs.mkdir(path.join(canon, STAGING_DIR), { recursive: true }) + await fs.writeFile(path.join(canon, STAGING_DIR, ".gitignore"), "*\n").catch(() => {}) + await fs.rm(staging, { recursive: true, force: true }) + const next: Manifest = { + version: 1, + tenant: creds.altimateInstanceName, + apiUrl: creds.altimateUrl, + datamateId: binding.datamateId, + skills: {}, + } + try { + let totalFiles = 0 + let totalBytes = 0 + for (const summary of remote) { + if (!safePathComponent(summary.publicId)) { + throw new WorkspaceApiError(`unusable skill id in the workspace listing: ${summary.publicId}`) + } + const detail = await altimateRequest( + "GET", + `/${encodeURIComponent(summary.publicId)}`, + { base: SKILLS_BASE }, + ) + const files = parseDetailFiles(detail) + if (!files) throw new WorkspaceApiError(`unrecognised detail for ${summary.publicId}`) + const recorded: Record = {} + for (const file of files) { + totalFiles += 1 + totalBytes += file.size + if (totalFiles > MAX_TOTAL_FILES || totalBytes > MAX_TOTAL_BYTES) { + throw new WorkspaceApiError( + `workspace skill bundle exceeds the client limit (${totalFiles} files, ${totalBytes} bytes)`, + ) + } + const encoded = file.path.split("/").map(encodeURIComponent).join("/") + // The file endpoint answers with ``{path, content}`` JSON, not the raw + // object — the server decodes the bundle file and hands back a string. + const body = await altimateRequest( + "GET", + `/${encodeURIComponent(summary.publicId)}/files/${encoded}`, + // Bounded: this is the one response whose size is set by remote + // bundle content rather than by our own query. + { base: SKILLS_BASE, boundResponse: true }, + ) + const content = parseFileContent(body, file.path) + if (content === null) { + throw new WorkspaceApiError(`unrecognised file body for ${summary.publicId}/${file.path}`) + } + // No checksum exists in the API, so length is the only integrity check + // available. `size` is the stored object's byte count, so the + // comparison has to be on UTF-8 bytes rather than string length — the + // two differ for any non-ASCII skill. It still catches a truncated + // download, which is what would otherwise publish half a skill. + const bytes = Buffer.from(content, "utf8") + if (bytes.byteLength !== file.size) { + throw new WorkspaceApiError( + `size mismatch for ${summary.publicId}/${file.path}: expected ${file.size}, got ${bytes.byteLength}`, + ) + } + const dest = path.join(staging, summary.publicId, file.path) + await fs.mkdir(path.dirname(dest), { recursive: true }) + await fs.writeFile(dest, bytes) + recorded[file.path] = file.size + } + next.skills[summary.publicId] = { updatedAt: summary.updatedAt, files: recorded } + } + // Manifest goes inside the staged tree so files and manifest commit + // together — a snapshot is never live without the record of what it is. + // Ignore everything this directory holds, itself included. The tree is + // a mirror of the workspace and is rebuilt from the server on demand, so + // it has no business in the user's history — and committing it would put + // one workspace's private instructions into a repo other workspaces read. + // 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)) + // 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 + // after the new one is in place. + await fs.mkdir(path.dirname(root), { recursive: true }) + const retired = path.join(canon, STAGING_DIR, `retired-${process.pid}`) + await fs.rm(retired, { recursive: true, force: true }).catch(() => {}) + let hadPrevious = true + try { + await fs.rename(root, retired) + } catch { + hadPrevious = false // nothing published yet + } + try { + await fs.rename(staging, root) + } catch (err) { + if (hadPrevious) await fs.rename(retired, root).catch(() => {}) + throw err + } + await fs.rm(retired, { recursive: true, force: true }).catch(() => {}) + changed = true + log.info("workspace skills synced", { + datamateId: binding.datamateId, + skills: remote.length, + }) + } catch (err) { + failed = true + await fs.rm(staging, { recursive: true, force: true }).catch(() => {}) + log.warn("workspace skill sync failed; kept the existing snapshot", { err: String(err) }) + } + })() + // Published to `inFlight` so a joining caller awaits the SAME settled result + // this one returns, rather than a hard-coded guess. + const settled = (async () => { + let ok = true + try { + await run + } catch (err) { + ok = false + log.warn("workspace skill sync errored", { err: String(err) }) + } + // 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()) + return { changed } + })() + inFlight.set(canon, settled) + try { + return await settled + } finally { + inFlight.delete(canon) + } +} + +/** Walk every page of the list endpoint. Returns null on any error or + * unrecognised payload — callers must treat that as "unknown", not "empty". */ +async function listAll(binding: CachedBinding): Promise { + const all: RemoteSummary[] = [] + for (let page = 1; page <= MAX_PAGES; page++) { + let payload: unknown + try { + payload = await altimateRequest("GET", "", { + base: SKILLS_BASE, + query: { datamate_id: String(binding.datamateId), page: String(page) }, + }) + } catch (err) { + log.warn("could not list workspace skills; keeping the existing snapshot", { + err: String(err), + }) + return null + } + const parsed = parsePage(payload, page) + if (!parsed) { + log.warn("workspace skill list was not in a recognised shape; keeping the existing snapshot") + return null + } + // Dedupe across pages. The manifest is keyed by id, so a row repeated + // across a page boundary makes the length comparison in `upToDate` + // permanently unequal and re-downloads the whole workspace every poll. + for (const row of parsed.rows) { + if (!all.some((seen) => seen.publicId === row.publicId)) all.push(row) + } + if (page >= parsed.pages || parsed.rows.length === 0) return all + } + log.warn("workspace skill list exceeded the page bound; keeping the existing snapshot") + return null +} + +/** Does the on-disk snapshot already match the remote set? + * + * Compared on the server's ``updated_at`` per skill, because the API exposes no + * checksum and the list carries only ``file_count``. */ +async function upToDate( + directory: string, + manifest: Manifest | null, + remote: RemoteSummary[], +): Promise { + if (!manifest) return false + const ids = Object.keys(manifest.skills) + if (ids.length !== remote.length) return false + for (const summary of remote) { + const local = manifest.skills[summary.publicId] + if (!local || local.updatedAt !== summary.updatedAt) return false + } + // The manifest agreeing with the server says nothing about the files still + // being there. A deleted, truncated or partially checked-out snapshot would + // otherwise be declared current forever, and the missing skill would never + // come back. The recorded sizes are already on hand, so verify against them. + const root = managedRoot(directory) + for (const [publicId, entry] of Object.entries(manifest.skills)) { + for (const [rel, size] of Object.entries(entry.files)) { + try { + const stat = await fs.stat(path.join(root, publicId, rel)) + if (stat.size !== size) return false + } catch { + return false + } + } + } + return true +} diff --git a/packages/opencode/src/altimate/workspace/state.ts b/packages/opencode/src/altimate/workspace/state.ts index 1c5930e02a..33f3b797b5 100644 --- a/packages/opencode/src/altimate/workspace/state.ts +++ b/packages/opencode/src/altimate/workspace/state.ts @@ -16,6 +16,9 @@ import { AltimateApi } from "@/altimate/api/client" import { Global } from "@/global" import { Filesystem } from "@/util/filesystem" import { Log } from "@/altimate/util/log" +// Type-only: the value side is imported dynamically in resolveBinding to keep +// this module's import graph free of the API client at load time. +import type { Binding, ProjectBindingLookup } from "./api-client" const CACHE_VERSION = 1 @@ -30,6 +33,11 @@ export interface CachedBinding { repoRemote: string | null projectPath: string | null linkedAt: number + /** True when this row was adopted from the server rather than created by an + * explicit link. Consumers that mean "the user approved this" must require + * ``!adopted``; the absent ``seededAt`` is not a substitute, because only the + * memory backfill consults it. */ + adopted?: boolean /** Set once a bind-time seed completed without failures. Absent means the * seed has not run, errored, or was skipped because memory was off — all of * which must stay retryable, so a later warm sweeps again. */ @@ -245,6 +253,201 @@ function sameBinding(a: CachedBinding, b: CachedBinding): boolean { ) } +/** Projects the server has already said are unbound, so an unbound project + * pays the lookup once per process instead of once per sync. Keyed on the + * canonical directory. Never holds a positive result — a hit is written to the + * real cache, which is what later reads consult. */ +const serverLookupMissed = new Map() + +/** The composite key both the negative-lookup memo and the revalidation stamp + * are filed under. Includes the account, so switching tenants never inherits + * the other account's verdict for the same directory. */ +function accountScopedKey(directory: string, key: { tenant: string; apiUrl: string }): string { + return `${key.tenant}\u0000${key.apiUrl}\u0000${canonicalizeKey(directory)}` +} + +/** Forget a memoized "no binding here" answer. An explicit link is newer + * information than any miss recorded before it: without this, linking within + * `MISS_TTL_MS` of a turn taken while unlinked has the revalidation below read + * the stale miss, call it authoritative, and delete the row the link just + * wrote. (bot review) */ +function clearLookupMiss(directory: string, key: { tenant: string; apiUrl: string }): void { + serverLookupMissed.delete(accountScopedKey(directory, key)) +} + +/** How long a "this project is unbound" answer is trusted. Bounded because the + * answer changes the moment someone links the project in the SaaS: a permanent + * memo means skills and memory never appear until the process restarts. Keyed + * with the tenant and API host so switching accounts does not inherit the other + * account's verdict. */ +const MISS_TTL_MS = 5 * 60 * 1000 + +/** How long a cached POSITIVE binding is trusted before the server is asked + * again. The cache is written by an explicit link and otherwise never expires, + * so without this a project rebound or detached in the SaaS keeps serving its + * OLD workspace's skills on this machine forever — including any carrying + * `alwaysApply`. The server is authoritative; the cache covers the window + * between checks and the case where the server cannot be reached. */ +const REVALIDATE_MS = 5 * 60 * 1000 + +/** When each project's cached binding was last confirmed against the server. */ +const lastValidatedAt = new Map() + +/** The binding for ``directory``: the local cache when it has one, otherwise + * the server's answer, written to the cache for next time. + * + * The cache is only ever written by an explicit link. A project that is bound + * server-side but has no local entry — a fresh clone of a repo a teammate + * linked, a new machine, cleared state — therefore looks unbound to every + * consumer, while ``link`` refuses to help because the server reports it as + * already linked. That combination leaves the project permanently without + * workspace skills and with no way out from the CLI. + * + * Adopting a binding here is a read, not an approval. The lookup is + * access-controlled server-side (a workspace the caller cannot see answers 404 + * exactly as an unbound remote does), so this can only surface a binding the + * caller could already see. It deliberately writes NO ``seededAt`` and does not + * run the memory backfill: pulling a workspace's skills is read-only, whereas + * pushing this machine's memory into a shared workspace is a write that stays + * behind a real link. + * + * Never throws — a lookup failure is "unknown", which callers treat as "leave + * whatever is on disk alone". */ +export async function resolveBinding(directory: string): Promise { + const outcome = await resolveBindingOutcome(directory) + return outcome.status === "bound" ? outcome.binding : null +} + +/** Whether a project is bound, and — crucially — whether we actually know. + * + * `null` collapses "the server confirmed this project is unbound" with "we + * could not find out". Callers that DELETE on unbound must not act on the + * second: a network blip would wipe a snapshot the user is still entitled to. + * Callers that only need a binding can keep using `resolveBinding`. */ +export type BindingOutcome = + | { status: "bound"; binding: CachedBinding } + | { status: "unbound" } + | { status: "unknown" } + +export async function resolveBindingOutcome(directory: string): Promise { + const local = await readLocalBinding(directory).catch(() => null) + + const key = await tenantKey() + if (!key) return local ? { status: "bound", binding: local } : { status: "unknown" } + + // A cached binding is trusted only inside the revalidation window. Past it + // the server decides, because it is the only thing that knows about a rebind + // or a detach performed elsewhere. + if (local) { + const validated = lastValidatedAt.get(accountScopedKey(directory, key)) + if (validated !== undefined && Date.now() - validated < REVALIDATE_MS) { + return { status: "bound", binding: local } + } + const fresh = await lookupBinding(directory, key) + if (fresh.status === "unknown") { + // Cannot reach the server: keep serving what we have rather than tearing + // a working setup down over a network blip. + return { status: "bound", binding: local } + } + lastValidatedAt.set(accountScopedKey(directory, key), Date.now()) + if (fresh.status === "unbound") { + forgetBinding(directory, key) + return { status: "unbound" } + } + // Rebound elsewhere: adopt the server's answer, replacing the cached row. + if (fresh.binding.datamateId !== local.datamateId) return fresh + return { status: "bound", binding: local } + } + return await lookupBinding(directory, key) +} + +/** Drop a cached row the server no longer recognises, so later reads do not + * resurrect it from disk. */ +function forgetBinding(directory: string, key: { tenant: string; apiUrl: string }): void { + try { + const cache = readCache() + if (!cache || cache.tenant !== key.tenant || cache.apiUrl !== key.apiUrl) return + delete cache.bindings[canonicalizeKey(directory)] + writeCache(cache) + } catch (err) { + log.warn("could not drop a binding the server no longer recognises", { err: String(err) }) + } +} + +/** The server's answer for this project, with no cache consulted. */ +async function lookupBinding( + directory: string, + key: { tenant: string; apiUrl: string }, +): Promise { + const canon = accountScopedKey(directory, key) + const missedAt = serverLookupMissed.get(canon) + if (missedAt !== undefined && Date.now() - missedAt < MISS_TTL_MS) return { status: "unbound" } + + let hit: ProjectBindingLookup | null = null + try { + const { resolveProjectIdentifier } = await import("./detect") + const { WorkspaceApi } = await import("./api-client") + hit = await WorkspaceApi.getBindingForProject(resolveProjectIdentifier(directory)) + } catch (err) { + // Unreachable or a 5xx: unknown, not unbound. Deliberately NOT memoized — + // the next session should ask again rather than inherit a network blip. + log.warn("could not look up the workspace binding for this project", { err: String(err) }) + return { status: "unknown" } + } + if (!hit) { + serverLookupMissed.set(canon, Date.now()) + return { status: "unbound" } + } + // cubic P2: a malformed 2xx would otherwise throw on the dereference below, + // outside the try above, aborting the whole sync. An unrecognised body is + // unknown, not unbound — the same rule the rest of this feature follows. + const row = (hit as { binding?: Partial }).binding + if ( + !row || + typeof row.datamate_id !== "number" || + typeof row.datamate_name !== "string" || + (row.repo_remote !== null && row.repo_remote !== undefined && typeof row.repo_remote !== "string") || + (row.project_path !== null && row.project_path !== undefined && typeof row.project_path !== "string") + ) { + log.warn("workspace binding lookup returned an unrecognised body; treating as unknown") + return { status: "unknown" } + } + + const adopted: CachedBinding = { + adopted: true, + datamateId: row.datamate_id, + datamateName: row.datamate_name, + repoRemote: row.repo_remote ?? null, + projectPath: row.project_path ?? null, + linkedAt: Date.now(), + } + try { + const existing = readCache() + const cache: CacheFile = + existing && existing.tenant === key.tenant && existing.apiUrl === key.apiUrl + ? existing + : { version: CACHE_VERSION, tenant: key.tenant, apiUrl: key.apiUrl, bindings: {} } + // Confirming the binding the cache already holds is not an adoption: keep + // the explicit-link label and the seed marker, or a later re-link re-runs + // the whole memory backfill and an explicit row silently becomes `adopted`. + const prior = cache.bindings[canonicalizeKey(directory)] + cache.bindings[canonicalizeKey(directory)] = + prior && prior.datamateId === adopted.datamateId + ? { ...adopted, adopted: prior.adopted, seededAt: prior.seededAt, linkedAt: prior.linkedAt } + : adopted + writeCache(cache) + } 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. + log.warn("could not cache the workspace binding discovered on the server", { err: String(err) }) + } + lastValidatedAt.set(accountScopedKey(directory, key), Date.now()) + log.info("adopted the workspace binding this project already has on the server", { + datamateId: adopted.datamateId, + }) + return { status: "bound", binding: adopted } +} + export async function recordApprovedBinding( directory: string, binding: CachedBinding, @@ -252,6 +455,13 @@ export async function recordApprovedBinding( ): Promise { const key = await tenantKey() if (!key) return + // An explicit link is the newest word on this project, so retire any memoized + // "no binding here" from before it and count the row as server-validated — + // the link is what created it. Without the first, revalidation reads the + // stale miss and deletes the row this call just wrote; without the second, + // every bind pays an immediate round trip to confirm what it just did. + clearLookupMiss(directory, key) + lastValidatedAt.set(accountScopedKey(directory, key), Date.now()) // Best-effort: cache persistence is a UX convenience, not the source of // truth (the server-side binding is). If the state directory is read-only // or the disk is full, callers otherwise report "link failed" and prompt @@ -298,6 +508,20 @@ export async function recordApprovedBinding( // as a command handler returns (src/index.ts): a detached sweep is killed // mid-flight there, so a bind that reported success could seed nothing. The // TUI stays resident and leaves it detached so the dialog closes at once. + // Pull the workspace's custom skills. Deliberately ABOVE the ``alreadySeeded`` + // return below: that marker tracks the one-shot memory seed, and skills are a + // different lifecycle — they must re-sync on every bind, including a rebind to + // a workspace this machine has already seeded memory for. Awaited on the same + // condition as the backfill, for the same reason: the CLI exits as soon as the + // handler returns, so a detached sync there would be killed mid-flight. + const skillsSynced = import("./skill-sync") + .then((m) => m.syncSkills(canonicalizeKey(directory))) + .catch((err) => { + log.warn("could not sync workspace skills", { err: String(err) }) + return { changed: false } + }) + if (opts?.awaitBackfill) await skillsSynced + // Skip only when this exact binding has already been seeded successfully. A // warm after a failed or skipped seed must try again, or the blocks this // machine already holds never reach the workspace. diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 50638bcd14..a4db4cb999 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -4,6 +4,9 @@ import { pathToFileURL } from "url" import { UI } from "../ui" import { cmd } from "./cmd" import { Flag } from "../../flag/flag" +// altimate_change start — workspace feature gate (see the flush after loopPromise) +import { Flag as CoreFlag } from "@opencode-ai/core/flag/flag" +// altimate_change end import { bootstrap } from "../bootstrap" import { EOL } from "os" import { Filesystem } from "../../util/filesystem" @@ -904,6 +907,19 @@ You are speaking to a non-technical business executive. Follow these rules stric // Wait for the event loop to drain (breaks when session reaches idle) await loopPromise + // altimate_change start — a cold workspace skill sync outlives a short + // turn, and this process exits the moment the turn ends. Without this the + // staged tree is discarded on exit and, since nothing was persisted, the + // next `run` starts cold and loses the same race — so such a project never + // 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(() => {}) + } + // altimate_change end + // Remove crash handlers — trace will be finalized cleanly process.removeListener("SIGINT", onSigint) process.removeListener("SIGTERM", onSigterm) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 268babfc66..523cb9474a 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1,4 +1,5 @@ import path from "path" +import { existsSync } from "node:fs" import os from "os" import fs from "fs/promises" import z from "zod" @@ -36,6 +37,9 @@ import { LSP } from "../lsp" import { ReadTool } from "../tool/read" import { FileTime } from "../file/time" import { Flag } from "../flag/flag" +// altimate_change — sync flag read, so the workspace-skill hook below can cost +// literally nothing (not even an await) for users who never opted in. +import { Flag as CoreFlag } from "@opencode-ai/core/flag/flag" import { ulid } from "ulid" import { spawn } from "child_process" import { Command } from "../command" @@ -96,6 +100,39 @@ const STRUCTURED_OUTPUT_SYSTEM_PROMPT = `IMPORTANT: The user has requested struc export namespace SessionPrompt { const log = Log.create({ service: "session.prompt" }) + // altimate_change start — how long a turn will wait for the workspace skill + // sync before proceeding without it. See the block in `prompt`. + const WORKSPACE_SKILL_WAIT_MS = 2000 + + /** Drop the caches in front of the synced skill files, if a sync moved them. + * Shared by both branches of the block in `prompt`: opting out removes a + * snapshot and needs the registry refreshed exactly as adding one does. */ + async function refreshSkillRegistry(dir: string): Promise { + const skillSync = await import("../altimate/workspace/skill-sync") + if (!skillSync.registryStale(dir)) return + // Marked BEFORE the work, not after: a refresh that throws must not be + // retried on every subsequent turn forever, and the next real snapshot + // change re-arms this anyway. + skillSync.markRegistryApplied(dir) + const { Skill } = await import("../skill") + // Both drops go through the in-context services rather than the imperative + // facades. Discovery re-derives its roots from `Config.directories()`, which + // walks up looking for `.altimate-code/`; on a project that has never synced, + // that directory does not exist at boot, so the list holds a miss that only + // an invalidate reaching *this* instance can clear. Invalidating through the + // facade leaves it, and the refreshed registry then rescans the same empty + // root set — the skills stay invisible for the rest of the session. + await AppRuntime.runPromise( + Effect.gen(function* () { + const config = yield* Config.Service + const skill = yield* Skill.Service + yield* config.invalidate() + yield* skill.refresh() + }), + ) + } + // altimate_change end + // altimate_change start (AI-7519) — first-answer latency instrumentation + // user-facing phase label. // @@ -112,12 +149,7 @@ export namespace SessionPrompt { // The trace span is a sibling of the root (tracing.ts:1009 assigns // parentSpanId to rootSpanId), not a nested child — good enough for // waterfall correlation via timestamps, and no schema change is required. - async function traceSpan( - name: string, - fn: () => Promise, - input?: unknown, - sessionID?: SessionID, - ): Promise { + async function traceSpan(name: string, fn: () => Promise, input?: unknown, sessionID?: SessionID): Promise { const startTime = Date.now() if (sessionID) void SessionStatus.publishPhase(sessionID, name, true) try { @@ -272,6 +304,76 @@ export namespace SessionPrompt { await SessionRevert.cleanup(session as unknown as Parameters[0]) // altimate_change end + // altimate_change start — make the bound workspace's custom skills visible + // before the agent is resolved. `createUserMessage` -> `Agent.get` -> + // `Skill.dirs()` is what first materialises the skill registry, so acting + // here lands the skills on this turn rather than the next one. + // + // The flag is read SYNCHRONOUSLY and the opt-out path is deliberately not + // awaited. An `await` here — even a zero-cost one — inserts an event-loop + // tick before `createUserMessage`, which reorders this turn against a + // forked `prompt.loop` fiber. That is not theoretical: it made + // "running subtask preserves metadata after tool-call transition" fail + // roughly two runs in three, while passing on main. A feature nobody + // enabled must not perturb the turn at all. + // + // Opting out still has to take effect, since discovery loads whatever is on + // disk without consulting the flag. The gate is a synchronous `existsSync`, + // not a detached cleanup: a run with the flag ON leaves a snapshot behind, + // and turning the flag off does not delete it, so a later opted-out turn + // CAN find one. Detaching the purge let `createUserMessage` materialise + // those stale skills first, which put `alwaysApply` instructions into a + // turn the operator had disabled the feature for. Awaiting only when a + // snapshot is actually there keeps the tick off the path that regressed — + // a user who never opted in has no directory, so this costs one `stat` and + // does not even load the sync module. + if (!CoreFlag.ALTIMATE_WORKSPACE) { + const dir = Instance.directory + // Mirrors `MANAGED_DIR` in ./altimate/workspace/skill-sync. Inlined + // rather than imported so the opted-out path stays free of that module. + if (existsSync(path.join(dir, ".altimate-code", "skill", "_workspace"))) { + try { + const m = await import("../altimate/workspace/skill-sync") + if ((await m.syncSkills(dir)).changed) await refreshSkillRegistry(dir) + } catch (err) { + log.warn("workspace skill opt-out cleanup failed", { err: String(err) }) + } + } + } else { + try { + const skillSync = await import("../altimate/workspace/skill-sync") + const dir = Instance.directory + const refreshRegistry = () => refreshSkillRegistry(dir) + + // A sync that ran elsewhere — a bind, most commonly — changes the + // snapshot with no instance context to refresh from. Pick that up before + // deciding whether this turn needs to poll at all. + await refreshRegistry() + + if (!(await skillSync.recentlySynced(dir))) { + const applied = skillSync.syncSkills(dir).then(refreshRegistry) + applied.catch((err) => log.warn("workspace skill sync failed", { err: String(err) })) + // Timer cleared when the sync wins the race: an armed timer keeps the + // event loop alive, so a short-lived `run` would linger for the rest + // of the bound, once per turn. + let timer: ReturnType | undefined + try { + await Promise.race([ + applied, + new Promise((r) => { + timer = setTimeout(r, WORKSPACE_SKILL_WAIT_MS) + }), + ]) + } finally { + if (timer) clearTimeout(timer) + } + } + } catch (err) { + log.warn("workspace skill sync failed", { err: String(err) }) + } + } + // altimate_change end + const message = await createUserMessage(input) await Session.touch(input.sessionID) @@ -435,12 +537,7 @@ export namespace SessionPrompt { let session: Awaited> let altCfg: Awaited> try { - session = await traceSpan( - "bootstrap.session-get", - () => Session.get(sessionID), - { sessionID }, - sessionID, - ) + session = await traceSpan("bootstrap.session-get", () => Session.get(sessionID), { sessionID }, sessionID) // altimate_change start - detect environment fingerprint at session start altCfg = await traceSpan("bootstrap.config-get", () => Config.get(), undefined, sessionID) if (altCfg.experimental?.env_fingerprint_skill_selection === true) { @@ -570,10 +667,12 @@ export namespace SessionPrompt { // into the next loop instead of terminating the session. const lastAssistantHasToolParts = lastAssistant !== undefined && - (msgs.find((msg) => msg.info.id === lastAssistant.id)?.parts.some((part) => { - if (part.type !== "tool") return false - return !(part.state.status === "error" && part.state.metadata?.interrupted === true) - }) ?? + (msgs + .find((msg) => msg.info.id === lastAssistant.id) + ?.parts.some((part) => { + if (part.type !== "tool") return false + return !(part.state.status === "error" && part.state.metadata?.interrupted === true) + }) ?? false) if ( lastAssistant?.finish && @@ -613,9 +712,7 @@ export namespace SessionPrompt { // TODO: centralize "invoke tool" logic if (task?.type === "subtask") { // altimate_change start — v1.17.9: TaskTool is an Effect of Info; init() yields the executable def - const taskTool = await AppRuntime.runPromise( - Effect.flatMap(TaskTool, (info) => info.init()), - ) + const taskTool = await AppRuntime.runPromise(Effect.flatMap(TaskTool, (info) => info.init())) // altimate_change end const taskModel = task.model ? await Provider.getModel(task.model.providerID, task.model.modelID) : model const assistantMessage = (await Session.updateMessage({ @@ -849,9 +946,7 @@ export namespace SessionPrompt { model, }) msgs = reminderResult.messages - const hoistedReminders = isAnthropicLikeModel(model) - ? [] - : reminderResult.trustedReminderParts.map((p) => p.text) + const hoistedReminders = isAnthropicLikeModel(model) ? [] : reminderResult.trustedReminderParts.map((p) => p.text) // altimate_change end // altimate_change start — plan refinement detection and telemetry @@ -1422,7 +1517,13 @@ export namespace SessionPrompt { // eslint-disable-next-line no-console console.error( "[altimate-validators] " + - JSON.stringify({ kind: "dispatch_enter", sessionID, step, cwd: vCtx.workingDirectory, sessionStartMs: vCtx.sessionStartMs }), + JSON.stringify({ + kind: "dispatch_enter", + sessionID, + step, + cwd: vCtx.workingDirectory, + sessionStartMs: vCtx.sessionStartMs, + }), ) } const checks = await ValidatorRegistry.runAll(vCtx) @@ -1525,7 +1626,12 @@ export namespace SessionPrompt { // eslint-disable-next-line no-console console.error( "[altimate-validators] " + - JSON.stringify({ kind: "dispatch_error", sessionID, step, error: e instanceof Error ? e.message : String(e) }), + JSON.stringify({ + kind: "dispatch_error", + sessionID, + step, + error: e instanceof Error ? e.message : String(e), + }), ) } } @@ -2884,7 +2990,6 @@ NOTE: At any point in time through this workflow you should feel free to ask the // altimate_change start — /mcps enable/disable: direct handler bypasses LLM if (input.command === "mcps") { - // Helper: build and persist an assistant reply for a command shortcut. async function respond( parentID: MessageID, @@ -2893,17 +2998,28 @@ NOTE: At any point in time through this workflow you should feel free to ask the ): Promise { const now = Date.now() const assistantMsg: MessageV2.Assistant = { - id: MessageID.ascending(), role: "assistant", sessionID: input.sessionID, - parentID, modelID: model.modelID, providerID: model.providerID, - mode: "builder", agent: "builder", + id: MessageID.ascending(), + role: "assistant", + sessionID: input.sessionID, + parentID, + modelID: model.modelID, + providerID: model.providerID, + mode: "builder", + agent: "builder", path: { cwd: Instance.directory, root: Instance.worktree }, - cost: 0, tokens: { total: 0, input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, - finish: "stop", time: { created: now, completed: now }, + cost: 0, + tokens: { total: 0, input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + finish: "stop", + time: { created: now, completed: now }, } await Session.updateMessage(assistantMsg) const textPart: MessageV2.TextPart = { - id: PartID.ascending(), sessionID: input.sessionID, messageID: assistantMsg.id, - type: "text", text: responseText, time: { start: now, end: now }, + id: PartID.ascending(), + sessionID: input.sessionID, + messageID: assistantMsg.id, + type: "text", + text: responseText, + time: { start: now, end: now }, } await Session.updatePart(textPart) AppRuntime.runPromise( @@ -2957,11 +3073,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the if (!cfg.mcp?.[name]) { const known = Object.keys(cfg.mcp ?? {}) const suffix = known.length ? ` Known servers: ${known.join(", ")}.` : "" - return respond( - userMsg.info.id, - `MCP server **${name}** not found in config.${suffix}`, - model, - ) + return respond(userMsg.info.id, `MCP server **${name}** not found in config.${suffix}`, model) } let responseText: string @@ -2974,7 +3086,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the responseText = `MCP server **${name}** enabled. Status: connected.` } else { const errSuffix = entry?.status === "failed" ? " — " + entry.error : "" - responseText = `Attempted to enable MCP server **${name}**. Status: ${entry?.status ?? "unknown"}${errSuffix}.` + responseText = `Attempted to enable MCP server **${name}**. Status: ${entry?.status ?? "unknown"}${errSuffix}.` } } else { await MCP.disconnect(name) diff --git a/packages/opencode/src/session/system.ts b/packages/opencode/src/session/system.ts index 8c55fb6c99..6e035616c5 100644 --- a/packages/opencode/src/session/system.ts +++ b/packages/opencode/src/session/system.ts @@ -149,7 +149,17 @@ export namespace SystemPrompt { for (const skill of autoLoaded) { parts.push("") parts.push(``) - parts.push(skill.content.trim()) + // altimate_change start — neutralise the closing tag inside the body. + // The name is escaped but the body was not, so content containing + // `` closed the wrapper and continued as unwrapped + // system-prompt text — able to impersonate the harness's own framing, + // directly after the prompt has told the model to treat this as binding + // guidance. Skill bodies are now remote content (a bound workspace + // syncs them), so this is reachable by anyone who can upload a skill. + // Deliberately not a full XML escape: bodies legitimately contain code + // and angle brackets, and mangling those would break working skills. + parts.push(neutralizeSkillWrapper(skill.content.trim())) + // altimate_change end parts.push(``) } parts.push("") @@ -194,6 +204,12 @@ export namespace SystemPrompt { .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F]/g, "") } + // altimate_change start — see the auto-loaded skill block below. + function neutralizeSkillWrapper(content: string): string { + return content.replace(/<(\/?)auto_loaded_skill\b/gi, "<$1auto_loaded_skill") + } + // altimate_change end + async function collectAutoLoadedSkills(list: Skill.Info[]): Promise { const out: Skill.Info[] = [] for (const skill of list) { diff --git a/packages/opencode/src/skill/index.ts b/packages/opencode/src/skill/index.ts index ac0a3925f7..053543bb85 100644 --- a/packages/opencode/src/skill/index.ts +++ b/packages/opencode/src/skill/index.ts @@ -119,6 +119,13 @@ export interface Interface { readonly all: () => Effect.Effect readonly dirs: () => Effect.Effect readonly available: (agent?: Agent.Info) => Effect.Effect + // altimate_change start — drop the per-instance discovery/registry caches so + // the next read re-scans disk. Skills can appear mid-session: a workspace + // bind, or a poll that finds new bundles, writes them under the project + // config dir, and both caches below are otherwise populated once per instance + // and never refreshed. + readonly refresh: () => Effect.Effect + // altimate_change end } const add = Effect.fnUntraced(function* (state: State, match: string, events: EventV2Bridge.Service["Service"]) { @@ -379,7 +386,17 @@ export const layer = Layer.effect( return list.filter((skill) => Permission.evaluate("skill", skill.name, agent.permission).action !== "deny") }) - return Service.of({ get, require, all, dirs, available }) + // altimate_change start — see Interface.refresh. `discovered` and `state` + // are separate InstanceStates and `state` closes over the discovery result, + // so invalidating only `discovered` would leave a stale registry. + const refresh = Effect.fn("Skill.refresh")(function* () { + yield* InstanceState.invalidate(discovered) + yield* InstanceState.invalidate(state) + }) + + // altimate_change: `refresh` added to the upstream service surface + return Service.of({ get, require, all, dirs, available, refresh }) + // altimate_change end }), ) @@ -444,6 +461,14 @@ export async function get(name: string) { export async function available(agent?: Agent.Info) { return runSkill((svc) => svc.available(agent)) } +// Imperative wrapper for the same reason as the three above: the workspace +// skill sync is plain async code running under the instance ALS, which +// `attach()` propagates into this runtime. No marker of its own — this is +// already inside the block opened above, and nesting them makes marker +// coverage harder to account for. (bot review) +export async function refresh() { + return runSkill((svc) => svc.refresh()) +} // altimate_change end export * as Skill from "." diff --git a/packages/opencode/test/altimate/plugin/workspace.test.ts b/packages/opencode/test/altimate/plugin/workspace.test.ts index cf8ed145fe..f545c3e0d5 100644 --- a/packages/opencode/test/altimate/plugin/workspace.test.ts +++ b/packages/opencode/test/altimate/plugin/workspace.test.ts @@ -233,8 +233,11 @@ describe("workspace binding cache", () => { // (harness-bot #1116 comment 3840503346 hardened that gate.) let memPostSerial = 0 globalThis.fetch = (async (_input?: unknown, _init?: unknown) => { - calls++ const url = String(_input) + // Count memory traffic only. Skills re-sync on every bind by design, and + // a cached binding is revalidated against the server — neither is the + // memory seed this test is about. + if (!url.includes("/skills") && !url.includes("/datamate-project-bindings/by-")) calls++ if (url.includes("/datamates/memory/") && !url.includes("/list")) { memPostSerial += 1 return new Response( @@ -250,6 +253,19 @@ describe("workspace binding cache", () => { headers: { "Content-Type": "application/json" }, }) } + // Answer binding lookups in the shape `lookupBinding` actually parses. + // Falling through to the `{datamates:[…]}` body below classified every + // revalidation as "unknown", which is neither memoized nor stamped — so + // the counter filter above was hiding a lookup that could never succeed + // and a fresh round trip on every bind. (bot review) + if (url.includes("/datamate-project-bindings/by-")) { + return new Response( + JSON.stringify({ + binding: { datamate_id: 9, datamate_name: "Warm", repo_remote: null, project_path: proj }, + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ) + } return new Response(JSON.stringify({ datamates: [{ id: 9, name: "Warm", memory_enabled: true }] }), { status: 200, headers: { "Content-Type": "application/json" }, @@ -275,6 +291,61 @@ describe("workspace binding cache", () => { } }) + test("a warm bind still syncs skills even though the memory seed is skipped", async () => { + // The ``alreadySeeded`` marker is memory's one-shot gate. Skills have a + // different lifecycle — the workspace's bundles can change at any time — so + // the skill pull sits above that early return. Without it, every bind after + // the first would silently stop refreshing skills. + const ORIGINAL_FLAG = process.env.ALTIMATE_WORKSPACE + process.env.ALTIMATE_WORKSPACE = "1" + const proj = path.join(SANDBOX, "warm-skills-proj") + mkdirSync(proj, { recursive: true }) + const binding = { + datamateId: 11, + datamateName: "WarmSkills", + repoRemote: null, + projectPath: proj, + linkedAt: 1, + } + + let skillListCalls = 0 + const originalFetch = globalThis.fetch + globalThis.fetch = (async (_input?: unknown) => { + const url = String(_input) + if (url.includes("/skills")) { + skillListCalls++ + return new Response(JSON.stringify({ items: [], total: 0, page: 1, size: 50, pages: 1 }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + } + if (url.includes("/datamates/memory/") && !url.includes("/list")) { + return new Response(JSON.stringify({ result: { results: [{ id: "m1", event: "ADD" }] } }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + } + return new Response(JSON.stringify({ datamates: [{ id: 11, name: "WarmSkills", memory_enabled: true }] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + }) as typeof fetch + + try { + await recordApprovedBinding(proj, binding, { awaitBackfill: true }) + const afterFirst = skillListCalls + expect(afterFirst).toBeGreaterThan(0) + + // Same workspace, same project: memory will skip, skills must not. + await recordApprovedBinding(proj, { ...binding, linkedAt: 2 }, { awaitBackfill: true }) + expect(skillListCalls).toBeGreaterThan(afterFirst) + } finally { + globalThis.fetch = originalFetch + if (ORIGINAL_FLAG === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_FLAG + } + }) + test("a seed that never ran stays retryable on the next warm", async () => { // Memory disabled at bind time means the sweep is a no-op, not a completed // seed. Treating it as done left the blocks this machine already holds diff --git a/packages/opencode/test/altimate/workspace/memory-sync.test.ts b/packages/opencode/test/altimate/workspace/memory-sync.test.ts index 774366449c..e823ff6fd9 100644 --- a/packages/opencode/test/altimate/workspace/memory-sync.test.ts +++ b/packages/opencode/test/altimate/workspace/memory-sync.test.ts @@ -87,6 +87,20 @@ function stubFetch() { return new Response(JSON.stringify({ detail: "boom" }), { status: 500 }) } const payload = (() => { + // Server-side binding lookup, used when no local cache entry exists. + if (url.includes("/datamate-project-bindings/by-")) { + if (!serverBinding) return { detail: "not found" } + return { + binding: { + id: 5, + datamate_id: serverBinding.datamateId, + datamate_name: serverBinding.datamateName, + repo_remote: serverBinding.repoRemote, + project_path: serverBinding.projectPath, + }, + datamate: { id: serverBinding.datamateId, name: serverBinding.datamateName }, + } + } if (url.includes("/datamates/memory/list")) return listResponse if (url.includes("/datamates/memory/")) { // A created record becomes visible to later reads, as it would on the @@ -103,8 +117,10 @@ function stubFetch() { if (url.includes("/datamates/")) return { datamates: workspaces } return { message: "ok" } })() + const status = + url.includes("/datamate-project-bindings/by-") && !serverBinding ? 404 : 200 return new Response(JSON.stringify(payload), { - status: 200, + status, headers: { "Content-Type": "application/json" }, }) }) as typeof fetch @@ -128,6 +144,10 @@ function block(over: Partial = {}): any { } } +/** When set, the stubbed server reports this project as bound. Null means the + * lookup 404s, exactly as an unbound remote does. */ +let serverBinding: typeof BINDING | null = null + const BINDING = { datamateId: 42, datamateName: "acme", @@ -142,6 +162,7 @@ beforeEach(() => { listFails = false createResult = [{ id: "mem-new" }] workspaces = [{ id: 42, name: "acme", memory_enabled: true }] + serverBinding = null stubCreds("acme", "https://api.example.com") stubFetch() resetOverlay() @@ -153,6 +174,10 @@ beforeEach(() => { afterEach(() => { globalThis.fetch = originalFetch + // Reset the server-binding fixture too: tests that delete the resolveBinding + // seam fall through to the real lookup, so a leaked value from a previous + // test would decide their outcome. + serverBinding = null ;(AltimateApi as unknown as { isConfigured: typeof originalIsConfigured }).isConfigured = originalIsConfigured ;(AltimateApi as unknown as { getCredentials: typeof originalGetCreds }).getCredentials = @@ -582,6 +607,7 @@ describe("memory_enabled", () => { expect(callsTo("/datamates/memory/", "POST").length).toBe(0) workspaces = [{ id: 42, name: "acme", memory_enabled: true }] + serverBinding = null captured = [] await mirrorBlock(block({ id: "after-enable" })) expect(callsTo("/datamates/memory/", "POST").length).toBe(1) @@ -1080,3 +1106,48 @@ describe("whenHydrated", () => { expect(elapsed).toBeLessThan(2_000) }) }) + +// ── binding resolution ────────────────────────────────────────────────────── +describe("binding resolution for the mirror", () => { + test("mirrors from a directory bound only on the server", async () => { + // The local cache is written only by an explicit link, so a directory + // holding a repo that IS bound — a git worktree, a second clone, a + // teammate's checkout — has no entry. Reading only that cache made the + // mirror a silent no-op in every one of those. + // + // This encodes a DECISION, so it is worth stating plainly: adopting a + // server-side binding does enable the ongoing memory mirror, which POSTs + // blocks to the workspace. Only the one-shot backfill of memory this + // machine already held stays behind an explicit link, via `seededAt`. + // The reasoning is that a worktree of a linked repo is the same project by + // the same user, and a mirror that silently does nothing there is the bug + // being fixed. Flip this test if that trade is ever reversed. + delete syncInternals.resolveBinding + serverBinding = BINDING + const dir = path.join(SANDBOX, "server-bound-proj") + mkdirSync(dir, { recursive: true }) + + await mirrorBlock(block(), dir) + + const posts = captured.filter( + (c) => c.method === "POST" && c.url.includes("/datamates/memory/"), + ) + expect(posts.length).toBe(1) + }) + + test("an unbound directory still mirrors nothing", async () => { + // The fallback must not invent a binding: a 404 means unbound, and an + // unbound directory has no workspace to attribute a memory to. + delete syncInternals.resolveBinding + serverBinding = null + const dir = path.join(SANDBOX, "genuinely-unbound-proj") + mkdirSync(dir, { recursive: true }) + + await mirrorBlock(block(), dir) + + const posts = captured.filter( + (c) => c.method === "POST" && c.url.includes("/datamates/memory/"), + ) + expect(posts.length).toBe(0) + }) +}) diff --git a/packages/opencode/test/altimate/workspace/skill-sync.test.ts b/packages/opencode/test/altimate/workspace/skill-sync.test.ts new file mode 100644 index 0000000000..3082ea450f --- /dev/null +++ b/packages/opencode/test/altimate/workspace/skill-sync.test.ts @@ -0,0 +1,1169 @@ +// altimate_change - new file +// Unit coverage for the workspace skill mirror +// (src/altimate/workspace/skill-sync.ts). +// +// Network is stubbed at globalThis.fetch so assertions are about what actually +// reaches disk after a given server response. The cases that matter most are +// the destructive ones: a failed or malformed list must NEVER delete a user's +// synced skills, and a rebind must never leave the previous workspace's skills +// where discovery can load them. +import { afterAll, afterEach, beforeEach, describe, expect, test } from "bun:test" +import { + existsSync, + mkdirSync, + utimesSync, + readdirSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs" +import path from "node:path" +import os from "node:os" +import matter from "gray-matter" + +const ORIGINAL_XDG_STATE_HOME = process.env.XDG_STATE_HOME +const ORIGINAL_TEST_HOME = process.env.OPENCODE_TEST_HOME +const ORIGINAL_WORKSPACE_FLAG = process.env.ALTIMATE_WORKSPACE +const SANDBOX = path.join(os.tmpdir(), `altimate-skillsync-${process.pid}-${Date.now()}`) +mkdirSync(path.join(SANDBOX, "state"), { recursive: true }) +mkdirSync(path.join(SANDBOX, "home", ".altimate"), { recursive: true }) +process.env.XDG_STATE_HOME = path.join(SANDBOX, "state") +process.env.OPENCODE_TEST_HOME = path.join(SANDBOX, "home") + +const API_URL = "https://api.example.test" +const TENANT = "acme" + +// Real credentials file, so the module resolves them through the same path it +// uses in production rather than a stubbed export. +writeFileSync( + path.join(SANDBOX, "home", ".altimate", "altimate.json"), + JSON.stringify({ + altimateUrl: API_URL, + altimateInstanceName: TENANT, + altimateApiKey: "test-key", + }), +) + +const { syncSkills, recentlySynced, registryStale, markRegistryApplied, flushPendingSyncs } = + await import("@/altimate/workspace/skill-sync") +const { cachePath, recordApprovedBinding } = await import("@/altimate/workspace/state") + +const MANAGED = path.join(".altimate-code", "skill", "_workspace") +const ORIGINAL_FETCH = globalThis.fetch + +let project: string + +/** Write a real binding cache entry, so ``readLocalBinding`` is exercised for + * real instead of being replaced. */ +function bindTo(datamateId: number) { + writeFileSync( + cachePath(), + JSON.stringify({ + version: 1, + tenant: TENANT, + apiUrl: API_URL, + bindings: { + [project]: { + datamateId, + datamateName: `ws-${datamateId}`, + repoRemote: null, + projectPath: project, + linkedAt: Date.now(), + }, + }, + }), + ) +} + +beforeEach(() => { + // Only the workspace flag is scoped per test. It matters because leaving it + // set makes OTHER files' prompt path attempt a real sync against this + // sandbox's credentials, which cost 15s timeouts. + // + // XDG_STATE_HOME / OPENCODE_TEST_HOME are deliberately NOT scoped this way, + // despite the same argument applying in principle. Flipping them per test is + // worse: a file sharing this bun worker sets its own values at module load, + // and restoring "the original" here deletes theirs mid-run. Tried it — seven + // tests in onboarding/materialize.test.ts began materializing into the real + // home directory. Module-scope + afterAll is the lesser of the two evils + // until test files stop sharing a process. + process.env.ALTIMATE_WORKSPACE = "1" + project = path.join(SANDBOX, `proj-${Math.random().toString(36).slice(2)}`) + mkdirSync(project, { recursive: true }) + bindTo(1) +}) + +afterEach(() => { + globalThis.fetch = ORIGINAL_FETCH + if (ORIGINAL_WORKSPACE_FLAG === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_WORKSPACE_FLAG +}) + +afterAll(() => { + if (ORIGINAL_XDG_STATE_HOME === undefined) delete process.env.XDG_STATE_HOME + else process.env.XDG_STATE_HOME = ORIGINAL_XDG_STATE_HOME + if (ORIGINAL_TEST_HOME === undefined) delete process.env.OPENCODE_TEST_HOME + else process.env.OPENCODE_TEST_HOME = ORIGINAL_TEST_HOME + if (ORIGINAL_WORKSPACE_FLAG === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_WORKSPACE_FLAG + try { + rmSync(SANDBOX, { recursive: true, force: true }) + } catch { + /* best effort */ + } +}) + +/** Serve the real contract: a paginated summary page, a detail view carrying + * files[{path,size}], and raw file bytes. */ +function serve(skills: Record>, updatedAt = "2026-01-01T00:00:00Z") { + // Shapes verified against a local backend on `development`: the list is NOT + // wrapped, the detail IS wrapped in `{skill: ...}`, and the file endpoint + // answers `{path, content}` JSON rather than raw bytes. + const items = Object.keys(skills).map((id) => ({ + public_id: id, + name: id, + file_count: Object.keys(skills[id]).length, + updated_at: updatedAt, + })) + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + const files = url.match(/skills\/([^/]+)\/files\/(.+)$/) + if (files) { + const id = decodeURIComponent(files[1]) + const rel = files[2].split("/").map(decodeURIComponent).join("/") + return json({ path: rel, content: skills[id][rel] }) + } + const detail = url.match(/skills\/([^/?]+)(?:\?|$)/) + if (detail && !url.includes("datamate_id")) { + const id = decodeURIComponent(detail[1]) + return json({ + skill: { + public_id: id, + files: Object.entries(skills[id]).map(([p, c]) => ({ + path: p, + size: Buffer.from(c).byteLength, + })), + content: skills[id]["SKILL.md"] ?? "", + }, + }) + } + return json({ items, total: items.length, page: 1, size: 50, pages: 1 }) + }) as unknown as typeof fetch +} + +function json(body: unknown) { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) +} + +/** Remove the local binding, leaving the project bound only server-side. */ +function unbind() { + writeFileSync( + cachePath(), + JSON.stringify({ version: 1, tenant: TENANT, apiUrl: API_URL, bindings: {} }), + ) +} + +/** Like `serve`, but the project is unbound locally and the server answers the + * binding lookup — the fresh-clone shape. */ +function serveWithServerBinding(skills: Record>) { + serve(skills) + const inner = globalThis.fetch + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = String(input) + if (url.includes("/datamate-project-bindings/by-")) { + return json({ + binding: { + id: 7, + datamate_id: 1, + datamate_name: "ws-1", + repo_remote: null, + project_path: project, + }, + datamate: { id: 1, name: "ws-1" }, + }) + } + return inner(input as never, init as never) + }) as unknown as typeof fetch +} + +function skillFile(id: string, rel: string) { + return path.join(project, MANAGED, id, rel) +} + +describe("workspace skill sync", () => { + test("writes the bundle, references included", async () => { + serve({ + "pub-1": { + "SKILL.md": "---\nname: acme\n---\nbody", + "references/guide.md": "reference body", + }, + }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + expect(readFileSync(skillFile("pub-1", "references/guide.md"), "utf8")).toBe("reference body") + }) + + test("a failed list leaves existing skills untouched", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + globalThis.fetch = (async () => { + throw new Error("offline") + }) as unknown as typeof fetch + await syncSkills(project) + + // The whole point: a network failure must not read as "no skills". + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a malformed 200 is treated as an error, not an empty workspace", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + globalThis.fetch = (async () => + new Response(JSON.stringify({ unexpected: "envelope" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + })) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a genuinely empty workspace removes the snapshot", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + serve({}) + await syncSkills(project) + expect(existsSync(path.join(project, MANAGED))).toBe(false) + }) + + test("rebinding to another workspace drops the previous snapshot", async () => { + serve({ "pub-1": { "SKILL.md": "from workspace 1" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + // Rebind, then fail the pull. The old workspace's skills must be gone — + // discovery does not read the manifest, so leaving them would feed the + // model another workspace's guidance. + bindTo(2) + globalThis.fetch = (async () => { + throw new Error("offline") + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(false) + }) + + test("a truncated download publishes nothing and keeps the previous snapshot", async () => { + serve({ "pub-1": { "SKILL.md": "good" } }) + await syncSkills(project) + + // The API exposes no checksum, so byte length is the only integrity check. + // A short body must abandon the whole snapshot rather than publish half a + // skill. + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) return json({ path: "SKILL.md", content: "short" }) + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "pub-2", name: "p2", file_count: 1, updated_at: "2026-02-02T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ skill: { public_id: "pub-2", files: [{ path: "SKILL.md", size: 9999 }], content: "" } }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-2", "SKILL.md"))).toBe(false) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("an unchanged workspace issues no detail or file requests on the second run", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + let detailOrFile = 0 + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + // Binding revalidation is not a detail or file fetch; this test is about + // not re-downloading an unchanged workspace. + if (url.includes("/datamate-project-bindings/by-")) return json({ detail: "not found" }) + if (url.includes("/files/") || !url.includes("datamate_id")) detailOrFile++ + return json({ + items: [{ public_id: "pub-1", name: "p1", file_count: 1, updated_at: "2026-01-01T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(detailOrFile).toBe(0) + }) + + test("never writes outside the managed directory", async () => { + writeFileSync(path.join(project, "user-file.txt"), "mine") + mkdirSync(path.join(project, ".altimate-code", "skill", "hand-written"), { recursive: true }) + writeFileSync( + path.join(project, ".altimate-code", "skill", "hand-written", "SKILL.md"), + "hand written", + ) + + serve({ "pub-1": { "SKILL.md": "synced" } }) + await syncSkills(project) + serve({}) + await syncSkills(project) + + expect(readFileSync(path.join(project, "user-file.txt"), "utf8")).toBe("mine") + expect( + readFileSync(path.join(project, ".altimate-code", "skill", "hand-written", "SKILL.md"), "utf8"), + ).toBe("hand written") + }) + + test("path traversal in a file entry is refused", async () => { + // The body is served at exactly the advertised size, so the size check + // cannot be what stops this — only the path guard can. + const escape = "escaped" + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) return json({ path: "../escape.md", content: escape }) + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "pub-1", name: "p1", file_count: 1, updated_at: "2026-01-01T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ + skill: { + public_id: "pub-1", + files: [{ path: "../escape.md", size: Buffer.from(escape).byteLength }], + content: "", + }, + }) + }) as unknown as typeof fetch + await syncSkills(project) + + // Nothing is published at all: an unrecognised inventory aborts the sync. + expect(existsSync(path.join(project, MANAGED))).toBe(false) + // And specifically not one level up from where the skill would have gone. + expect(existsSync(path.join(project, ".altimate-code", "skill", "escape.md"))).toBe(false) + expect(existsSync(path.join(project, MANAGED, "escape.md"))).toBe(false) + }) + + test("`changed` is true only when disk actually changed", async () => { + // `changed` is the gate on refreshing the skill registry, which costs a full + // config reread plus a re-scan. Reporting it on a no-op sync would put that + // on every turn; failing to report it on a real change would leave the model + // looking at the previous snapshot. + serve({ "pub-1": { "SKILL.md": "one" } }) + expect((await syncSkills(project)).changed).toBe(true) + + // Same content, same updated_at: nothing to do. + expect((await syncSkills(project)).changed).toBe(false) + + // A newer updated_at is a real change. + serve({ "pub-1": { "SKILL.md": "two" } }, "2026-03-03T00:00:00Z") + expect((await syncSkills(project)).changed).toBe(true) + }) + + test("a file body without content is an error, not an empty file", async () => { + // The size check alone does not cover this: a bundle may legitimately hold + // a zero-byte file, and a malformed body coerced to "" would match size 0 + // and publish silently, advancing the manifest as though it had succeeded. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) return json({ path: "SKILL.md" }) + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "pub-3", name: "p3", file_count: 1, updated_at: "2026-04-04T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ skill: { public_id: "pub-3", files: [{ path: "SKILL.md", size: 0 }], content: "" } }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-3", "SKILL.md"))).toBe(false) + // And the previous snapshot is intact — an error never publishes. + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a project bound only on the server still gets its skills", async () => { + // The local cache is written only by an explicit link. Without the server + // fallback a fresh clone of a linked repo gets no skills at all, and `link` + // refuses to help because the server reports it as already linked. + unbind() + serveWithServerBinding({ "pub-1": { "SKILL.md": "from the server binding" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + // And the discovered binding is cached, so the next process skips the lookup. + const cached = JSON.parse(readFileSync(cachePath(), "utf8")) + expect(cached.bindings[realpathSync(project)].datamateId).toBe(1) + }) + + test("a failed binding lookup is not read as unbound", async () => { + // Same rule as the skill list: an error means "unknown", so whatever is on + // disk stays. Treating it as unbound would wipe a synced project offline. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + unbind() + globalThis.fetch = (async () => { + throw new Error("offline") + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + // And the failure must stay retryable: a network blip must not memoize this + // project as unbound for the rest of the process. + serveWithServerBinding({ "pub-2": { "SKILL.md": "after recovery" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-2", "SKILL.md"))).toBe(true) + }) + + test("recentlySynced rate-limits the per-message poll", async () => { + // The caller on the per-message path skips the network while this is true. + // If it never went true, every turn would pay an HTTP round trip; if it + // never went false, a skill added in the SaaS would never reach an open + // session. + expect(await recentlySynced(project)).toBe(false) + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(await recentlySynced(project)).toBe(true) + + // Scoped per project — a different directory is still due a check. + expect(await recentlySynced(path.join(SANDBOX, "some-other-proj"))).toBe(false) + }) + + test("a skill added later is picked up by a subsequent sync", async () => { + // The SaaS-adds-a-skill case: the same project, already synced, gains a + // second skill upstream. A later sync must report changed and land it. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + serve( + { "pub-1": { "SKILL.md": "one" }, "pub-9": { "SKILL.md": "added in the saas" } }, + "2026-05-05T00:00:00Z", + ) + const { changed } = await syncSkills(project) + expect(changed).toBe(true) + expect(existsSync(skillFile("pub-9", "SKILL.md"))).toBe(true) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a public_id that is not a single path component is refused", async () => { + // `public_id` is server-generated but still remote input spliced into a + // filesystem path, one component ABOVE the per-file guard — so the file + // guard cannot catch an escape here. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) return json({ path: "SKILL.md", content: "x" }) + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "../../escape", name: "e", file_count: 1, updated_at: "2026-06-06T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ skill: { public_id: "../../escape", files: [{ path: "SKILL.md", size: 1 }], content: "" } }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(path.join(project, ".altimate-code", "escape"))).toBe(false) + expect(existsSync(path.join(project, "escape"))).toBe(false) + // The previous snapshot is untouched: an unusable id is an error, not empty. + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("refuses to replace a managed directory it did not create", async () => { + // The directory name is ours by convention, and convention is not + // ownership. Anything already there without our manifest is a user's file. + const managed = path.join(project, MANAGED) + mkdirSync(path.join(managed, "hand-rolled"), { recursive: true }) + writeFileSync(path.join(managed, "hand-rolled", "SKILL.md"), "mine, not synced") + + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + expect(readFileSync(path.join(managed, "hand-rolled", "SKILL.md"), "utf8")).toBe("mine, not synced") + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(false) + }) + + test("staging never lands where skill discovery scans", async () => { + // Discovery globs `{skill,skills}/**/SKILL.md` under the config dir, so a + // staging tree beside `_workspace` would be scanned and a half-written + // snapshot loaded as real skills. Observed DURING the sync: staging is + // removed on success, so checking afterwards proves nothing. + const seen: string[][] = [] + serve({ "pub-1": { "SKILL.md": "one", "references/g.md": "ref" } }) + const inner = globalThis.fetch + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + if (String(input).includes("/files/")) { + try { + seen.push(readdirSync(path.join(project, ".altimate-code", "skill"))) + } catch { + seen.push([]) + } + } + return inner(input as never, init as never) + }) as unknown as typeof fetch + + await syncSkills(project) + + expect(seen.length).toBeGreaterThan(0) + for (const entries of seen) { + expect(entries.filter((e) => e !== "_workspace")).toEqual([]) + } + }) + + test("a damaged snapshot is repaired rather than declared up to date", async () => { + serve({ "pub-1": { "SKILL.md": "one", "references/g.md": "ref" } }) + await syncSkills(project) + rmSync(skillFile("pub-1", "references/g.md")) + + // Same updated_at: only checking the manifest would call this current. + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "references/g.md"))).toBe(true) + }) + + test("a failed sync does not consume the poll window", async () => { + globalThis.fetch = (async () => { + throw new Error("offline") + }) as unknown as typeof fetch + await syncSkills(project) + // Stamping here would suppress the retry for a full interval on a blip. + expect(await recentlySynced(project)).toBe(false) + }) + + test("registryStale reports a snapshot the caller has not applied yet", async () => { + // A bind syncs with no instance context to refresh from; the next turn has + // to notice on its own, without re-fetching. + expect(registryStale(project)).toBe(false) + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(registryStale(project)).toBe(true) + markRegistryApplied(project) + expect(registryStale(project)).toBe(false) + }) + + test("registryStale follows the snapshot on disk, not an in-process stamp", async () => { + // The bind and the turn that must refresh do not share memory — the runtime + // loads this module once per thread, so each has its own module record and + // its own `globalThis`. An in-process "changed" stamp is therefore invisible + // to the thread serving the next turn, which is what kept a workspace linked + // mid-session from ever reaching the agent. Simulating that here: the + // manifest moves WITHOUT this module having run a sync, and staleness must + // still be reported. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + markRegistryApplied(project) + expect(registryStale(project)).toBe(false) + + const manifest = path.join(project, MANAGED, ".manifest.json") + const later = new Date(Date.now() + 5000) + utimesSync(manifest, later, later) + + expect(registryStale(project)).toBe(true) + }) + + test("registryStale reports a purge, so opting out refreshes too", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + markRegistryApplied(project) + expect(registryStale(project)).toBe(false) + + // A deactivate removes the whole managed tree, manifest included. That is a + // registry change in the other direction and must refresh just the same. + rmSync(path.join(project, MANAGED), { recursive: true, force: true }) + expect(registryStale(project)).toBe(true) + }) + + test("flushPendingSyncs waits for a sync a short-lived process would abandon", async () => { + // `run` exits as soon as its turn ends, which is routinely sooner than a + // cold sync finishes. Without this the staged tree was dropped on exit and, + // since nothing had been persisted, the next `run` started cold and lost the + // same race — the project never got its skills at all. + serve({ "pub-1": { "SKILL.md": "one" } }) + const inner = globalThis.fetch + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + await new Promise((r) => setTimeout(r, 40)) + return inner(input as never, init as never) + }) as unknown as typeof fetch + + const running = syncSkills(project) + // Still in flight: this is what a process exiting here would have thrown + // away, and it is what makes the assertion after the flush meaningful. + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(false) + + await flushPendingSyncs() + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + await running + }) + + test("the published snapshot ignores itself in git", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(readFileSync(path.join(project, MANAGED, ".gitignore"), "utf8")).toBe("*\n") + }) + + test("a file response for the wrong path is refused", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + // Same length, different file — undetectable without checking the path. + if (url.includes("/files/")) return json({ path: "OTHER.md", content: "abc" }) + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "pub-7", name: "p7", file_count: 1, updated_at: "2026-07-07T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ skill: { public_id: "pub-7", files: [{ path: "SKILL.md", size: 3 }], content: "" } }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-7", "SKILL.md"))).toBe(false) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("an inconsistent empty page is an error, not an empty workspace", async () => { + // "Empty workspace" is the one answer that deletes the snapshot, so a page + // claiming rows exist while returning none must not be believed. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + globalThis.fetch = (async () => + json({ items: [], total: 4, page: 1, size: 50, pages: 1 })) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a bundle beyond the client limit is refused whole", async () => { + // Counted on the ADVERTISED inventory, before anything is downloaded, so + // an oversized workspace fails fast instead of being read into memory. + // Uses file count rather than bytes so the ceiling is what trips — an + // oversized `size` would be caught by the integrity check instead, and the + // test would pass without the ceiling existing. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + const many = Array.from({ length: 2500 }, (_, i) => ({ path: `f${i}.md`, size: 1 })) + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) { + const rel = url.split("/files/")[1] + return json({ path: decodeURIComponent(rel), content: "x" }) + } + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "pub-many", name: "m", file_count: many.length, updated_at: "2026-08-08T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ skill: { public_id: "pub-many", files: many, content: "" } }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-many", "f0.md"))).toBe(false) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a synced bundle has the shape a skill needs", async () => { + // Shape only. Most fixtures here assert bytes reached disk, which does not + // show a bundle yields a USABLE skill — but neither does this: discovery is + // not run here, because this file has no instance harness. The end-to-end + // claim is made where it can be: "a workspace-synced bundle layout is + // discovered as a real skill" in test/skill/skill.test.ts. + serve({ + "pub-real": { + "SKILL.md": "---\nname: synced-probe\ndescription: A synced workspace skill.\n---\n\nBody.\n", + "references/guide.md": "reference body", + }, + }) + await syncSkills(project) + + const onDisk = readFileSync(skillFile("pub-real", "SKILL.md"), "utf8") + const parsed = matter(onDisk) + expect(parsed.data.name).toBe("synced-probe") + expect(parsed.data.description).toBe("A synced workspace skill.") + // The bundled reference has to survive too — the model is handed the skill's + // directory and reads these itself. + expect(readFileSync(skillFile("pub-real", "references/guide.md"), "utf8")).toBe("reference body") + // And it must sit where discovery globs `{skill,skills}/**/SKILL.md`. + expect(skillFile("pub-real", "SKILL.md")).toContain(path.join(".altimate-code", "skill")) + }) + + test("disconnecting the account takes the snapshot out of service", async () => { + // Leaving it is not neutral: discovery loads whatever is on disk without + // consulting the manifest, so a disconnected user keeps getting the + // workspace's skills — and an `alwaysApply` one keeps entering every prompt. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + const credsFile = path.join(SANDBOX, "home", ".altimate", "altimate.json") + const saved = readFileSync(credsFile, "utf8") + rmSync(credsFile) + try { + const { changed } = await syncSkills(project) + expect(changed).toBe(true) + expect(existsSync(path.join(project, MANAGED))).toBe(false) + } finally { + writeFileSync(credsFile, saved) + } + }) + + test("turning the workspace flag off takes the snapshot out of service", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + process.env.ALTIMATE_WORKSPACE = "0" + try { + await syncSkills(project) + expect(existsSync(path.join(project, MANAGED))).toBe(false) + } finally { + process.env.ALTIMATE_WORKSPACE = "1" + } + }) + + test("the opt-out purge serialises with a sync already in flight", async () => { + // Both paths write the same tree. With the purge outside the in-flight gate + // an enabled run — already past its own flag check — could republish + // `_workspace` moments after a disabled run deleted it, leaving a snapshot + // on disk for a feature that is off. + serve({ "pub-1": { "SKILL.md": "one" } }) + + const enabled = syncSkills(project) + process.env.ALTIMATE_WORKSPACE = "0" + try { + // Joins the in-flight enabled run rather than deleting underneath it, so + // both observers agree and the tree is not left half-published. + const [first, second] = await Promise.all([enabled, syncSkills(project)]) + expect(second).toEqual(first) + } finally { + process.env.ALTIMATE_WORKSPACE = "1" + } + + // The purge still runs once nothing is in flight. + process.env.ALTIMATE_WORKSPACE = "0" + try { + await syncSkills(project) + expect(existsSync(path.join(project, MANAGED))).toBe(false) + } finally { + process.env.ALTIMATE_WORKSPACE = "1" + } + }) + + test("an unreadable credentials file keeps the snapshot", async () => { + // Unknown is not disconnected. A corrupt or unreadable file must not + // destroy a snapshot the user is still entitled to. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + const credsFile = path.join(SANDBOX, "home", ".altimate", "altimate.json") + const saved = readFileSync(credsFile, "utf8") + writeFileSync(credsFile, "{ not json") + try { + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + } finally { + writeFileSync(credsFile, saved) + } + }) + + test("pages are walked, and a partial listing never prunes the rest", async () => { + // The reviewers all named this: nothing constructed a 2-page listing, so + // MAX_PAGES, the terminator, the echoed page and cross-page accumulation + // were unexercised. + const bodies: Record = { + "p1-a": "---\nname: p1a\ndescription: d.\n---\nA\n", + "p2-b": "---\nname: p2b\ndescription: d.\n---\nB\n", + } + const row = (id: string) => ({ + public_id: id, + name: id, + file_count: 1, + updated_at: "2026-09-09T00:00:00Z", + }) + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) { + const id = /skills\/([^/]+)\/files/.exec(url)![1] + return json({ path: "SKILL.md", content: bodies[id] }) + } + if (url.includes("datamate_id")) { + const page = Number(/page=(\d+)/.exec(url)?.[1] ?? 1) + return json({ + items: [row(page === 1 ? "p1-a" : "p2-b")], + total: 2, + page, + size: 1, + pages: 2, + }) + } + const id = /skills\/([^/?]+)/.exec(url)![1] + return json({ + skill: { + public_id: id, + files: [{ path: "SKILL.md", size: Buffer.from(bodies[id]).byteLength }], + content: "", + }, + }) + }) as unknown as typeof fetch + + await syncSkills(project) + expect(existsSync(skillFile("p1-a", "SKILL.md"))).toBe(true) + expect(existsSync(skillFile("p2-b", "SKILL.md"))).toBe(true) + }) + + test("a listing with an unusable `pages` is an error, not a one-page workspace", async () => { + // Defaulting `pages` to 1 turns a partial first page into "the whole + // workspace" and deletes everything the later pages held. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + for (const bad of [undefined, 0, 1.5, "2"]) { + globalThis.fetch = (async () => + json({ items: [], total: 0, page: 1, size: 50, pages: bad })) as unknown as typeof fetch + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + } + }) + + test("a file response omitting `path` is refused", async () => { + // The mis-routed response this guard exists for is exactly the case where + // the echoed field may be missing, so "checked when present" is no check. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + globalThis.fetch = (async (input: string | URL) => { + const url = String(input) + if (url.includes("/files/")) return json({ content: "abc" }) + if (url.includes("datamate_id")) + return json({ + items: [{ public_id: "pub-np", name: "n", file_count: 1, updated_at: "2026-09-10T00:00:00Z" }], + total: 1, + page: 1, + size: 50, + pages: 1, + }) + return json({ skill: { public_id: "pub-np", files: [{ path: "SKILL.md", size: 3 }], content: "" } }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-np", "SKILL.md"))).toBe(false) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a plain file at the managed path is never deleted", async () => { + // `readdir` throws ENOTDIR here, which the old guard read as "absent, + // therefore ours" and handed straight to fs.rm. + const managed = path.join(project, MANAGED) + mkdirSync(path.dirname(managed), { recursive: true }) + writeFileSync(managed, "a user's file, not a directory") + + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + expect(readFileSync(managed, "utf8")).toBe("a user's file, not a directory") + }) + + test("a symlinked staging directory is refused rather than traversed", async () => { + // `readdir` follows symlinks, so sweeping through one would recursively + // delete whatever it points at — outside the project. + const outside = path.join(SANDBOX, `outside-${Math.random().toString(36).slice(2)}`) + mkdirSync(outside, { recursive: true }) + writeFileSync(path.join(outside, "precious.txt"), "must survive") + + mkdirSync(path.join(project, ".altimate-code", "skill"), { recursive: true }) + symlinkSync(outside, path.join(project, ".altimate-code", "skill-staging")) + + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + expect(readFileSync(path.join(outside, "precious.txt"), "utf8")).toBe("must survive") + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(false) + }) + + test("switching to an account with no binding drops the old tenant's skills", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + // New account: the cached binding no longer matches, and the server has no + // binding for this project either. + const credsFile = path.join(SANDBOX, "home", ".altimate", "altimate.json") + const saved = readFileSync(credsFile, "utf8") + writeFileSync( + credsFile, + JSON.stringify({ altimateUrl: API_URL, altimateInstanceName: "other-tenant", altimateApiKey: "k" }), + ) + globalThis.fetch = (async () => json({ detail: "not found" })) as unknown as typeof fetch + try { + await syncSkills(project) + expect(existsSync(path.join(project, MANAGED))).toBe(false) + } finally { + writeFileSync(credsFile, saved) + } + }) + + test("a directory holding a manifest we cannot read is not ours", async () => { + // Ownership was decided on the FILENAME `.manifest.json`. A directory with + // an unrelated or corrupt file of that name is someone else's, and was + // being deleted wholesale. + const managed = path.join(project, MANAGED) + mkdirSync(path.join(managed, "someone-elses"), { recursive: true }) + writeFileSync(path.join(managed, "someone-elses", "SKILL.md"), "not ours") + writeFileSync(path.join(managed, ".manifest.json"), "{ not valid json") + + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + expect(readFileSync(path.join(managed, "someone-elses", "SKILL.md"), "utf8")).toBe("not ours") + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(false) + }) + + test("a manifest from a foreign shape does not confer ownership", async () => { + const managed = path.join(project, MANAGED) + mkdirSync(path.join(managed, "other-tool"), { recursive: true }) + writeFileSync(path.join(managed, "other-tool", "SKILL.md"), "another tool's file") + // Valid JSON, wrong shape — `readManifest` must reject it. + writeFileSync(path.join(managed, ".manifest.json"), JSON.stringify({ some: "other tool" })) + + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + expect(readFileSync(path.join(managed, "other-tool", "SKILL.md"), "utf8")).toBe("another tool's file") + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(false) + }) + + test("a confirmed unbind takes the snapshot out of service", async () => { + // Discovery does not consult the manifest, so a project detached in the + // SaaS would keep serving that workspace's skills forever. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + + unbind() + // 404 on the binding lookup: confirmed unbound, not a failure. + globalThis.fetch = (async (input: string | URL) => { + if (String(input).includes("/datamate-project-bindings/by-")) { + return new Response(JSON.stringify({ detail: "not found" }), { status: 404 }) + } + return json({ items: [], total: 0, page: 1, size: 50, pages: 1 }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(path.join(project, MANAGED))).toBe(false) + }) + + test("an unreachable binding lookup keeps the snapshot", async () => { + // Unknown is not unbound. Deleting on a network blip would wipe a snapshot + // the user is still entitled to. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + unbind() + globalThis.fetch = (async () => { + throw new Error("offline") + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a malformed binding response is unknown, not a crash", async () => { + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + unbind() + globalThis.fetch = (async (input: string | URL) => { + if (String(input).includes("/datamate-project-bindings/by-")) { + return json({ binding: { nonsense: true } }) + } + return json({ items: [], total: 0, page: 1, size: 50, pages: 1 }) + }) as unknown as typeof fetch + await syncSkills(project) + + // Treated as unknown: nothing published, nothing destroyed. + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("the disabled-path purge refuses to follow a symlink", async () => { + // The opt-out branch deletes, and it runs before the check inside the sync. + // The link target must hold a tree the purge WOULD delete, or the test + // passes for the wrong reason (nothing there to remove). + const outside = path.join(SANDBOX, `optout-${Math.random().toString(36).slice(2)}`) + const victim = path.join(outside, "skill", "_workspace") + mkdirSync(path.join(victim, "pub-x"), { recursive: true }) + writeFileSync(path.join(victim, "pub-x", "SKILL.md"), "must survive") + writeFileSync( + path.join(victim, ".manifest.json"), + JSON.stringify({ version: 1, tenant: TENANT, apiUrl: API_URL, datamateId: 1, skills: {} }), + ) + + const proj2 = path.join(SANDBOX, `symlinked-${Math.random().toString(36).slice(2)}`) + mkdirSync(proj2, { recursive: true }) + symlinkSync(outside, path.join(proj2, ".altimate-code")) + + process.env.ALTIMATE_WORKSPACE = "0" + try { + await syncSkills(proj2) + expect(readFileSync(path.join(victim, "pub-x", "SKILL.md"), "utf8")).toBe("must survive") + } finally { + process.env.ALTIMATE_WORKSPACE = "1" + } + }) + + test("the no-credentials purge refuses to follow a symlink", async () => { + // Same hazard as the opt-out purge: this branch runs before the + // `pathsAreReal` check inside the sync, so it needs its own guard or a + // symlinked `.altimate-code` has the delete resolve through the link. The + // target must hold a tree the purge WOULD remove, or this passes for the + // wrong reason. + const outside = path.join(SANDBOX, `nocreds-${Math.random().toString(36).slice(2)}`) + const victim = path.join(outside, "skill", "_workspace") + mkdirSync(path.join(victim, "pub-x"), { recursive: true }) + writeFileSync(path.join(victim, "pub-x", "SKILL.md"), "must survive") + writeFileSync( + path.join(victim, ".manifest.json"), + JSON.stringify({ version: 1, tenant: TENANT, apiUrl: API_URL, datamateId: 1, skills: {} }), + ) + + const proj2 = path.join(SANDBOX, `symlinked-nocreds-${Math.random().toString(36).slice(2)}`) + mkdirSync(proj2, { recursive: true }) + symlinkSync(outside, path.join(proj2, ".altimate-code")) + + const credsFile = path.join(SANDBOX, "home", ".altimate", "altimate.json") + const saved = readFileSync(credsFile, "utf8") + rmSync(credsFile, { force: true }) + try { + await syncSkills(proj2) + expect(readFileSync(path.join(victim, "pub-x", "SKILL.md"), "utf8")).toBe("must survive") + } finally { + writeFileSync(credsFile, saved) + } + }) + + test("a cached binding the server no longer recognises is not retained", async () => { + // The cache is written by an explicit link and otherwise never expires, so + // without revalidation a project detached in the SaaS keeps serving its old + // workspace's skills on this machine forever. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + // The local binding is still on disk — this is NOT the unbound-cache case. + expect(JSON.parse(readFileSync(cachePath(), "utf8")).bindings[realpathSync(project)]).toBeDefined() + + globalThis.fetch = (async (input: string | URL) => { + if (String(input).includes("/datamate-project-bindings/by-")) { + return new Response(JSON.stringify({ detail: "not found" }), { status: 404 }) + } + return json({ items: [], total: 0, page: 1, size: 50, pages: 1 }) + }) as unknown as typeof fetch + await syncSkills(project) + + expect(existsSync(path.join(project, MANAGED))).toBe(false) + // And the stale row is gone, so a later read cannot resurrect it. + expect(JSON.parse(readFileSync(cachePath(), "utf8")).bindings[realpathSync(project)]).toBeUndefined() + }) + + test("linking just after an unbound turn is not undone by the negative cache", async () => { + // A turn taken while the project is unlinked memoizes "no binding here" for + // MISS_TTL_MS. If linking inside that window reads the memo as an + // authoritative answer, revalidation deletes the row the link just wrote and + // the link silently does nothing — the user links and gets no skills. + rmSync(cachePath(), { force: true }) + globalThis.fetch = (async (input: string | URL) => { + if (String(input).includes("/datamate-project-bindings/by-")) { + return new Response(JSON.stringify({ detail: "not found" }), { status: 404 }) + } + return json({ items: [], total: 0, page: 1, size: 50, pages: 1 }) + }) as unknown as typeof fetch + await syncSkills(project) + + // The link. Deliberately still inside the miss window. + await recordApprovedBinding(project, { + datamateId: 7, + datamateName: "ws-7", + repoRemote: null, + projectPath: project, + linkedAt: Date.now(), + }) + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + expect(JSON.parse(readFileSync(cachePath(), "utf8")).bindings[realpathSync(project)]).toBeDefined() + expect(existsSync(skillFile("pub-1", "SKILL.md"))).toBe(true) + }) + + test("a cached binding survives a server that cannot be reached", async () => { + // Revalidation must not tear down a working setup over a network blip. + serve({ "pub-1": { "SKILL.md": "one" } }) + await syncSkills(project) + + // Workspace unchanged; only the binding lookup is unreachable. Serving an + // empty list here instead would delete the snapshot for a different and + // entirely correct reason, proving nothing about revalidation. + serve({ "pub-1": { "SKILL.md": "one" } }) + const inner = globalThis.fetch + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + if (String(input).includes("/datamate-project-bindings/by-")) 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(JSON.parse(readFileSync(cachePath(), "utf8")).bindings[realpathSync(project)]).toBeDefined() + }) + + test("does nothing when the workspace flag is off", async () => { + process.env.ALTIMATE_WORKSPACE = "0" + let calls = 0 + globalThis.fetch = (async () => { + calls++ + return new Response("[]", { status: 200 }) + }) as unknown as typeof fetch + try { + await syncSkills(project) + expect(calls).toBe(0) + } finally { + process.env.ALTIMATE_WORKSPACE = "1" + } + }) +}) diff --git a/packages/opencode/test/skill/skill.test.ts b/packages/opencode/test/skill/skill.test.ts index fd79a68cee..bdacc10472 100644 --- a/packages/opencode/test/skill/skill.test.ts +++ b/packages/opencode/test/skill/skill.test.ts @@ -568,4 +568,78 @@ description: A skill in the .opencode/skills directory. { git: true }, ), ) + + // altimate_change start — coverage for Skill.refresh, which lets a workspace + // sync make newly written skill bundles visible without restarting. The + // registry is cached per instance across two separate InstanceStates + // (`discovered` and `state`); dropping only one leaves a stale read, so this + // case fails unless refresh drops both. + it.live("refresh picks up a skill added after the registry was first read", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const write = (name: string) => + Effect.promise(() => + Bun.write( + path.join(dir, ".opencode", "skill", name, "SKILL.md"), + `---\nname: ${name}\ndescription: Skill ${name}.\n---\n\nBody.\n`, + ), + ) + + // Written before the first read so the config directory already exists + // and this case is about the skill cache alone, not config discovery. + yield* write("refresh-a") + + const skill = yield* Skill.Service + const first = (yield* skill.all()).map((s) => s.name) + expect(first).toContain("refresh-a") + expect(first).not.toContain("refresh-b") + + yield* write("refresh-b") + + // Still invisible: proves the cache under test is real, so the + // assertion after refresh cannot pass by accident. + expect((yield* skill.all()).map((s) => s.name)).not.toContain("refresh-b") + + yield* skill.refresh() + + const after = (yield* skill.all()).map((s) => s.name) + expect(after).toContain("refresh-b") + expect(after).toContain("refresh-a") + }), + { git: true }, + ), + ) + // altimate_change end + + // altimate_change start — the sync side asserts bytes on disk; this asserts + // the thing that actually matters, that discovery loads such a bundle as a + // usable skill. Written here because this file has the instance harness. + it.live("a workspace-synced bundle layout is discovered as a real skill", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + // Exactly what skill-sync writes: `.altimate-code/skill/_workspace//`. + const base = path.join(dir, ".altimate-code", "skill", "_workspace", "pub-abc123") + yield* Effect.promise(() => + Bun.write( + path.join(base, "SKILL.md"), + `---\nname: workspace-synced\ndescription: Synced from a bound workspace.\n---\n\nBody.\n`, + ), + ) + yield* Effect.promise(() => Bun.write(path.join(base, "references", "guide.md"), "ref")) + // The ignore file the sync stages alongside must not upset discovery. + yield* Effect.promise(() => Bun.write(path.join(base, "..", ".gitignore"), "*\n")) + + const skill = yield* Skill.Service + const found = (yield* skill.all()).find((s) => s.name === "workspace-synced") + expect(found).toBeDefined() + expect(found!.description).toBe("Synced from a bound workspace.") + expect(found!.content).toContain("Body.") + expect(found!.location).toContain("_workspace") + }), + { git: true }, + ), + ) + // altimate_change end })