From 2f6abc7b8e731d32ef61b6ebb3a20c8e15cef3eb Mon Sep 17 00:00:00 2001 From: Sarav Date: Fri, 25 Sep 2026 10:45:15 +0530 Subject: [PATCH] feat(workspace): publish a project skill to the workspace over `serve` HTTP - `GET /altimate/skill/publishable` and `POST /altimate/skill/publish` - IDE-delivered skills and parent-dir snapshots count as workspace-owned - a synced copy never shadows the user's own project skill - harden create-then-rebind against stray background lookups Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/docs/configure/skills.md | 2 +- .../opencode/src/altimate/telemetry/index.ts | 3 +- .../src/altimate/workspace/publishable.ts | 54 ++++ .../src/altimate/workspace/skill-publish.ts | 19 +- .../src/altimate/workspace/snapshot-path.ts | 37 +++ packages/opencode/src/server/server.ts | 213 +++++++++++++-- packages/opencode/src/skill/index.ts | 45 ++- .../altimate/workspace/publishable.test.ts | 147 ++++++++++ .../altimate/workspace/skill-publish.test.ts | 35 +++ .../altimate-skill-publish-routes.test.ts | 256 ++++++++++++++++++ 10 files changed, 780 insertions(+), 31 deletions(-) create mode 100644 packages/opencode/src/altimate/workspace/publishable.ts create mode 100644 packages/opencode/src/altimate/workspace/snapshot-path.ts create mode 100644 packages/opencode/test/altimate/workspace/publishable.test.ts create mode 100644 packages/opencode/test/server/altimate-skill-publish-routes.test.ts diff --git a/docs/docs/configure/skills.md b/docs/docs/configure/skills.md index c3736aa9c7..f1c8315c02 100644 --- a/docs/docs/configure/skills.md +++ b/docs/docs/configure/skills.md @@ -197,7 +197,7 @@ altimate-code skill remove my-tool # remove skill + paired tool altimate-code skill publish my-tool # upload every file in the skill directory; re-run to update ``` -`skill publish` sends the whole skill directory, not just `SKILL.md`, so keep secrets out of it. A built-in filter skips known file and directory names — `.env*`, `.git`, `id_rsa`, `*.pem`, `*.key`, `*.p12`, `.npmrc`/`.netrc`, `credentials.json`, `secrets.*`, `.ssh`/`.aws`, editor swap files — but it matches names only and never scans file contents, so a token inside `config.yaml` or a key named `server.crt` would still be uploaded. Built-in skills, global skills and skills the workspace itself sent you cannot be published. +`skill publish` sends the whole skill directory, not just `SKILL.md`, so keep secrets out of it. A built-in filter skips known file and directory names — `.env*`, `.git`, `id_rsa`, `*.pem`, `*.key`, `*.p12`, `.npmrc`/`.netrc`, `credentials.json`, `secrets.*`, `.ssh`/`.aws`, editor swap files — but it matches names only and never scans file contents, so a token inside `config.yaml` or a key named `server.crt` would still be uploaded. Built-in skills, global skills and skills the workspace itself sent you cannot be published — including the copies the VS Code / Cursor extension delivers into `.claude/skills/` and `.agents/skills/`, which carry an `.altimate-managed.json` marker. ### TUI diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index 6f09e3de16..52d695e548 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -730,7 +730,8 @@ export namespace Telemetry { skill_name: string action: "created" | "updated" file_count: number - source: "cli" | "tui" + // "serve": published from the IDE extension over the serve route + source: "cli" | "tui" | "serve" } // altimate_change end // altimate_change start — plan refinement telemetry event diff --git a/packages/opencode/src/altimate/workspace/publishable.ts b/packages/opencode/src/altimate/workspace/publishable.ts new file mode 100644 index 0000000000..2043145a83 --- /dev/null +++ b/packages/opencode/src/altimate/workspace/publishable.ts @@ -0,0 +1,54 @@ +// altimate_change - new file +// +// Which of the skills this project can reach may be published to its workspace, for the serve +// routes the IDE extension calls. The CLI's `skill publish` and the TUI's "Publish to workspace" +// row apply the same rules inline (`skillSource`, `isManagedSkill`, `assertProjectSkill`); the +// refusal wording here matches theirs so a user moving between surfaces reads the same thing. +import path from "path" +import { skillSource } from "@/cli/cmd/skill-helpers" +import { assertProjectSkill, IDE_DELIVERED_MARKER, isManagedSkill } from "./skill-publish" + +export type PublishEligibility = "publishable" | "builtin" | "personal" | "workspace" | "outside-project" + +/** The boundary a skill must lie within: the worktree, since discovery walks up to it — except for + * a project with no git, whose worktree is the sentinel `/`, which would contain everything. */ +export function projectRootFor(directory: string, worktree: string): string { + return worktree !== "/" ? worktree : directory +} + +/** Whether a skill at `location` (its `SKILL.md`) may be published from `projectDirectory`. */ +export function publishEligibility(location: string, projectDirectory: string, projectRoot: string): PublishEligibility { + if (!path.isAbsolute(location) || skillSource(location) === "builtin") return "builtin" + if (skillSource(location) === "global") return "personal" + const skillDirectory = path.dirname(location) + if (isManagedSkill(projectDirectory, skillDirectory)) return "workspace" + try { + assertProjectSkill(projectRoot, skillDirectory) + } catch { + return "outside-project" + } + return "publishable" +} + +/** Why a skill cannot be published, in the words the CLI uses, or null when it can. */ +export function explainIneligible(name: string, location: string, eligibility: PublishEligibility): string | null { + switch (eligibility) { + case "publishable": + return null + case "builtin": + return `"${name}" is a built-in skill and cannot be published.` + case "personal": + return ( + `"${name}" is a personal skill (${path.dirname(location)}), not one of this project's. ` + + `Copy it into the project's skills directory to publish it.` + ) + case "workspace": + return ( + `"${name}" is a skill this workspace sent to you, not one you authored. ` + + `Publishing it would send the workspace's own skill back to it. ` + + `If you copied it to make your own, rename it and delete any ${IDE_DELIVERED_MARKER} in its folder.` + ) + case "outside-project": + return `"${name}" is not inside this project (${path.dirname(location)}), so it cannot be published from here.` + } +} diff --git a/packages/opencode/src/altimate/workspace/skill-publish.ts b/packages/opencode/src/altimate/workspace/skill-publish.ts index 363a81d3ba..626e5ac62f 100644 --- a/packages/opencode/src/altimate/workspace/skill-publish.ts +++ b/packages/opencode/src/altimate/workspace/skill-publish.ts @@ -33,11 +33,12 @@ // interpret. import fs from "fs/promises" import path from "path" -import { realpathSync } from "fs" +import { existsSync, realpathSync } from "fs" import { Log } from "@/altimate/util/log" import { Global } from "@/global" import { Filesystem } from "@/util/filesystem" import { AltimateApi } from "@/altimate/api/client" +import { isInWorkspaceSnapshot } from "./snapshot-path" import { ConflictError, ForbiddenError, NotFoundError, WorkspaceApi, altimateRequest } from "./api-client" import { resolveBinding } from "./state" @@ -49,6 +50,13 @@ const SKILLS_BASE = "/skills" * process-global store — into every caller that only wants to publish. */ const MANAGED_DIR = path.join(".altimate-code", "skill", "_workspace") +/** Written by the VS Code / Cursor extension into every skill directory it delivers from the + * workspace (`.claude/skills/altimate-*`, `.agents/skills/altimate-*`). Those roots are also this + * project's own skill-discovery roots, so without the marker a delivered skill would be offered for + * publish — and uploaded back to the workspace that sent it, as a new skill owned by the publisher. + * The name is the extension's (`OWNERSHIP_MARKER` in its `customSkillDelivery.ts`); keep in step. */ +export const IDE_DELIVERED_MARKER = ".altimate-managed.json" + /** Mirrors the server's own ceilings so an oversized bundle fails locally, with a * usable message, instead of after a long upload. `MAX_BUNDLE_FILES` and * `MAX_BUNDLE_BYTES` in `app/service/custom_skills/bundle.py`; a mismatch @@ -355,7 +363,8 @@ export async function collectBundle(dir: string): Promise { return files } -/** True when this path lives inside the workspace-owned snapshot. */ +/** True when this skill came from the workspace: it lives inside the workspace-owned snapshot, or + * the IDE extension delivered it (see {@link IDE_DELIVERED_MARKER}). */ export function isManagedSkill(projectDirectory: string, skillDirectory: string): boolean { // `path.resolve` is lexical: it normalises `..` and makes the path absolute, // but it does not follow links. A skill directory that IS a symlink into the @@ -374,7 +383,11 @@ export function isManagedSkill(projectDirectory: string, skillDirectory: string) } const managed = real(path.resolve(projectDirectory, MANAGED_DIR)) const candidate = real(skillDirectory) - return candidate === managed || candidate.startsWith(managed + path.sep) + if (candidate === managed || candidate.startsWith(managed + path.sep)) return true + // A snapshot above the project directory: discovery reads config directories up to the worktree. + if (isInWorkspaceSnapshot(candidate)) return true + // A delivered skill sits in the project's own discovery roots, not under the snapshot. + return existsSync(path.join(candidate, IDE_DELIVERED_MARKER)) } // --------------------------------------------------------------------------- diff --git a/packages/opencode/src/altimate/workspace/snapshot-path.ts b/packages/opencode/src/altimate/workspace/snapshot-path.ts new file mode 100644 index 0000000000..3571aebaf1 --- /dev/null +++ b/packages/opencode/src/altimate/workspace/snapshot-path.ts @@ -0,0 +1,37 @@ +// altimate_change - new file +// +// Whether a path lies inside a workspace skill snapshot (`.altimate-code/skill/_workspace`), judged +// by path segments rather than against one project directory: discovery walks config directories up +// to the worktree, so a session started in `repo/sub` also reads `repo/.altimate-code/...`. +// Dependency-free on purpose — skill discovery imports it, and the workspace modules are heavy. +import path from "path" + +const SNAPSHOT_SEGMENTS = [".altimate-code", "skill", "_workspace"] + +export function isInWorkspaceSnapshot(location: string): boolean { + const parts = path.resolve(location).split(path.sep) + for (let i = 0; i + SNAPSHOT_SEGMENTS.length <= parts.length; i++) { + if (SNAPSHOT_SEGMENTS.every((segment, j) => parts[i + j] === segment)) return true + } + return false +} + +/** Whether `location` lies inside `root`, by path segments. */ +export function isWithin(root: string, location: string): boolean { + const rel = path.relative(path.resolve(root), path.resolve(location)) + return rel === "" || (!rel.startsWith(".." + path.sep) && rel !== ".." && !path.isAbsolute(rel)) +} + +/** Whether a skill found in the workspace snapshot must yield to a same-name skill already + * registered at `existingLocation`: only when that one is the user's own, inside the project. + * Built-in (`builtin:` / ``), personal and snapshot entries are overridden as before. */ +export function snapshotCopyYields(match: string, existingLocation: unknown, projectRoot: string | undefined): boolean { + return ( + !!projectRoot && + typeof existingLocation === "string" && + isInWorkspaceSnapshot(match) && + path.isAbsolute(existingLocation) && + !isInWorkspaceSnapshot(existingLocation) && + isWithin(projectRoot, existingLocation) + ) +} diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index e2d070474a..db59b3d4d2 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -89,10 +89,18 @@ export namespace Server { origin: string | undefined, host: string | undefined, password: string | undefined = Flag.OPENCODE_SERVER_PASSWORD, + fetchSite?: string, ): { status: 403 | 409; body: { ok: false; error: string } } | undefined { if (!CoreFlag.ALTIMATE_WORKSPACE) { return { status: 409, body: { ok: false, error: "Workspace mode is not enabled for this server." } } } + // A browser labels every request it sends, including Origin-less ones such as an `` GET + // from another site. Native clients send no such header, so only a browser's cross-site request + // is refused here; the Origin rules below handle the rest. + if (fetchSite && fetchSite !== "same-origin" && fetchSite !== "none") { + log.warn("refused cross-site workspace action", { fetchSite }) + return { status: 403, body: { ok: false, error: "Workspace actions cannot be run from another site." } } + } if (!origin) return undefined if (!password) { log.warn("refused browser-originated workspace action on an unsecured server", { origin }) @@ -112,6 +120,59 @@ export namespace Server { } return undefined } + /** The skill registry this instance serves, reloaded so a skill written since it loaded is found + * — also one in a skills directory that did not exist at boot. Same in-context path as + * `refreshSkillRegistry` in session/prompt.ts: the facade's invalidate keeps the stale root list + * `Config.directories()` cached. The CLI's `skill publish` reads this registry too. + * + * Not free: `Config.Service.invalidate()` drops the config cache for every instance and the + * refresh re-pulls any `skills.urls`. Acceptable for a user-triggered command, and the only + * invalidation the service offers. A config file mid-edit (invalid) fails this call — and would + * fail the session's next config read the same way. */ + async function reloadSkills() { + const [{ Effect }, { Config }, { Skill: Registry }] = await Promise.all([ + import("effect"), + import("../config/config"), + import("../skill"), + ]) + await AppRuntime.runPromise( + Effect.gen(function* () { + const config = yield* Config.Service + const skill = yield* Registry.Service + yield* config.invalidate() + yield* skill.refresh() + }), + ) + return Registry + } + + /** A request body that must be absent or a JSON object: the parsed object, or the response + * to send instead. A failed read keeps the routes' `{ ok: false, error }` 500 contract. */ + async function readJsonObject( + read: () => Promise, + route: string, + ): Promise<{ body: Record } | { failure: { status: 400 | 500; body: { ok: false; error: string } } }> { + let text: string + try { + text = await read() + } catch (err) { + const error = err instanceof Error ? err.message : String(err) + log.error(`${route}: could not read the request body`, { error }) + return { failure: { status: 500, body: { ok: false, error } } } + } + if (!text.trim()) return { body: {} } + let body: unknown + try { + body = JSON.parse(text) + } catch { + return { failure: { status: 400, body: { ok: false, error: "Request body is not valid JSON." } } } + } + if (body === null || typeof body !== "object" || Array.isArray(body)) { + return { failure: { status: 400, body: { ok: false, error: "Request body must be a JSON object." } } } + } + return { body: body as Record } + } + export function sameOrigin(origin: string, host: string | undefined): boolean { try { return !!host && new URL(origin).host === host @@ -1003,31 +1064,20 @@ export namespace Server { // headless and cannot reach the TUI slash command. Both act on the request's instance // directory and return the `Manage` report as is; wording is the caller's job. .post("/altimate/workspace/refresh", async (c) => { - const refused = workspaceRouteRefusal(c.req.header("origin"), c.req.header("host")) + const refused = workspaceRouteRefusal( + c.req.header("origin"), + c.req.header("host"), + undefined, + c.req.header("sec-fetch-site"), + ) if (refused) return c.json(refused.body, refused.status) // An absent or empty body is a session-less refresh; anything else must be well formed. // Falling back to "no session" on bad input would silently widen the operation to // resetting every session's memory overlay. - let text: string - try { - text = await c.req.text() - } catch (err) { - const error = err instanceof Error ? err.message : String(err) - log.error("workspace refresh: could not read the request body", { error }) - return c.json({ ok: false, error }, 500) - } - let body: unknown = {} - if (text.trim()) { - try { - body = JSON.parse(text) - } catch { - return c.json({ ok: false, error: "Request body is not valid JSON." }, 400) - } - } - if (body === null || typeof body !== "object" || Array.isArray(body)) { - return c.json({ ok: false, error: "Request body must be a JSON object." }, 400) - } - const raw = (body as Record).sessionID + const read = await readJsonObject(() => c.req.text(), "workspace refresh") + if ("failure" in read) return c.json(read.failure.body, read.failure.status) + const body = read.body + const raw = body.sessionID if (raw !== undefined && (typeof raw !== "string" || !raw)) { return c.json({ ok: false, error: "sessionID must be a non-empty string." }, 400) } @@ -1067,7 +1117,12 @@ export namespace Server { } }) .post("/altimate/workspace/sync", async (c) => { - const refused = workspaceRouteRefusal(c.req.header("origin"), c.req.header("host")) + const refused = workspaceRouteRefusal( + c.req.header("origin"), + c.req.header("host"), + undefined, + c.req.header("sec-fetch-site"), + ) if (refused) return c.json(refused.body, refused.status) try { const Manage = await import("../altimate/workspace/manage") @@ -1080,6 +1135,120 @@ export namespace Server { } }) // altimate_change end + // altimate_change start — GET /altimate/skill/publishable, POST /altimate/skill/publish + // The CLI's `skill publish ` for the IDE extension. Only `serve` holds the extension's + // pin, so publishing here targets the workspace selected in the panel — through the same + // engine, ledger and error wording as the CLI and TUI. + .get("/altimate/skill/publishable", async (c) => { + const refused = workspaceRouteRefusal( + c.req.header("origin"), + c.req.header("host"), + undefined, + c.req.header("sec-fetch-site"), + ) + if (refused) return c.json(refused.body, refused.status) + try { + const { projectRootFor, publishEligibility } = await import("../altimate/workspace/publishable") + const registry = await reloadSkills() + const root = projectRootFor(Instance.directory, Instance.worktree) + const skills = (await registry.all()) + .filter((skill) => publishEligibility(skill.location, Instance.directory, root) === "publishable") + // `description` is optional in frontmatter; sent as "" so every entry has the same shape. + .map((skill) => ({ name: skill.name, description: skill.description ?? "", location: skill.location })) + .sort((a, b) => a.name.localeCompare(b.name)) + return c.json({ ok: true as const, skills }) + } catch (err) { + const error = err instanceof Error ? err.message : String(err) + log.error("skill publishable: failed", { error }) + return c.json({ ok: false, error }, 500) + } + }) + .post("/altimate/skill/publish", async (c) => { + const refused = workspaceRouteRefusal( + c.req.header("origin"), + c.req.header("host"), + undefined, + c.req.header("sec-fetch-site"), + ) + if (refused) return c.json(refused.body, refused.status) + const read = await readJsonObject(() => c.req.text(), "skill publish") + if ("failure" in read) return c.json(read.failure.body, read.failure.status) + const name = read.body.name + if (typeof name !== "string" || !name.trim()) { + return c.json({ ok: false, error: "name must be a non-empty string." }, 400) + } + try { + const { explainIneligible, projectRootFor, publishEligibility } = await import( + "../altimate/workspace/publishable" + ) + const { describePublish, explainPublishError, publishSkill } = await import( + "../altimate/workspace/skill-publish" + ) + const registry = await reloadSkills() + const skill = await registry.get(name.trim()) + if (!skill) { + // Lookup is exact; a name that differs only in case is almost always what was meant. + const wanted = name.trim().toLowerCase() + const near = (await registry.all()).find((s) => s.name.toLowerCase() === wanted) + return c.json( + { + ok: false, + error: `Skill "${name.trim()}" not found in this project.${near ? ` Did you mean "${near.name}"?` : ""}`, + }, + 404, + ) + } + const root = projectRootFor(Instance.directory, Instance.worktree) + const ineligible = explainIneligible( + skill.name, + skill.location, + publishEligibility(skill.location, Instance.directory, root), + ) + if (ineligible) { + log.info("skill publish: refused", { skill: skill.name, reason: ineligible }) + return c.json({ ok: false, error: ineligible }, 422) + } + try { + const report = await publishSkill({ + projectDirectory: Instance.directory, + projectRoot: root, + skillDirectory: nodePath.dirname(skill.location), + name: skill.name, + description: skill.description ?? "", + }) + try { + const { Telemetry } = await import("../altimate/telemetry") + Telemetry.track({ + type: "skill_published", + timestamp: Date.now(), + session_id: Telemetry.getContext().sessionId || "", + skill_name: skill.name, + action: report.action, + file_count: report.files, + source: "serve", + }) + } catch {} + return c.json({ ok: true as const, report, message: describePublish(report) }) + } catch (err) { + // A refusal the engine raised on purpose already says what to do; 422, since 409 is + // the pilot gate's. Anything else is a failure, reported as the engine gave it. + // A backend conflict the engine did not classify is still a refusal, not a crash: + // its detail is the server's own explanation. + const { ConflictError } = await import("../altimate/workspace/api-client") + const known = explainPublishError(err) ?? (err instanceof ConflictError ? err.message : null) + if (known) { + log.info("skill publish: refused by the engine", { skill: skill.name, reason: known }) + return c.json({ ok: false, error: known }, 422) + } + throw err + } + } catch (err) { + const error = err instanceof Error ? err.message : String(err) + log.error("skill publish: failed", { error }) + return c.json({ ok: false, error }, 500) + } + }) + // altimate_change end .all("/*", async (c) => { const path = c.req.path diff --git a/packages/opencode/src/skill/index.ts b/packages/opencode/src/skill/index.ts index 369ec5673b..44957247b5 100644 --- a/packages/opencode/src/skill/index.ts +++ b/packages/opencode/src/skill/index.ts @@ -2,6 +2,9 @@ import { LayerNode } from "@opencode-ai/core/effect/layer-node" // altimate_change start — makeRuntime for the restored Promise wrapper (see bottom of file) import { makeRuntime } from "@/effect/run-service" // altimate_change end +// altimate_change start — workspace snapshot precedence in `add` +import { snapshotCopyYields } from "@/altimate/workspace/snapshot-path" +// altimate_change end import path from "path" import { pathToFileURL } from "url" import { Effect, Layer, Context, Schema } from "effect" @@ -128,7 +131,14 @@ export interface Interface { // altimate_change end } -const add = Effect.fnUntraced(function* (state: State, match: string, events: EventV2Bridge.Service["Service"]) { +// altimate_change start — `add` takes the project boundary for the workspace-snapshot precedence rule +const add = Effect.fnUntraced(function* ( + state: State, + match: string, + events: EventV2Bridge.Service["Service"], + projectRoot?: string, +) { + // altimate_change end const md = yield* Effect.tryPromise({ try: () => ConfigMarkdown.parse(match), catch: (err) => err, @@ -149,6 +159,25 @@ const add = Effect.fnUntraced(function* (state: State, match: string, events: Ev if (!isSkillFrontmatter(md.data)) return if (state.skills[md.data.name]) { + // altimate_change start — a workspace-synced copy never shadows a project skill of the same name. + // Matches are added concurrently, so which duplicate lands last is not fixed; for a skill the + // user wrote in the project (commonly `.claude/skills`) and then published, the workspace's + // copy of it could win after the next sync — the agent read the stale copy, and publishing an + // update was refused as "a skill this workspace sent to you". This makes that pair + // order-independent: the project skill wins. Only a skill inside the project is protected; + // built-in, personal and configured-path skills are still overridden by the workspace's. + // Own entries only: `state.skills` is a plain object, so a skill named `constructor` would + // otherwise find `Object.prototype.constructor` here. + const existing = Object.hasOwn(state.skills, md.data.name) ? state.skills[md.data.name] : undefined + if (existing && snapshotCopyYields(match, existing.location, projectRoot)) { + yield* Effect.logWarning("workspace skill shadowed by a project skill of the same name", { + name: md.data.name, + project: existing.location, + workspace: match, + }) + return + } + // altimate_change end yield* Effect.logWarning("duplicate skill name", { name: md.data.name, existing: state.skills[md.data.name].location, @@ -263,12 +292,15 @@ const discoverSkills = Effect.fnUntraced(function* ( } }) +// altimate_change start — `loadSkills` passes the project boundary through to `add` const loadSkills = Effect.fnUntraced(function* ( state: State, discovered: DiscoveryState, events: EventV2Bridge.Service["Service"], + projectRoot?: string, ) { - yield* Effect.forEach(discovered.matches, (match) => add(state, match, events), { + yield* Effect.forEach(discovered.matches, (match) => add(state, match, events, projectRoot), { + // altimate_change end concurrency: "unbounded", discard: true, }) @@ -302,7 +334,9 @@ export const layer = Layer.effect( }), ) const state = yield* InstanceState.make( - Effect.fn("Skill.state")(function* () { + // altimate_change start — the state factory reads the instance context for the project boundary + Effect.fn("Skill.state")(function* (ctx) { + // altimate_change end const s: State = { skills: {}, dirs: new Set() } // Register the built-in skill BEFORE disk discovery so a user-disk // skill with the same name can override it. @@ -353,7 +387,10 @@ export const layer = Layer.effect( } } // altimate_change end - yield* loadSkills(s, yield* InstanceState.get(discovered), events) + // altimate_change start — the worktree bounds the project, except without git (the `/` sentinel) + const projectRoot = ctx.worktree !== "/" ? ctx.worktree : ctx.directory + yield* loadSkills(s, yield* InstanceState.get(discovered), events, projectRoot) + // altimate_change end return s }), ) diff --git a/packages/opencode/test/altimate/workspace/publishable.test.ts b/packages/opencode/test/altimate/workspace/publishable.test.ts new file mode 100644 index 0000000000..9bcdf45d59 --- /dev/null +++ b/packages/opencode/test/altimate/workspace/publishable.test.ts @@ -0,0 +1,147 @@ +// altimate_change - new file +// +// Which skills the serve routes offer for publish (publishable.ts). Real directories in a sandbox; +// the home directory is redirected with OPENCODE_TEST_HOME so "personal" is judged against it. +import { afterAll, beforeEach, describe, expect, test } from "bun:test" +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs" +import os from "node:os" +import path from "node:path" + +const { explainIneligible, projectRootFor, publishEligibility } = await import( + "../../../src/altimate/workspace/publishable" +) +const { IDE_DELIVERED_MARKER } = await import("../../../src/altimate/workspace/skill-publish") +const { isInWorkspaceSnapshot, snapshotCopyYields } = await import( + "../../../src/altimate/workspace/snapshot-path" +) + +const SANDBOX = mkdtempSync(path.join(os.tmpdir(), "altimate-publishable-")) +const HOME = path.join(SANDBOX, "home") +const ORIGINAL_TEST_HOME = process.env.OPENCODE_TEST_HOME +let project = "" + +function skill(dir: string): string { + mkdirSync(dir, { recursive: true }) + const location = path.join(dir, "SKILL.md") + writeFileSync(location, `---\nname: ${path.basename(dir)}\n---\n`) + return location +} + +beforeEach(() => { + process.env.OPENCODE_TEST_HOME = HOME + project = mkdtempSync(path.join(SANDBOX, "proj-")) +}) + +afterAll(() => { + if (ORIGINAL_TEST_HOME === undefined) delete process.env.OPENCODE_TEST_HOME + else process.env.OPENCODE_TEST_HOME = ORIGINAL_TEST_HOME + rmSync(SANDBOX, { recursive: true, force: true }) +}) + +describe("publishEligibility", () => { + test("a skill the user wrote in the project is publishable", () => { + const location = skill(path.join(project, ".opencode", "skills", "deploy")) + expect(publishEligibility(location, project, project)).toBe("publishable") + }) + + test("an embedded built-in is not", () => { + expect(publishEligibility("builtin:dbt-develop", project, project)).toBe("builtin") + }) + + test("a personal skill under the home directory is not the project's", () => { + const location = skill(path.join(HOME, ".claude", "skills", "mine")) + expect(publishEligibility(location, project, project)).toBe("personal") + }) + + test("a skill from the workspace snapshot is the workspace's", () => { + const location = skill(path.join(project, ".altimate-code", "skill", "_workspace", "theirs")) + expect(publishEligibility(location, project, project)).toBe("workspace") + }) + + test("a skill the IDE extension delivered is the workspace's", () => { + const dir = path.join(project, ".claude", "skills", "altimate-theirs") + const location = skill(dir) + writeFileSync(path.join(dir, IDE_DELIVERED_MARKER), "{}") + expect(publishEligibility(location, project, project)).toBe("workspace") + }) + + test("a skill outside the project boundary is refused", () => { + const location = skill(path.join(SANDBOX, "elsewhere", "skills", "stray")) + expect(publishEligibility(location, project, project)).toBe("outside-project") + }) + + test("the boundary is the worktree, so a skill at the repository root counts from a subdirectory", () => { + const location = skill(path.join(project, ".opencode", "skills", "deploy")) + const subdirectory = path.join(project, "models") + mkdirSync(subdirectory, { recursive: true }) + expect(publishEligibility(location, subdirectory, projectRootFor(subdirectory, project))).toBe("publishable") + }) +}) + +describe("snapshots and built-ins, wherever they sit", () => { + test("a workspace snapshot in a parent directory is still the workspace's", () => { + // Discovery reads config directories up to the worktree, so a session in `repo/sub` also sees + // `repo/.altimate-code/skill/_workspace`; the snapshot carries no marker by design. + const location = skill(path.join(project, ".altimate-code", "skill", "_workspace", "theirs")) + const subdirectory = path.join(project, "sub") + mkdirSync(subdirectory, { recursive: true }) + expect(publishEligibility(location, subdirectory, project)).toBe("workspace") + }) + + test("the `` placeholder location is a built-in", () => { + expect(publishEligibility("", project, project)).toBe("builtin") + }) +}) + +describe("isInWorkspaceSnapshot", () => { + test("matches the snapshot by whole path segments only", () => { + expect(isInWorkspaceSnapshot("/r/.altimate-code/skill/_workspace/x/SKILL.md")).toBe(true) + expect(isInWorkspaceSnapshot("/r/sub/.altimate-code/skill/_workspace")).toBe(true) + expect(isInWorkspaceSnapshot("/r/.altimate-code/skill/_workspace-notes/x")).toBe(false) + expect(isInWorkspaceSnapshot("/r/.altimate-code/skills/_workspace/x")).toBe(false) + expect(isInWorkspaceSnapshot("/r/.opencode/skills/x")).toBe(false) + }) +}) + +describe("snapshotCopyYields", () => { + const root = "/work/repo" + const snapshot = "/work/repo/.altimate-code/skill/_workspace/pub-1/SKILL.md" + test("the workspace copy yields to the user's own skill in the project", () => { + expect(snapshotCopyYields(snapshot, "/work/repo/.claude/skills/mine/SKILL.md", root)).toBe(true) + expect(snapshotCopyYields(snapshot, "/work/repo/.opencode/skills/mine/SKILL.md", root)).toBe(true) + }) + + test("but still overrides built-in, personal and other snapshot entries, as before", () => { + expect(snapshotCopyYields(snapshot, "builtin:dbt-develop/SKILL.md", root)).toBe(false) + expect(snapshotCopyYields(snapshot, "", root)).toBe(false) + expect(snapshotCopyYields(snapshot, "/home/me/.claude/skills/mine/SKILL.md", root)).toBe(false) + expect(snapshotCopyYields(snapshot, "/work/repo/.altimate-code/skill/_workspace/pub-2/SKILL.md", root)).toBe(false) + }) + + test("a location that is not a string (an inherited property, say) never yields", () => { + expect(snapshotCopyYields(snapshot, undefined, root)).toBe(false) + expect(snapshotCopyYields(snapshot, Object.prototype.constructor, root)).toBe(false) + }) + + test("only a snapshot entry ever yields, and only with a project to judge against", () => { + expect(snapshotCopyYields("/work/repo/.claude/skills/x/SKILL.md", "/work/repo/.opencode/skills/x/SKILL.md", root)).toBe(false) + expect(snapshotCopyYields(snapshot, "/work/repo/.claude/skills/mine/SKILL.md", undefined)).toBe(false) + }) +}) + +describe("projectRootFor", () => { + test("a project with no git uses its directory, never the filesystem root", () => { + expect(projectRootFor("/work/proj", "/")).toBe("/work/proj") + expect(projectRootFor("/work/proj/models", "/work/proj")).toBe("/work/proj") + }) +}) + +describe("explainIneligible", () => { + test("says nothing for a publishable skill and why for the rest", () => { + expect(explainIneligible("deploy", "/p/.opencode/skills/deploy/SKILL.md", "publishable")).toBeNull() + expect(explainIneligible("dbt-develop", "builtin:dbt-develop", "builtin")).toContain("built-in") + expect(explainIneligible("mine", "/h/.claude/skills/mine/SKILL.md", "personal")).toContain("personal skill") + expect(explainIneligible("theirs", "/p/x/SKILL.md", "workspace")).toContain("this workspace sent to you") + expect(explainIneligible("stray", "/e/stray/SKILL.md", "outside-project")).toContain("not inside this project") + }) +}) diff --git a/packages/opencode/test/altimate/workspace/skill-publish.test.ts b/packages/opencode/test/altimate/workspace/skill-publish.test.ts index cfe4c8f5ae..944210dc2a 100644 --- a/packages/opencode/test/altimate/workspace/skill-publish.test.ts +++ b/packages/opencode/test/altimate/workspace/skill-publish.test.ts @@ -42,6 +42,7 @@ const { AltimateApi } = await import("../../../src/altimate/api/client") const { BinaryFileError, EmptyBundleError, + IDE_DELIVERED_MARKER, NotProjectSkillError, NotWorkspaceOwnerError, SkillChangedElsewhereError, @@ -251,6 +252,23 @@ describe("isManagedSkill", () => { test("does not flag the user's own skills", () => { expect(isManagedSkill(project, skillDir)).toBe(false) }) + + test("recognises a skill the IDE extension delivered into the project's own discovery roots", () => { + // `.claude/skills` is a project discovery root, so a delivered skill is found alongside the + // user's own; only the extension's ownership marker tells them apart. + const delivered = path.join(project, ".claude", "skills", "altimate-theirs") + mkdirSync(delivered, { recursive: true }) + writeFileSync(path.join(delivered, "SKILL.md"), "---\nname: altimate-theirs\n---\n") + writeFileSync(path.join(delivered, IDE_DELIVERED_MARKER), "{}") + expect(isManagedSkill(project, delivered)).toBe(true) + }) + + test("does not flag a hand-written skill that only shares the delivered naming", () => { + const own = path.join(project, ".claude", "skills", "altimate-mine") + mkdirSync(own, { recursive: true }) + writeFileSync(path.join(own, "SKILL.md"), "---\nname: altimate-mine\n---\n") + expect(isManagedSkill(project, own)).toBe(false) + }) }) describe("publishSkill", () => { @@ -271,6 +289,23 @@ describe("publishSkill", () => { expect(requests).toHaveLength(0) }) + test("refuses to publish a skill the IDE extension delivered, sending nothing", async () => { + const delivered = path.join(project, ".agents", "skills", "altimate-theirs") + mkdirSync(delivered, { recursive: true }) + writeFileSync(path.join(delivered, "SKILL.md"), "---\nname: altimate-theirs\n---\n") + writeFileSync(path.join(delivered, IDE_DELIVERED_MARKER), "{}") + + const err = await publishSkill({ + projectDirectory: project, + skillDirectory: delivered, + name: "altimate-theirs", + description: "d", + }).catch((e) => e) + + expect(err).toBeInstanceOf(ManagedSkillError) + expect(requests).toHaveLength(0) + }) + test("creates on the first publish and carries the bundle", async () => { const report = await publish() diff --git a/packages/opencode/test/server/altimate-skill-publish-routes.test.ts b/packages/opencode/test/server/altimate-skill-publish-routes.test.ts new file mode 100644 index 0000000000..3a4623ebfc --- /dev/null +++ b/packages/opencode/test/server/altimate-skill-publish-routes.test.ts @@ -0,0 +1,256 @@ +// altimate_change - new file +// +// `/altimate/skill/{publishable,publish}`: the CLI's `skill publish` for the IDE extension. These +// cover the ROUTES — gating, input validation, which skills are offered, status mapping — with the +// skill registry and the publish engine stubbed; eligibility runs for real (publishable.test.ts) +// and the engine is covered by test/altimate/workspace/skill-publish.test.ts. +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs" +import os from "node:os" +import path from "node:path" +import { Server } from "../../src/server/server" +import * as SkillRegistry from "../../src/skill" +import * as SkillPublish from "../../src/altimate/workspace/skill-publish" +import { resetDatabase } from "./db" +import { disposeAllInstances } from "../fixture/fixture" + +const ORIGINAL_FLAG = process.env.ALTIMATE_WORKSPACE +let spies: Array<{ mockRestore: () => void }> = [] + +// The instance directory in these tests is the process's cwd (no directory header). +const OWN = path.join(process.cwd(), ".opencode", "skills", "deploy", "SKILL.md") +const SKILLS = [ + { name: "deploy", description: "Deploy things", location: OWN, content: "" }, + { name: "dbt-develop", description: "Built in", location: "builtin:dbt-develop", content: "" }, + // `description` is optional in frontmatter. + { + name: "Bare", + location: path.join(process.cwd(), ".opencode", "skills", "bare", "SKILL.md"), + content: "", + }, +] + +function request(method: "GET" | "POST", url: string, body?: unknown, headers: Record = {}) { + return Server.Default().request(url, { + method, + headers: { "content-type": "application/json", ...headers }, + body: body === undefined ? undefined : typeof body === "string" ? body : JSON.stringify(body), + }) +} + +// The reload before each lookup runs for real; only what the registry then answers is stubbed. +function stubSkills(skills = SKILLS) { + spies.push(spyOn(SkillRegistry, "all").mockResolvedValue(skills as never)) + spies.push( + spyOn(SkillRegistry, "get").mockImplementation(async (name: string) => skills.find((s) => s.name === name) as never), + ) +} + +const REPORT = { action: "created" as const, publicId: "pub-1", name: "deploy", files: 2, bytes: 2048, datamateId: 42 } + +beforeEach(() => { + process.env.ALTIMATE_WORKSPACE = "1" +}) + +afterEach(async () => { + for (const spy of spies) spy.mockRestore() + spies = [] + if (ORIGINAL_FLAG === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_FLAG + await disposeAllInstances() + await resetDatabase() +}) + +describe("GET /altimate/skill/publishable", () => { + test("lists only the skills that can be published", async () => { + stubSkills() + const response = await request("GET", "/altimate/skill/publishable") + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + ok: true, + skills: [ + { name: "Bare", description: "", location: path.join(process.cwd(), ".opencode", "skills", "bare", "SKILL.md") }, + { name: "deploy", description: "Deploy things", location: OWN }, + ], + }) + }) + + test("is refused outside the workspace pilot", async () => { + delete process.env.ALTIMATE_WORKSPACE + stubSkills() + expect((await request("GET", "/altimate/skill/publishable")).status).toBe(409) + }) + + test("refuses a browser origin on an unsecured server", async () => { + stubSkills() + expect((await request("GET", "/altimate/skill/publishable", undefined, { origin: "https://evil.test" })).status).toBe(403) + }) + + test("refuses a browser's Origin-less cross-site request, such as an image GET", async () => { + // A cross-site GET reloads config and re-pulls skill URLs, so it must not run at all. + stubSkills() + const refused = await request("GET", "/altimate/skill/publishable", undefined, { "sec-fetch-site": "cross-site" }) + expect(refused.status).toBe(403) + expect((await request("GET", "/altimate/skill/publishable", undefined, { "sec-fetch-site": "same-site" })).status).toBe(403) + expect((await request("GET", "/altimate/skill/publishable", undefined, { "sec-fetch-site": "none" })).status).toBe(200) + }) +}) + +describe("POST /altimate/skill/publish", () => { + test("publishes a project skill and returns the report with the CLI's wording", async () => { + stubSkills() + const publish = spyOn(SkillPublish, "publishSkill").mockResolvedValue(REPORT) + spies.push(publish) + + const response = await request("POST", "/altimate/skill/publish", { name: "deploy" }) + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + ok: true, + report: REPORT, + message: 'Published "deploy" in the workspace (2 files, 2KB).', + }) + const input = publish.mock.calls[0][0] + expect(input.skillDirectory).toBe(path.dirname(OWN)) + expect(input.name).toBe("deploy") + expect(input.description).toBe("Deploy things") + }) + + test("refuses a built-in before calling the engine", async () => { + stubSkills() + const publish = spyOn(SkillPublish, "publishSkill") + spies.push(publish) + + const response = await request("POST", "/altimate/skill/publish", { name: "dbt-develop" }) + expect(response.status).toBe(422) + expect(((await response.json()) as { error: string }).error).toContain("built-in") + expect(publish).not.toHaveBeenCalled() + }) + + test("answers 404 for a skill the project cannot reach, suggesting a same-name skill in another case", async () => { + stubSkills() + expect((await request("POST", "/altimate/skill/publish", { name: "nope" })).status).toBe(404) + const response = await request("POST", "/altimate/skill/publish", { name: "DEPLOY" }) + expect(response.status).toBe(404) + expect(((await response.json()) as { error: string }).error).toContain('Did you mean "deploy"?') + }) + + test("reports the engine's own refusals as 422 with their message", async () => { + stubSkills() + const refusal = new SkillPublish.NotLinkedError() + spies.push(spyOn(SkillPublish, "publishSkill").mockRejectedValue(refusal)) + + const response = await request("POST", "/altimate/skill/publish", { name: "deploy" }) + expect(response.status).toBe(422) + expect(await response.json()).toEqual({ ok: false, error: refusal.message }) + }) + + test("reports a backend conflict the engine did not classify as a 422 with the server's detail", async () => { + stubSkills() + const { ConflictError } = await import("../../src/altimate/workspace/api-client") + spies.push( + spyOn(SkillPublish, "publishSkill").mockRejectedValue(new ConflictError({ message: "Bundle is locked." } as never)), + ) + + const response = await request("POST", "/altimate/skill/publish", { name: "deploy" }) + expect(response.status).toBe(422) + expect(await response.json()).toEqual({ ok: false, error: "Bundle is locked." }) + }) + + test("reports anything else as a 500 with its message", async () => { + stubSkills() + spies.push(spyOn(SkillPublish, "publishSkill").mockRejectedValue(new Error("boom"))) + + const response = await request("POST", "/altimate/skill/publish", { name: "deploy" }) + expect(response.status).toBe(500) + expect(await response.json()).toEqual({ ok: false, error: "boom" }) + }) + + test("rejects a missing, empty or non-string name, and a malformed body", async () => { + stubSkills() + const publish = spyOn(SkillPublish, "publishSkill") + spies.push(publish) + + expect((await request("POST", "/altimate/skill/publish", {})).status).toBe(400) + expect((await request("POST", "/altimate/skill/publish", { name: " " })).status).toBe(400) + expect((await request("POST", "/altimate/skill/publish", { name: 7 })).status).toBe(400) + expect((await request("POST", "/altimate/skill/publish", "{bad")).status).toBe(400) + expect(publish).not.toHaveBeenCalled() + }) + + test("is refused outside the workspace pilot and from a browser origin", async () => { + stubSkills() + const publish = spyOn(SkillPublish, "publishSkill") + spies.push(publish) + + expect((await request("POST", "/altimate/skill/publish", { name: "deploy" }, { origin: "https://evil.test" })).status).toBe(403) + delete process.env.ALTIMATE_WORKSPACE + expect((await request("POST", "/altimate/skill/publish", { name: "deploy" })).status).toBe(409) + expect(publish).not.toHaveBeenCalled() + }) +}) + +describe("the workspace's copy of your own skill", () => { + test("does not hide the project skill it was published from", async () => { + // Real registry. `.claude/skills` is scanned before the snapshot; before the precedence rule + // the synced copy won, so the user's own skill vanished from the list and could not be updated. + const project = mkdtempSync(path.join(os.tmpdir(), "altimate-shadow-route-")) + try { + const write = (dir: string) => { + mkdirSync(dir, { recursive: true }) + writeFileSync(path.join(dir, "SKILL.md"), "---\nname: shared-skill\ndescription: d\n---\n\nbody\n") + } + write(path.join(project, ".claude", "skills", "shared-skill")) + write(path.join(project, ".altimate-code", "skill", "_workspace", "pub-1")) + const response = await request("GET", "/altimate/skill/publishable", undefined, { "x-opencode-directory": project }) + const skills = ((await response.json()) as { skills: { name: string; location: string }[] }).skills + const shared = skills.find((s) => s.name === "shared-skill") + expect(shared?.location).toContain(path.join(".claude", "skills", "shared-skill")) + } finally { + rmSync(project, { recursive: true, force: true }) + } + }) +}) + +describe("a workspace skill named like an Object.prototype property", () => { + test("loads without tripping the precedence check", async () => { + // `state.skills` is a plain object; `constructor` must not resolve to the inherited one. + const project = mkdtempSync(path.join(os.tmpdir(), "altimate-proto-route-")) + try { + const dir = path.join(project, ".altimate-code", "skill", "_workspace", "pub-1") + mkdirSync(dir, { recursive: true }) + writeFileSync(path.join(dir, "SKILL.md"), "---\nname: constructor\ndescription: d\n---\n\nbody\n") + const response = await request("GET", "/altimate/skill/publishable", undefined, { "x-opencode-directory": project }) + expect(response.status).toBe(200) + // A workspace skill is never offered for publish. + expect(((await response.json()) as { skills: { name: string }[] }).skills.map((s) => s.name)).not.toContain( + "constructor", + ) + } finally { + rmSync(project, { recursive: true, force: true }) + } + }) +}) + +describe("a skill written after the server loaded", () => { + test("is listed, even when its skills directory did not exist at first", async () => { + // Real registry: the case the in-context reload exists for. A project with no `.opencode/` + // at all, then `altimate-code skill create` (or the chat) writes one. + const project = mkdtempSync(path.join(os.tmpdir(), "altimate-publishable-route-")) + try { + const list = async () => { + const response = await request("GET", "/altimate/skill/publishable", undefined, { + "x-opencode-directory": project, + }) + expect(response.status).toBe(200) + return ((await response.json()) as { skills: { name: string }[] }).skills.map((s) => s.name) + } + expect(await list()).not.toContain("late-skill") + const dir = path.join(project, ".opencode", "skills", "late-skill") + mkdirSync(dir, { recursive: true }) + writeFileSync(path.join(dir, "SKILL.md"), "---\nname: late-skill\ndescription: Written late.\n---\n\nbody\n") + expect(await list()).toContain("late-skill") + } finally { + rmSync(project, { recursive: true, force: true }) + } + }) +})