diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1fdec96..c3155d0 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -65,7 +65,7 @@ export type { SetRulesSourceOptions } from "./commands/rules.js"; export type { PluginRegistry, RegisteredAgent, RegisteredDomain } from "./plugin-loader.js"; export { createLinker, describeSymlinkFailure, ensureLink } from "./fs/link.js"; export type { EnsureLinkResult } from "./fs/link.js"; -export { createRepoFetcher } from "./resolver.js"; +export { createRepoFetcher, gigetTarballPath } from "./resolver.js"; export { parseSource, parseCompositeSkillRef, isProvider, SUPPORTED_PROVIDERS } from "./source.js"; export type { ParsedSource, diff --git a/packages/core/src/resolver.ts b/packages/core/src/resolver.ts index fb6d1fd..7960014 100644 --- a/packages/core/src/resolver.ts +++ b/packages/core/src/resolver.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import { createHash } from "node:crypto"; import { downloadTemplate } from "giget"; @@ -55,6 +56,12 @@ async function fetchGit( await fs.rm(destDir, { recursive: true, force: true }); await fs.mkdir(path.dirname(destDir), { recursive: true }); + if (opts?.noCache) { + const tarPath = gigetTarballPath(source, ref); + await fs.rm(tarPath, { force: true }); + await fs.rm(`${tarPath}.json`, { force: true }); + } + const gigetSource = ref ? `${source.provider}:${source.owner}/${source.repo}#${ref}` : `${source.provider}:${source.owner}/${source.repo}`; @@ -85,6 +92,26 @@ async function fetchGit( return { path: destDir }; } +function gigetCacheDir(): string { + return process.env["XDG_CACHE_HOME"] + ? path.resolve(process.env["XDG_CACHE_HOME"], "giget") + : path.resolve(os.homedir(), ".cache/giget"); +} + +export function gigetTarballPath(source: GitSource, ref: string | undefined): string { + const name = `${source.owner}-${source.repo}`.replace(/[^\da-z-]/gi, "-"); + const version = ref ?? "main"; + const sourceDir = path.resolve(gigetCacheDir(), source.provider, name); + const tarPath = path.resolve(sourceDir, `${version}.tar.gz`); + const rel = path.relative(sourceDir, tarPath); + if (rel === "" || rel.startsWith("..") || path.isAbsolute(rel)) { + throw new Error( + `Refusing to compute giget tarball path: ref "${ref ?? ""}" escapes source cache dir`, + ); + } + return tarPath; +} + async function dirHasFiles(p: string): Promise { try { const entries = await fs.readdir(p); diff --git a/packages/core/test/resolver-nocache.test.ts b/packages/core/test/resolver-nocache.test.ts new file mode 100644 index 0000000..709112d --- /dev/null +++ b/packages/core/test/resolver-nocache.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import fs from "node:fs/promises"; +import path from "node:path"; +import os from "node:os"; + +const downloadTemplate = vi.fn(async (_src: string, opts: { dir: string }) => { + await fs.mkdir(opts.dir, { recursive: true }); + await fs.writeFile(path.join(opts.dir, "marker.txt"), "ok"); + return { dir: opts.dir }; +}); + +vi.mock("giget", () => ({ + downloadTemplate: (src: string, opts: { dir: string }) => downloadTemplate(src, opts), +})); + +import { createRepoFetcher, gigetTarballPath, parseSource } from "../src/index.js"; + +describe("createRepoFetcher noCache", () => { + let root: string; + let cacheHome: string; + let originalXdg: string | undefined; + + beforeEach(async () => { + downloadTemplate.mockClear(); + root = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-resolver-")); + cacheHome = await fs.mkdtemp(path.join(os.tmpdir(), "agnos-xdg-")); + originalXdg = process.env["XDG_CACHE_HOME"]; + process.env["XDG_CACHE_HOME"] = cacheHome; + }); + + afterEach(async () => { + if (originalXdg === undefined) delete process.env["XDG_CACHE_HOME"]; + else process.env["XDG_CACHE_HOME"] = originalXdg; + await fs.rm(root, { recursive: true, force: true }); + await fs.rm(cacheHome, { recursive: true, force: true }); + }); + + it("wipes giget's cached tarball before fetching when noCache is set", async () => { + const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); + if (source.kind !== "git") throw new Error("expected git source"); + + const tarPath = gigetTarballPath(source, undefined); + await fs.mkdir(path.dirname(tarPath), { recursive: true }); + await fs.writeFile(tarPath, "stale-tarball"); + await fs.writeFile(`${tarPath}.json`, JSON.stringify({ etag: "old" })); + + const fetcher = createRepoFetcher({ + projectRoot: root, + cacheDir: path.join(root, ".agnos", "cache"), + }); + await fetcher.fetch(source, { noCache: true }); + + await expect(fs.access(tarPath)).rejects.toThrow(); + await expect(fs.access(`${tarPath}.json`)).rejects.toThrow(); + expect(downloadTemplate).toHaveBeenCalledOnce(); + }); + + it("rejects refs that escape the source's cache directory", async () => { + const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); + if (source.kind !== "git") throw new Error("expected git source"); + + const sentinelDir = path.join(cacheHome, "giget", "github", "other-repo"); + await fs.mkdir(sentinelDir, { recursive: true }); + const sentinel = path.join(sentinelDir, "main.tar.gz"); + await fs.writeFile(sentinel, "sibling-tarball"); + + expect(() => gigetTarballPath(source, "../../other-repo/main")).toThrow(/escapes/); + + const fetcher = createRepoFetcher({ + projectRoot: root, + cacheDir: path.join(root, ".agnos", "cache"), + }); + await expect( + fetcher.fetch(source, { ref: "../../other-repo/main", noCache: true }), + ).rejects.toThrow(/escapes/); + + // The sibling tarball must not have been touched. + const sibling = await fs.readFile(sentinel, "utf8"); + expect(sibling).toBe("sibling-tarball"); + expect(downloadTemplate).not.toHaveBeenCalled(); + }); + + it("leaves giget's cached tarball untouched when noCache is not set", async () => { + const source = parseSource("github:vercel-labs/agent-skills", { projectRoot: root }); + if (source.kind !== "git") throw new Error("expected git source"); + + const tarPath = gigetTarballPath(source, undefined); + await fs.mkdir(path.dirname(tarPath), { recursive: true }); + await fs.writeFile(tarPath, "stale-tarball"); + + const fetcher = createRepoFetcher({ + projectRoot: root, + cacheDir: path.join(root, ".agnos", "cache"), + }); + await fetcher.fetch(source); + + const stillThere = await fs.readFile(tarPath, "utf8"); + expect(stillThere).toBe("stale-tarball"); + }); +}); diff --git a/packages/domain-docs/src/cli/generate.ts b/packages/domain-docs/src/cli/generate.ts index 0e83ac5..82a52bb 100644 --- a/packages/domain-docs/src/cli/generate.ts +++ b/packages/domain-docs/src/cli/generate.ts @@ -114,7 +114,7 @@ function renderIndexBody(docs: DocEntry[]): string { }); const lines: string[] = []; for (const section of sectionOrder) { - lines.push(`## ${section}`); + lines.push(`### ${section}`); for (const d of grouped.get(section) ?? []) { const desc = d.description ? `: ${d.description}` : ""; lines.push(`- [${d.title}](${d.relativeFromRoute})${desc}`); diff --git a/packages/domain-docs/src/cli/inject.ts b/packages/domain-docs/src/cli/inject.ts index 63aa97d..9037cd8 100644 --- a/packages/domain-docs/src/cli/inject.ts +++ b/packages/domain-docs/src/cli/inject.ts @@ -3,8 +3,8 @@ import path from "node:path"; import type { CliCommand, ResolveContext } from "@luxia/core"; import { readConfigOrDefault } from "@luxia/core"; import { readEffectiveDocsConfig, type EffectiveDocsConfig } from "../effective-config.js"; -import { INDEX_BLOCK, RULES_BLOCK } from "../schema.js"; -import { replaceBetweenMarkers, stripFrontmatter } from "../inject/markers.js"; +import { INDEX_HEADING, RULES_HEADING } from "../schema.js"; +import { replaceUnderHeading, stripFrontmatter } from "../inject/markers.js"; export const inject: CliCommand = { description: "Inject doc-rules and index into the project's rules file", @@ -40,14 +40,14 @@ export async function runInject( if (cfg.injectRules) { const payload = await readBodyOrEmpty(cfg.docRulesFile); if (payload !== null) { - const next = replaceBetweenMarkers(text, RULES_BLOCK.start, RULES_BLOCK.end, payload); + const next = replaceUnderHeading(text, RULES_HEADING, payload); text = next.text; } } if (cfg.injectIndex) { const payload = await readBodyOrEmpty(cfg.indexFile); if (payload !== null) { - const next = replaceBetweenMarkers(text, INDEX_BLOCK.start, INDEX_BLOCK.end, payload); + const next = replaceUnderHeading(text, INDEX_HEADING, payload); text = next.text; } } diff --git a/packages/domain-docs/src/inject/markers.ts b/packages/domain-docs/src/inject/markers.ts index fd678dc..73140f6 100644 --- a/packages/domain-docs/src/inject/markers.ts +++ b/packages/domain-docs/src/inject/markers.ts @@ -5,37 +5,41 @@ export interface ReplaceResult { } /** - * Replace content between `startMarker` and `endMarker` lines. - * Markers themselves are preserved verbatim. + * Replace the content under a `## Heading` (or `# Heading`) line. + * The block ends at the next sibling-or-higher heading (`## ` or `# `) or EOF. * - * - If both markers are present and in correct order, replace the lines between them with `payload`. - * - If either marker is missing, append `\n\n` at the end. - * - Equality short-circuit: if the resulting text equals input, returns changed=false. + * - Heading absent: append `\n\n` at end. + * - Heading present: replace lines after the heading up to the boundary with + * `payload`, then a single blank line before the boundary (or EOF). + * - Equality short-circuit: if the resulting text equals input, changed=false. * - * `payload` is inserted verbatim between markers; callers strip leading/trailing whitespace as desired. + * Payload is inserted verbatim; callers strip leading/trailing whitespace as + * desired. */ -export function replaceBetweenMarkers( - text: string, - startMarker: string, - endMarker: string, - payload: string, -): ReplaceResult { +export function replaceUnderHeading(text: string, heading: string, payload: string): ReplaceResult { const lines = text.split(/\r?\n/); - const startIdx = lines.findIndex((line) => line === startMarker); - const endIdx = - startIdx >= 0 ? lines.findIndex((line, i) => i > startIdx && line === endMarker) : -1; + const startIdx = lines.findIndex((line) => line === heading); - if (startIdx < 0 || endIdx < 0) { - const trailingBlank = text.endsWith("\n") ? "" : "\n"; + if (startIdx < 0) { + const trailingNewline = text.endsWith("\n") ? "" : "\n"; const sep = text.length === 0 ? "" : "\n"; - const newText = `${text}${trailingBlank}${sep}${startMarker}\n${payload}\n${endMarker}\n`; + const newText = `${text}${trailingNewline}${sep}${heading}\n${payload}\n`; return { text: newText, changed: true, appended: true }; } + let endIdx = lines.length; + for (let j = startIdx + 1; j < lines.length; j++) { + if (/^#{1,2} /.test(lines[j]!)) { + endIdx = j; + break; + } + } + const before = lines.slice(0, startIdx + 1); const after = lines.slice(endIdx); const payloadLines = payload.split(/\r?\n/); - const next = [...before, ...payloadLines, ...after].join("\n"); + const tail = after.length === 0 ? [] : ["", ...after]; + const next = [...before, ...payloadLines, ...tail].join("\n"); return { text: next, changed: next !== text, appended: false }; } diff --git a/packages/domain-docs/src/schema.ts b/packages/domain-docs/src/schema.ts index e649fdf..c8329ad 100644 --- a/packages/domain-docs/src/schema.ts +++ b/packages/domain-docs/src/schema.ts @@ -39,12 +39,5 @@ export const DEFAULTS = { injectRules: true, }; -export const RULES_BLOCK = { - start: "## Documentation Rules", - end: ">__Documentation rules end__", -}; - -export const INDEX_BLOCK = { - start: "## Documentation Index", - end: ">__Documentation index end__", -}; +export const RULES_HEADING = "## Documentation Rules"; +export const INDEX_HEADING = "## Documentation Index"; diff --git a/packages/domain-docs/test/generate.test.ts b/packages/domain-docs/test/generate.test.ts index 60a6adf..636dee2 100644 --- a/packages/domain-docs/test/generate.test.ts +++ b/packages/domain-docs/test/generate.test.ts @@ -83,8 +83,8 @@ describe("runGenerate", () => { expect(result.docRulesChanged).toBe(true); const indexText = await fs.readFile(cfg.indexFile, "utf8"); - expect(indexText).toContain("## Overview"); - expect(indexText).toContain("## Getting Started"); + expect(indexText).toContain("### Overview"); + expect(indexText).toContain("### Getting Started"); expect(indexText).toContain("[Auth Flow](auth-flow.md): How auth works."); expect(indexText).toContain("[Local setup](getting-started/local.md): Run it locally."); diff --git a/packages/domain-docs/test/inject.test.ts b/packages/domain-docs/test/inject.test.ts index 87fa458..d675244 100644 --- a/packages/domain-docs/test/inject.test.ts +++ b/packages/domain-docs/test/inject.test.ts @@ -82,7 +82,7 @@ describe("runInject gating", () => { ); await fs.writeFile( path.join(root, "AGENTS.md"), - "# AGENTS\n\n## Documentation Rules\n>__Documentation rules end__\n\n## Documentation Index\n>__Documentation index end__\n", + "# AGENTS\n\n## Documentation Rules\n\n## Documentation Index\n\n## Trailing Section\n", ); await fs.mkdir(path.join(root, ".docs"), { recursive: true }); await fs.writeFile(path.join(root, ".docs", "index.md"), "- [foo](foo.md)\n"); @@ -95,5 +95,7 @@ describe("runInject gating", () => { const after = await fs.readFile(path.join(root, "AGENTS.md"), "utf8"); expect(after).toContain("- [foo](foo.md)"); expect(after).toContain("rule line"); + expect(after).not.toContain(">__Documentation"); + expect(after).toContain("## Trailing Section"); }); }); diff --git a/packages/domain-docs/test/markers.test.ts b/packages/domain-docs/test/markers.test.ts index 138c729..9e7ea05 100644 --- a/packages/domain-docs/test/markers.test.ts +++ b/packages/domain-docs/test/markers.test.ts @@ -1,54 +1,74 @@ import { describe, it, expect } from "vitest"; -import { replaceBetweenMarkers, stripFrontmatter } from "../src/inject/markers.js"; +import { replaceUnderHeading, stripFrontmatter } from "../src/inject/markers.js"; -const RULES_START = "## Documentation Rules"; -const RULES_END = ">__Documentation rules end__"; +const HEADING = "## Documentation Rules"; -describe("replaceBetweenMarkers", () => { - it("appends both markers when absent", () => { - const result = replaceBetweenMarkers("hello world\n", RULES_START, RULES_END, "first body"); +describe("replaceUnderHeading", () => { + it("appends the heading + payload when absent", () => { + const result = replaceUnderHeading("hello world\n", HEADING, "first body"); expect(result.appended).toBe(true); expect(result.changed).toBe(true); - expect(result.text).toContain(RULES_START); + expect(result.text).toContain(HEADING); expect(result.text).toContain("first body"); - expect(result.text).toContain(RULES_END); - // payload is between the markers - const a = result.text.indexOf(RULES_START); - const b = result.text.indexOf("first body"); - const c = result.text.indexOf(RULES_END); - expect(a).toBeLessThan(b); - expect(b).toBeLessThan(c); + expect(result.text.indexOf(HEADING)).toBeLessThan(result.text.indexOf("first body")); }); - it("replaces content between existing markers without removing them", () => { - const text = ["Preamble", RULES_START, "old line 1", "old line 2", RULES_END, "Trailer"].join( - "\n", - ); - const result = replaceBetweenMarkers(text, RULES_START, RULES_END, "new body"); + it("replaces content under heading up to the next ## boundary", () => { + const text = [ + "Preamble", + HEADING, + "old line 1", + "old line 2", + "", + "## Next Section", + "Trailer", + ].join("\n"); + const result = replaceUnderHeading(text, HEADING, "new body"); expect(result.appended).toBe(false); expect(result.changed).toBe(true); expect(result.text).toContain("Preamble"); - expect(result.text).toContain(RULES_START); + expect(result.text).toContain(HEADING); expect(result.text).toContain("new body"); - expect(result.text).toContain(RULES_END); + expect(result.text).toContain("## Next Section"); expect(result.text).toContain("Trailer"); expect(result.text).not.toContain("old line 1"); + expect(result.text).not.toContain("old line 2"); + // Single blank line between payload and next heading. + expect(result.text).toContain("new body\n\n## Next Section"); }); - it("idempotent when content already matches", () => { - const text = [RULES_START, "same body", RULES_END].join("\n"); - const result = replaceBetweenMarkers(text, RULES_START, RULES_END, "same body"); - expect(result.changed).toBe(false); - expect(result.text).toBe(text); + it("stops at a `# ` (level-1) heading", () => { + const text = [HEADING, "old", "# Top-Level", "kept"].join("\n"); + const result = replaceUnderHeading(text, HEADING, "fresh"); + expect(result.text).toContain("fresh"); + expect(result.text).not.toContain("old"); + expect(result.text).toContain("# Top-Level"); + expect(result.text).toContain("kept"); }); - it("appends marker pair when only start marker is present", () => { - const text = [RULES_START, "dangling content"].join("\n"); - const result = replaceBetweenMarkers(text, RULES_START, RULES_END, "fresh"); - expect(result.appended).toBe(true); - expect(result.changed).toBe(true); - // the new markers are appended at the end, leaving the dangling start in place - expect(result.text.lastIndexOf(RULES_START)).toBeGreaterThan(result.text.indexOf(RULES_START)); + it("replaces everything to EOF when heading is the last ## section", () => { + const text = [HEADING, "old line", "### sub", "more"].join("\n"); + const result = replaceUnderHeading(text, HEADING, "fresh body"); + expect(result.text).toContain("fresh body"); + expect(result.text).not.toContain("old line"); + expect(result.text).not.toContain("### sub"); + expect(result.text).not.toContain("more"); + }); + + it("preserves nested ### subsections inside the replaced payload", () => { + const text = [HEADING, "old"].join("\n"); + const payload = "### Sub\n- item"; + const result = replaceUnderHeading(text, HEADING, payload); + expect(result.text).toContain("### Sub"); + expect(result.text).toContain("- item"); + expect(result.text).not.toContain("old"); + }); + + it("is idempotent when content already matches", () => { + const text = [HEADING, "same body", "", "## Next", "trail"].join("\n"); + const result = replaceUnderHeading(text, HEADING, "same body"); + expect(result.changed).toBe(false); + expect(result.text).toBe(text); }); });