diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index eccd2a3c49..8754661a9c 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -716,6 +716,15 @@ export namespace Telemetry { skill_name: string source: "cli" | "tui" } + | { + type: "skill_published" + timestamp: number + session_id: string + skill_name: string + action: "created" | "updated" + file_count: number + source: "cli" | "tui" + } // altimate_change end // altimate_change start — plan refinement telemetry event | { diff --git a/packages/opencode/src/altimate/workspace/skill-publish.ts b/packages/opencode/src/altimate/workspace/skill-publish.ts index b31b420b6a..d1ea64ab74 100644 --- a/packages/opencode/src/altimate/workspace/skill-publish.ts +++ b/packages/opencode/src/altimate/workspace/skill-publish.ts @@ -168,6 +168,22 @@ export class NotLinkedError extends Error { } } +/** The skill directory is not a project skill: it lives outside the project + * (a personal skill under `~/.claude/skills` or the like), or it reaches + * the project only through a symbolic link. Publishing shares a bundle with + * the whole workspace — a personal skill is not the user's to share by + * accident, and a linked root would publish whatever it points at, which + * `isManagedSkill` cannot see if the target is not the managed snapshot. */ +export class NotProjectSkillError extends Error { + constructor(readonly skillDirectory: string) { + super( + `"${skillDirectory}" is not a skill of this project — it lives outside the project, or is reached ` + + `through a symbolic link. Only a project's own skills can be published to its workspace.`, + ) + this.name = "NotProjectSkillError" + } +} + /** The project is linked to a workspace the caller does not own. Linking * needs only visibility — a colleague's shared workspace can be linked to — * but attaching a skill is a write against the workspace and needs @@ -577,23 +593,35 @@ async function attachToWorkspace(publicId: string, datamateId: number): Promise< * `privacy` is left unset: the server defaults to `private`. Publishing should * attach a skill to a workspace, not disclose it to the whole organisation as a * side effect of a command whose name says nothing about visibility. */ -export async function publishSkill(input: { +export interface PublishInput { + /** Where the workspace binding lives — the directory the session was + * started in. `resolveBinding` is keyed on it. */ projectDirectory: string + /** The boundary a skill must lie within to count as this project's. + * Discovery walks up to the git worktree root, so a skill under + * `repo/.opencode/skills` is the project's even when the session started + * in `repo/models` — and `projectDirectory` alone would refuse it. + * Defaults to `projectDirectory` for a project with no worktree. */ + projectRoot?: string skillDirectory: string name: string description: string -}): Promise { +} + +export async function publishSkill(input: PublishInput): Promise { return withPublishLock(input.skillDirectory, () => publishSkillUnlocked(input)) } -async function publishSkillUnlocked(input: { - projectDirectory: string - skillDirectory: string - name: string - description: string -}): Promise { +async function publishSkillUnlocked(input: PublishInput): Promise { if (isManagedSkill(input.projectDirectory, input.skillDirectory)) throw new ManagedSkillError(input.skillDirectory) + // The root itself, resolved: `collectBundle` refuses links INSIDE the + // skill, but a root that is a link is followed, and would publish whatever + // it points at. And the resolved root must be inside the project: the + // loader also serves personal skills from under the home directory, which + // are not this workspace's to receive. The REAL path that passed is what + // the walk reads, so a root swapped after the check is not what uploads. + const skillRoot = assertProjectSkill(input.projectRoot ?? input.projectDirectory, input.skillDirectory) // Before the bundle is even read. An unlinked project has nowhere to attach // to, and uploading first would create the orphan this module exists to @@ -614,7 +642,7 @@ async function publishSkillUnlocked(input: { // uploaded until the attach is known to be possible. await assertOwnsWorkspace(binding.datamateId, binding.datamateName, scope.userId) - const files = await collectBundle(input.skillDirectory) + const files = await collectBundle(skillRoot) if (files.length === 0) throw new EmptyBundleError() const bytes = files.reduce((n, f) => n + Buffer.byteLength(f.content, "utf8"), 0) @@ -721,6 +749,35 @@ async function publishSkillUnlocked(input: { return { action: "created", publicId, name: input.name, files: files.length, bytes, datamateId: binding.datamateId } } +/** One line for a surface to show after a publish. Both the CLI and the TUI + * say the same thing, so a user moving between them recognises the outcome. */ +export function describePublish(report: PublishReport): string { + const verb = report.action === "created" ? "Published" : "Updated" + const size = report.bytes >= 1024 ? `${Math.round(report.bytes / 1024)}KB` : `${report.bytes}B` + return `${verb} "${report.name}" in the workspace (${report.files} file${report.files === 1 ? "" : "s"}, ${size}).` +} + +/** The message for an error this module raised on purpose, or null for one it + * did not — a surface shows the former as-is (each already says what to do) + * and wraps the latter as a failure. */ +export function explainPublishError(err: unknown): string | null { + if ( + err instanceof NotLinkedError || + err instanceof ManagedSkillError || + err instanceof NotProjectSkillError || + err instanceof BinaryFileError || + err instanceof SymlinkError || + err instanceof EmptyBundleError || + err instanceof BundleTooLargeError || + err instanceof SkillNameConflictError || + err instanceof SkillChangedElsewhereError || + err instanceof NotWorkspaceOwnerError || + err instanceof AttachFailedError + ) + return err.message + return null +} + /** Which of the update path's conflicts this is. The server distinguishes them * in its detail; this module's typed errors then carry advice that fits. * Anything unrecognised keeps the server's own words rather than being @@ -732,6 +789,51 @@ function updateConflict(err: ConflictError, skillName: string): Error { return err } +/** The skill's real directory, once it has passed. Exported for its test: + * the boundary rule has to hold for every caller, and a root of `/` — the + * sentinel a project with no git carries — is not a boundary at all. */ +export function assertProjectSkill(projectRoot: string, skillDirectory: string): string { + // The boundary is the root's REAL path, resolved once and used for both + // checks below. A filesystem root would contain everything: both callers + // substitute the session directory for the `/` sentinel, and this refuses + // it in case one forgets, since the failure mode is publishing anything + // on the machine. Judged after resolving — a root that is a symbolic link + // to `/` is `/` for the containment comparison, so it must be refused on + // the same value that comparison uses. + let root: string + try { + root = realpathSync(projectRoot) + } catch { + root = path.resolve(projectRoot) + } + if (root === path.parse(root).root) throw new NotProjectSkillError(skillDirectory) + const lexical = path.resolve(skillDirectory) + let real: string + try { + real = realpathSync(lexical) + } catch { + // Absent: `collectBundle` fails on it in a moment with a better message. + return lexical + } + // "The root is a link" is judged on the LAST component only: the parent's + // real path plus the skill's own name must equal the skill's real path. + // Comparing the whole path to its lexical form would call every skill on + // macOS a link, since `/var` and `/tmp` are links to `/private/...`. + let parentReal: string + try { + parentReal = realpathSync(path.dirname(lexical)) + } catch { + return lexical + } + if (real !== path.join(parentReal, path.basename(lexical))) throw new NotProjectSkillError(skillDirectory) + const rel = path.relative(root, real) + // Parent traversal exactly, not any name that begins with two dots: a + // skill directory literally named `..foo` is inside the project. + if (rel === ".." || rel.startsWith(".." + path.sep) || path.isAbsolute(rel)) + throw new NotProjectSkillError(skillDirectory) + return real +} + /** Refuse before upload when the bound workspace is not the caller's. Read * from the same list the picker uses, which carries each workspace's owner. A * list that omits the owner (an older server) cannot answer, and the attach diff --git a/packages/opencode/src/cli/cmd/skill-helpers.ts b/packages/opencode/src/cli/cmd/skill-helpers.ts index d32620c31b..72c94e3e17 100644 --- a/packages/opencode/src/cli/cmd/skill-helpers.ts +++ b/packages/opencode/src/cli/cmd/skill-helpers.ts @@ -61,7 +61,7 @@ export function skillSource(location: string): string { if (location.startsWith("builtin:")) return "builtin" const home = Global.Path.home // Builtin skills shipped with altimate-code - if (location.startsWith(path.join(home, ".altimate", "builtin"))) return "builtin" + if (isInside(location, path.join(home, ".altimate", "builtin"))) return "builtin" // Global user skills (~/.claude/skills/, ~/.agents/skills/, ~/.config/altimate-code/skills/) const globalDirs = [ path.join(home, ".claude", "skills"), @@ -69,11 +69,19 @@ export function skillSource(location: string): string { path.join(home, ".altimate-code", "skills"), path.join(Global.Path.config, "skills"), ] - if (globalDirs.some((dir) => location.startsWith(dir))) return "global" + if (globalDirs.some((dir) => isInside(location, dir))) return "global" // Everything else is project-level return "project" } +/** Path containment by segments, not by string prefix: `~/.claude/skills-archive/x` + * is not inside `~/.claude/skills`, and a raw `startsWith` said it was — which + * refused a project skill as personal. */ +function isInside(location: string, dir: string): boolean { + const rel = path.relative(dir, location) + return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel)) +} + /** Check if a tool is available on the current PATH (including .altimate-code/tools/ and .opencode/tools/). */ export async function isToolOnPath(toolName: string, cwd: string): Promise { // Check project tools/ in both cwd and worktree (they may differ in monorepos) diff --git a/packages/opencode/src/cli/cmd/skill.ts b/packages/opencode/src/cli/cmd/skill.ts index a658e6955c..30daacc8de 100644 --- a/packages/opencode/src/cli/cmd/skill.ts +++ b/packages/opencode/src/cli/cmd/skill.ts @@ -11,6 +11,7 @@ import { Global } from "@/global" import { detectToolReferences, skillSource, isToolOnPath } from "./skill-helpers" // altimate_change start — telemetry for skill operations import { Telemetry } from "@/altimate/telemetry" +import { describePublish, explainPublishError, publishSkill } from "@/altimate/workspace/skill-publish" // altimate_change end // --------------------------------------------------------------------------- @@ -457,6 +458,76 @@ const SkillTestCommand = cmd({ }, }) +const SkillPublishCommand = cmd({ + command: "publish ", + describe: "publish a skill to the workspace this project is linked to", + builder: (yargs) => + yargs.positional("name", { + type: "string", + describe: "name of the skill to publish", + demandOption: true, + }), + async handler(args) { + const name = args.name as string + const cwd = process.cwd() + await bootstrap(cwd, async () => { + const skill = await Skill.get(name) + if (!skill) { + process.stderr.write(`Skill "${name}" not found. Run \`altimate-code skill list\` to see the skills this project can reach.` + EOL) + process.exitCode = 1 + return + } + // Built-in skills ship with altimate-code — embedded, or installed under + // `~/.altimate/builtin` — and are not the user's to publish. A personal + // skill under the home directory is the user's, but not this project's: + // publishing shares it with the whole workspace, which is not what + // keeping it in `~/.claude/skills` says. A skill the workspace sent us + // is refused by `publishSkill` itself, as is a symlinked root. + const source = skillSource(skill.location) + if (source === "builtin" || !path.isAbsolute(skill.location)) { + process.stderr.write(`"${name}" is a built-in skill and cannot be published.` + EOL) + process.exitCode = 1 + return + } + if (source === "global") { + process.stderr.write( + `"${name}" is a personal skill (${path.dirname(skill.location)}), not one of this project's. ` + + `Copy it into the project's skills directory to publish it.` + EOL, + ) + process.exitCode = 1 + return + } + try { + const report = await publishSkill({ + projectDirectory: Instance.directory, + // Discovery walks up to the worktree; so must the boundary, or a + // skill under the repository root is refused from a subdirectory. + projectRoot: Instance.worktree !== "/" ? Instance.worktree : Instance.directory, + skillDirectory: path.dirname(skill.location), + name: skill.name, + description: skill.description ?? "", + }) + process.stdout.write(describePublish(report) + EOL) + try { + 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: "cli", + }) + } catch {} + } catch (err) { + const known = explainPublishError(err) + process.stderr.write((known ?? `Publish failed: ${err instanceof Error ? err.message : String(err)}`) + EOL) + process.exitCode = 1 + } + }) + }, +}) + const SkillShowCommand = cmd({ command: "show ", describe: "display the full content of a skill", @@ -738,6 +809,7 @@ export const SkillCommand = cmd({ .command(SkillListCommand) .command(SkillCreateCommand) .command(SkillTestCommand) + .command(SkillPublishCommand) .command(SkillShowCommand) .command(SkillInstallCommand) .command(SkillRemoveCommand) diff --git a/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx b/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx index e6da8c85d8..d59894f5c7 100644 --- a/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx +++ b/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx @@ -26,7 +26,9 @@ import type { TuiPlugin, TuiPluginApi, TuiDialogSelectOption } from "@opencode-ai/plugin/tui" import type { BuiltinTuiPlugin } from "@opencode-ai/tui/builtins" import { createMemo, createResource, createSignal, Show } from "solid-js" -import { detectToolReferences } from "@/cli/cmd/skill-helpers" +import { detectToolReferences, skillSource } from "@/cli/cmd/skill-helpers" +import { describePublish, explainPublishError, isManagedSkill, publishSkill } from "@/altimate/workspace/skill-publish" +import { Telemetry } from "@/altimate/telemetry" import { spawn } from "child_process" import os from "os" import path from "path" @@ -500,15 +502,94 @@ function isRemovable(info: SkillInfo): boolean { return gitCheck.exitCode !== 0 // only removable if NOT git-tracked } +/** Built-in the way the CLI decides it: embedded (`builtin:`), or installed + * under `~/.altimate/builtin`, which the loader prefers when present and + * registers by ABSOLUTE path. A prefix check alone let every shipped + * built-in through as publishable on a normal install — and a built-in + * published to a workspace syncs back as a managed skill that overrides the + * shipped one for every linked member, frozen at that version. */ +export function isBuiltinLocation(location: string | undefined): boolean { + return !location || skillSource(location) === "builtin" || !path.isAbsolute(location) +} + +/** A personal skill under the home directory: the user's, but not this + * project's, so not the workspace's to receive. Same rule as the CLI. */ +export function isGlobalLocation(location: string | undefined): boolean { + return !!location && skillSource(location) === "global" +} + +/** One publish at a time from the picker. `DialogSelect` calls the handler + * for every Enter without awaiting it, so a second press before the first + * settled entered `publishSkill` again — the per-directory lock serialised + * the two but did not coalesce them, and the user got a create, a redundant + * update, and two success toasts. */ +let publishInFlight: string | null = null + +/** The publish half of the action picker, as one call returning the toast to + * show. Kept out of the picker's `onSelect` so that switch stays readable. */ +export async function publishFromPicker( + info: SkillInfo, + skillName: string, + projectDirectory: string, + projectRoot: string, +): Promise<{ message: string; variant: "success" | "warning" | "error"; duration: number }> { + try { + const report = await publishSkill({ + projectDirectory, + projectRoot, + skillDirectory: path.dirname(info.location), + name: skillName, + description: info.description ?? "", + }) + try { + Telemetry.track({ + type: "skill_published", + timestamp: Date.now(), + session_id: Telemetry.getContext().sessionId || "", + skill_name: skillName, + action: report.action, + file_count: report.files, + source: "tui", + }) + } catch {} + return { message: describePublish(report), variant: "success", duration: 6000 } + } catch (err) { + const known = explainPublishError(err) + return { + message: known ?? `Publish failed: ${(err instanceof Error ? err.message : String(err)).slice(0, 150)}`, + variant: known ? "warning" : "error", + duration: 8000, + } + } +} + function openActionPicker(api: TuiPluginApi, info: SkillInfo | undefined, skillName: string, reopen: () => void) { - const isBuiltin = !info || info.location.startsWith("builtin:") || !path.isAbsolute(info.location) + const isBuiltin = isBuiltinLocation(info?.location) + const isGlobal = isGlobalLocation(info?.location) const removable = !!info && isRemovable(info) + // A skill the workspace sent us is not ours to publish back to it. Judged + // against the project directory workspace sync uses — the binding and the + // managed snapshot live under `api.state.path.directory`, not the git root + // `workdir` resolves to, and the two differ in a worktree subdirectory. + const projectDirectory = api.state.path.directory || workdir(api) + // The boundary a skill must lie within: the worktree, since discovery + // walks up to it — EXCEPT for a project with no git, where the worktree + // is the sentinel `/` and `workdir` returns it unchanged. A root of `/` + // would accept any skill on the machine. Same fallback as the CLI. + const projectRoot = api.state.path.worktree === "/" ? projectDirectory : workdir(api) + const managed = !isBuiltin && isManagedSkill(projectDirectory, path.dirname(info!.location)) const actions: TuiDialogSelectOption[] = ( [ { title: "Show details", value: "show", description: "View skill info, tools, and location" }, { title: "Edit", value: "edit", description: "Open SKILL.md in your default editor", disabled: isBuiltin }, { title: "Test", value: "test", description: "Validate the paired CLI tool works" }, + { + title: "Publish to workspace", + value: "publish", + description: "Upload this skill to the linked workspace so your team gets it", + disabled: isBuiltin || isGlobal || managed, + }, { title: "Remove", value: "remove", description: "Delete this skill and its paired tool", disabled: !removable }, ] as TuiDialogSelectOption[] ).filter((a) => !a.disabled) @@ -559,6 +640,22 @@ function openActionPicker(api: TuiPluginApi, info: SkillInfo | undefined, skillN reopen() break } + case "publish": { + if (!info) return + if (publishInFlight) { + api.ui.toast({ message: `Still publishing ${publishInFlight}…`, variant: "info", duration: 2000 }) + return + } + publishInFlight = skillName + try { + api.ui.toast({ message: `Publishing ${skillName}...`, variant: "info", duration: 120_000 }) + api.ui.toast(await publishFromPicker(info, skillName, projectDirectory, projectRoot)) + } finally { + publishInFlight = null + } + reopen() + break + } case "remove": { if (!info) return try { diff --git a/packages/opencode/test/altimate/plugin/skill-ops-builtin.test.ts b/packages/opencode/test/altimate/plugin/skill-ops-builtin.test.ts new file mode 100644 index 0000000000..a18c7d53d1 --- /dev/null +++ b/packages/opencode/test/altimate/plugin/skill-ops-builtin.test.ts @@ -0,0 +1,55 @@ +// altimate_change - new file +// +// The Skills dialog's action picker must decide "built-in" the way the CLI +// does. It decided by prefix alone — `builtin:` or a non-absolute path — and +// on any postinstall'd machine the loader prefers the filesystem copy under +// `~/.altimate/builtin`, registered by ABSOLUTE path. So every shipped +// built-in was publishable from the TUI, and one published to a workspace +// syncs back as a managed skill that overrides the shipped one for every +// linked member, frozen at that version. (Ralph, review of #1313; Kilo and +// Codex found the same trace independently.) +import { describe, expect, test } from "bun:test" +import path from "node:path" +import { Global } from "../../../src/global" +import { skillSource } from "../../../src/cli/cmd/skill-helpers" +import { isBuiltinLocation, isGlobalLocation } from "../../../src/plugin/tui/altimate/skill-ops" +import { isManagedSkill } from "../../../src/altimate/workspace/skill-publish" + +describe("the action picker's notion of built-in", () => { + test("agrees with the CLI's for a filesystem-installed built-in", () => { + const installed = path.join(Global.Path.home, ".altimate", "builtin", "x", "SKILL.md") + // The three predicates Ralph traced: CLI true, TUI true, managed false. + expect(skillSource(installed)).toBe("builtin") + expect(isBuiltinLocation(installed)).toBe(true) + expect(isManagedSkill("/some/project", path.dirname(installed))).toBe(false) + }) + + test("still treats the embedded and the non-absolute forms as built-in", () => { + expect(isBuiltinLocation("builtin:x/SKILL.md")).toBe(true) + expect(isBuiltinLocation("relative/x/SKILL.md")).toBe(true) + expect(isBuiltinLocation(undefined)).toBe(true) + }) + + test("does not call a project skill built-in", () => { + expect(isBuiltinLocation("/some/project/.opencode/skills/deploy/SKILL.md")).toBe(false) + }) + + test("a personal skill under the home directory is global, and not publishable", () => { + for (const dir of [".claude", ".agents", ".altimate-code"]) { + expect(isGlobalLocation(path.join(Global.Path.home, dir, "skills", "x", "SKILL.md"))).toBe(true) + } + expect(isGlobalLocation("/some/project/.opencode/skills/deploy/SKILL.md")).toBe(false) + }) +}) + +describe("skillSource contains by path segment, not by string prefix", () => { + test("a sibling directory sharing a global dir's prefix is a project skill", () => { + // `~/.claude/skills-archive/x` starts with the string `~/.claude/skills` + // but is not inside it; a raw prefix check refused it as personal. + const sibling = path.join(Global.Path.home, ".claude", "skills-archive", "x", "SKILL.md") + expect(skillSource(sibling)).toBe("project") + expect(isGlobalLocation(sibling)).toBe(false) + const builtinSibling = path.join(Global.Path.home, ".altimate", "builtin-old", "x", "SKILL.md") + expect(skillSource(builtinSibling)).toBe("project") + }) +}) diff --git a/packages/opencode/test/altimate/workspace/skill-publish.test.ts b/packages/opencode/test/altimate/workspace/skill-publish.test.ts index 853ec0c318..080d000a63 100644 --- a/packages/opencode/test/altimate/workspace/skill-publish.test.ts +++ b/packages/opencode/test/altimate/workspace/skill-publish.test.ts @@ -42,13 +42,17 @@ const { AltimateApi } = await import("../../../src/altimate/api/client") const { BinaryFileError, EmptyBundleError, + NotProjectSkillError, NotWorkspaceOwnerError, SkillChangedElsewhereError, ManagedSkillError, NotLinkedError, SkillNameConflictError, SymlinkError, + assertProjectSkill, collectBundle, + describePublish, + explainPublishError, isManagedSkill, ledgerPathForTests, publishSkill, @@ -578,12 +582,17 @@ describe("the published-id ledger", () => { // `/private/tmp`, a linked worktree) was two ledger keys, so the second // publish created again and 409'd on its own name — "published from // somewhere else", by this machine, a moment ago. - const alias = path.join(project, "skills", "deploy-alias") - symlinkSync(skillDir, alias) - await publishSkill({ projectDirectory: project, skillDirectory: alias, name: "deploy", description: "d" }) + // Two spellings of one directory, on every platform: the project's real + // path, and the project reached through a symlinked PARENT. Only the + // skill's own last component may not be a link, so a linked ancestor is + // allowed — it is the `/var` → `/private/var` case, made explicit. + const linkedParent = path.join(SANDBOX, `via-link-${Math.random().toString(36).slice(2)}`) + symlinkSync(project, linkedParent) + const viaLink = path.join(linkedParent, "skills", "deploy") + await publishSkill({ projectDirectory: project, skillDirectory: viaLink, name: "deploy", description: "d" }) requests = [] - const report = await publish() + const report = await publishSkill({ projectDirectory: project, skillDirectory: skillDir, name: "deploy", description: "d" }) expect(report.action).toBe("updated") expect(requests.filter((r) => r.method === "POST")).toHaveLength(0) @@ -627,6 +636,92 @@ describe("the published-id ledger", () => { }) +describe("what counts as a project skill", () => { + test("a skill root that is a symbolic link is refused before anything is read", async () => { + // The walk refuses links INSIDE a skill; a root that is itself a link was + // followed, and published whatever it pointed at — a built-in, say — + // which `isManagedSkill` cannot see because the target is not the + // managed snapshot. + // The target is INSIDE the project, so the outside-project rule does not + // catch it: only the root-is-a-link rule does. + const target = path.join(project, "vendor", "elsewhere") + mkdirSync(target, { recursive: true }) + writeFileSync(path.join(target, "SKILL.md"), "---\nname: elsewhere\n---\n") + const link = path.join(project, "skills", "looks-local") + symlinkSync(target, link) + + const err = await publishSkill({ projectDirectory: project, skillDirectory: link, name: "elsewhere", description: "d" }).catch( + (e) => e, + ) + + expect(err).toBeInstanceOf(NotProjectSkillError) + expect(requests.filter((r) => r.method === "POST")).toHaveLength(0) + }) + + test("a skill under the repository root publishes from a subdirectory", async () => { + // Discovery walks up to the worktree root, so a session started in + // `repo/models` can reach `repo/.opencode/skills/x`. The binding stays + // keyed on the session's directory; the containment boundary is the root. + const sub = path.join(project, "models") + mkdirSync(sub, { recursive: true }) + await link(42, "Growth", sub) + requests = [] + + const report = await publishSkill({ + projectDirectory: sub, + projectRoot: project, + skillDirectory: skillDir, // repo/skills/deploy — above `sub` + name: "deploy", + description: "d", + }) + + expect(report.action).toBe("created") + }) + + test("a root of `/` is no boundary: the session directory must be the fallback", () => { + // A project with no git carries the sentinel worktree `/`. Passed through + // as the boundary, it contains every skill on the machine — a parent + // directory's `.opencode/skills/x`, an absolute `skills.paths` entry. + // Ralph traced it on the TUI, where `workdir` returned `/` unchanged; + // the shared path refuses it too, for the next caller that forgets. + const elsewhere = mkdtempSync(path.join(SANDBOX, "elsewhere-")) + writeFileSync(path.join(elsewhere, "SKILL.md"), "---\nname: x\n---\n") + expect(() => assertProjectSkill("/", elsewhere)).toThrow(NotProjectSkillError) + // And a root that is merely a LINK to `/`: lexically it is a directory + // inside the sandbox, but the containment comparison resolves it, and + // the refusal must be judged on that same resolved value. + const rootLink = path.join(SANDBOX, `root-link-${Math.random().toString(36).slice(2)}`) + symlinkSync("/", rootLink) + expect(() => assertProjectSkill(rootLink, elsewhere)).toThrow(NotProjectSkillError) + // The same skill against the session directory: outside it, refused; + // inside it, allowed. + expect(() => assertProjectSkill(project, elsewhere)).toThrow(NotProjectSkillError) + expect(assertProjectSkill(project, skillDir)).toBe(realpathSync(skillDir)) + }) + + test("a directory named with two leading dots is still inside the project", () => { + // Directly under the root, so `path.relative` is `..foo` itself — the + // one spelling a `startsWith("..")` check mistakes for traversal. + const odd = path.join(project, "..foo") + mkdirSync(odd, { recursive: true }) + expect(assertProjectSkill(project, odd)).toBe(realpathSync(odd)) + }) + + test("a skill outside the project is refused", async () => { + // A personal skill under the home directory is the user's, not this + // project's, and publishing would share it with the whole workspace. + const personal = mkdtempSync(path.join(SANDBOX, "personal-")) + writeFileSync(path.join(personal, "SKILL.md"), "---\nname: personal\n---\n") + + const err = await publishSkill({ projectDirectory: project, skillDirectory: personal, name: "personal", description: "d" }).catch( + (e) => e, + ) + + expect(err).toBeInstanceOf(NotProjectSkillError) + expect(requests.filter((r) => r.method === "POST")).toHaveLength(0) + }) +}) + describe("a workspace the caller does not own", () => { // Linking needs only visibility, so a project can be bound to a colleague's // shared workspace. Attaching a skill needs ownership, and answers 404 @@ -803,3 +898,39 @@ describe("attaching to the workspace", () => { expect(requests.filter((r) => r.method === "POST")).toHaveLength(0) }) }) + +describe("what a surface says", () => { + // The CLI and the TUI share these lines so a user moving between them + // recognises the outcome. + test("names the outcome, the skill, and the size", () => { + const line = describePublish({ action: "created", publicId: "p", name: "deploy", files: 3, bytes: 2048, datamateId: 1 }) + expect(line).toContain("Published") + expect(line).toContain('"deploy"') + expect(line).toContain("3 files") + expect(line).toContain("2KB") + expect(describePublish({ action: "updated", publicId: "p", name: "d", files: 1, bytes: 12, datamateId: 1 })).toContain( + "Updated", + ) + expect(describePublish({ action: "updated", publicId: "p", name: "d", files: 1, bytes: 12, datamateId: 1 })).toContain( + "1 file,", + ) + }) + + test("passes a deliberate error through and wraps nothing else", async () => { + // Each typed error already says what to do; an unexpected one must not be + // shown as if it were advice. + const unlinked = mkdtempSync(path.join(SANDBOX, "unlinked-")) + const dir = path.join(unlinked, "skills", "x") + mkdirSync(dir, { recursive: true }) + writeFileSync(path.join(dir, "SKILL.md"), "---\nname: x\n---\n") + const err = await publishSkill({ projectDirectory: unlinked, skillDirectory: dir, name: "x", description: "d" }).catch( + (e) => e, + ) + expect(err).toBeInstanceOf(NotLinkedError) + expect(explainPublishError(err)).toContain("altimate-code link") + expect(explainPublishError(new SymlinkError("references"))).toContain("references") + // Advice, not a failure: the surfaces show this one as-is. + expect(explainPublishError(new NotWorkspaceOwnerError("ws"))).toContain("Skills can only be published") + expect(explainPublishError(new Error("ECONNRESET"))).toBeNull() + }) +})