diff --git a/src/github/repo-doc-pr.ts b/src/github/repo-doc-pr.ts index 6153b1bfab..5917caf00a 100644 --- a/src/github/repo-doc-pr.ts +++ b/src/github/repo-doc-pr.ts @@ -17,12 +17,21 @@ // common case is disabled, so this must be cheap); `allowOverwriteExisting` is checked later, once refresh // reports `manual-review-required` (the "this file looks hand-maintained" signal), and lets that specific case // proceed as a fresh wholesale generate instead of skipping. +// +// SKILL FILE, ADDITIVE (#3001): when `.gittensory.yml repoDocGeneration.scope` includes `"skills"` AND the repo +// profile's contribution workflow warrants one (src/review/repo-skill-render.ts's shouldGenerateRepoSkill), a +// generated skill file rides along in the SAME commit/PR as AGENTS.md/CLAUDE.md -- there is no parallel +// delivery path. It gets its OWN marker pair and its own refreshGeneratedDoc call (reused unchanged, per that +// module's own design intent), so a skill-only content change can still open a PR even when AGENTS.md itself +// is unchanged, and a skill-file conflict (manual-review-required without the overwrite opt-in) only excludes +// the skill from this run rather than blocking the AGENTS.md refresh it rode in with. import { githubErrorStatus, withInstallationTokenRetry } from "./app"; import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; import { getRepository } from "../db/repositories"; import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { extractRepoProfile } from "../review/repo-profile"; import { REPO_DOC_MARKERS, renderRepoDocContent } from "../review/repo-doc-render"; +import { REPO_SKILL_MARKERS, renderRepoSkillContent, repoSkillFilePath } from "../review/repo-skill-render"; import { refreshGeneratedDoc } from "../review/generated-doc-refresh"; import type { AgentActionMode } from "../settings/agent-execution"; @@ -59,12 +68,14 @@ function decodeGitHubFileContent(base64: string): string { return new TextDecoder().decode(bytes); } -/** The current AGENTS.md content on `ref`, or `null` when it doesn't exist yet (first run). Any OTHER failure +/** The current content of `path` on `ref`, or `null` when it doesn't exist yet (first run). Any OTHER failure * (rate limit, auth, a transient 5xx) is rethrown -- a repo we simply couldn't read must never be treated the - * same as a genuinely empty one, or a refresh could mistake "we don't know" for "there's nothing there yet". */ -async function fetchExistingAgentsMdContent(octokit: Octokit, owner: string, repo: string, ref: string): Promise { + * same as a genuinely empty one, or a refresh could mistake "we don't know" for "there's nothing there yet". + * Shared by AGENTS.md and the (optional) skill file -- both are "does this file exist, and what's in it" + * probes against the same Contents API, differing only in path. */ +async function fetchExistingFileContent(octokit: Octokit, owner: string, repo: string, path: string, ref: string): Promise { try { - const response = await octokit.request("GET /repos/{owner}/{repo}/contents/{path}", { owner, repo, path: AGENTS_FILE_PATH, ref }); + const response = await octokit.request("GET /repos/{owner}/{repo}/contents/{path}", { owner, repo, path, ref }); const data = response.data as { content?: string }; return typeof data.content === "string" ? decodeGitHubFileContent(data.content) : null; } catch (error) { @@ -73,30 +84,35 @@ async function fetchExistingAgentsMdContent(octokit: Octokit, owner: string, rep } } -/** Builds the two-file tree (AGENTS.md + CLAUDE.md) atop the branch's current tree in ONE commit, so first-run - * (paths absent) and refresh (paths present) are handled identically -- `base_tree` + explicit per-path entries - * add-or-replace regardless of whether the path previously existed, with no separate "does it exist yet" probe. - * Tries a real symlink (git mode 120000) first; if the target repo/platform rejects that tree, retries with - * CLAUDE.md as a byte-identical regular-file copy of AGENTS.md instead (#3000's own documented fallback). */ -async function buildRepoDocTree(octokit: Octokit, owner: string, repo: string, baseTreeSha: string, agentsContent: string): Promise<{ treeSha: string; claudeMode: "symlink" | "copy" }> { +/** Builds the AGENTS.md + CLAUDE.md tree (plus any `extraEntries`, e.g. a generated skill file, #3001) atop the + * branch's current tree in ONE commit, so first-run (paths absent) and refresh (paths present) are handled + * identically -- `base_tree` + explicit per-path entries add-or-replace regardless of whether the path + * previously existed, with no separate "does it exist yet" probe. Tries a real symlink (git mode 120000) for + * CLAUDE.md first; if the target repo/platform rejects that tree, retries with CLAUDE.md as a byte-identical + * regular-file copy of AGENTS.md instead (#3000's own documented fallback) -- `extraEntries` ride along in + * BOTH attempts unchanged, since the symlink fallback is only ever about CLAUDE.md's own tree entry. */ +async function buildRepoDocTree(octokit: Octokit, owner: string, repo: string, baseTreeSha: string, agentsContent: string, extraEntries: DocTreeEntry[] = []): Promise<{ treeSha: string; claudeMode: "symlink" | "copy" }> { const agentsEntry: DocTreeEntry = { path: AGENTS_FILE_PATH, mode: "100644", type: "blob", content: agentsContent }; try { const symlinkEntry: DocTreeEntry = { path: CLAUDE_FILE_PATH, mode: "120000", type: "blob", content: AGENTS_FILE_PATH }; - const response = await octokit.request("POST /repos/{owner}/{repo}/git/trees", { owner, repo, base_tree: baseTreeSha, tree: [agentsEntry, symlinkEntry] }); + const response = await octokit.request("POST /repos/{owner}/{repo}/git/trees", { owner, repo, base_tree: baseTreeSha, tree: [agentsEntry, symlinkEntry, ...extraEntries] }); return { treeSha: (response.data as { sha: string }).sha, claudeMode: "symlink" }; } catch { const copyEntry: DocTreeEntry = { path: CLAUDE_FILE_PATH, mode: "100644", type: "blob", content: agentsContent }; - const response = await octokit.request("POST /repos/{owner}/{repo}/git/trees", { owner, repo, base_tree: baseTreeSha, tree: [agentsEntry, copyEntry] }); + const response = await octokit.request("POST /repos/{owner}/{repo}/git/trees", { owner, repo, base_tree: baseTreeSha, tree: [agentsEntry, copyEntry, ...extraEntries] }); return { treeSha: (response.data as { sha: string }).sha, claudeMode: "copy" }; } } -function repoDocPullRequestBody(repoFullName: string): string { +function repoDocPullRequestBody(repoFullName: string, skillPath: string | null): string { + const skillParagraph = skillPath + ? `\n\nThis repo's contribution workflow has enough structure (a blocking gate check, a strict linked-issue rule, and/or multi-stage CI) that it also gets a generated skill file at \`${skillPath}\`, following this project's own \`.claude/skills/\` convention -- a frontmatter description plus a procedural body.` + : ""; return `Gittensory opened this pull request on the maintainer's behalf. This is an automated maintenance action, not a manual code review. ## What this is -\`AGENTS.md\`, generated from a profile of ${repoFullName}'s own code -- its indexed file layout, naming and test-file conventions, build/test/lint commands, and contribution-workflow settings (whether CI publishes a required check, the linked-issue policy, and indexed CI workflow files). \`CLAUDE.md\` is kept in sync with it (as a symlink where the platform supports one, otherwise an identical copy), so the two never drift apart. +\`AGENTS.md\`, generated from a profile of ${repoFullName}'s own code -- its indexed file layout, naming and test-file conventions, build/test/lint commands, and contribution-workflow settings (whether CI publishes a required check, the linked-issue policy, and indexed CI workflow files). \`CLAUDE.md\` is kept in sync with it (as a symlink where the platform supports one, otherwise an identical copy), so the two never drift apart.${skillParagraph} ## Why it looks like this @@ -109,14 +125,15 @@ Set \`repoDocGeneration.enabled: false\` in this repository's \`.gittensory.yml\ } /** - * Generate AGENTS.md/CLAUDE.md from this repo's profile and open (or find the already-open) pull request - * carrying them. Returns `{ opened: false, reason }` -- never throws -- when: the repo isn't installed, the repo - * profile has no data yet (#2999's fail-closed branch), `mode` is not `"live"` (dry-run/paused instances must not - * chain several dependent GitHub writes through synthetic suppressed responses -- see `maybeEscalateModeration` - * in `agent-action-executor.ts` for the same "no side effect for a write that didn't really happen" guard on a - * different action), the diff-aware refresh (#3004) found nothing meaningful to change, the existing file's - * marker block is missing/malformed (fails closed rather than guessing), or any step failed partway through. The - * ENTIRE body runs inside one try/catch (not just the GitHub-write chain) so a failure in the repo/profile + * Generate AGENTS.md/CLAUDE.md (and, when warranted and in scope, a skill file -- #3001) from this repo's + * profile and open (or find the already-open) pull request carrying them. Returns `{ opened: false, reason }` + * -- never throws -- when: the repo isn't installed, the repo profile has no data yet (#2999's fail-closed + * branch), `mode` is not `"live"` (dry-run/paused instances must not chain several dependent GitHub writes + * through synthetic suppressed responses -- see `maybeEscalateModeration` in `agent-action-executor.ts` for the + * same "no side effect for a write that didn't really happen" guard on a different action), the diff-aware + * refresh (#3004) found nothing meaningful to change in EITHER AGENTS.md or the skill file, AGENTS.md's own + * marker block is missing/malformed (fails closed rather than guessing), or any step failed partway through. + * The ENTIRE body runs inside one try/catch (not just the GitHub-write chain) so a failure in the repo/profile * lookups themselves is reported the same honest way, rather than propagating as an uncaught exception from * what the rest of the engine treats as a fail-safe call. */ @@ -152,7 +169,7 @@ export async function openRepoDocPullRequest(env: Env, repoFullName: string, mod // actually fell back to a copy. if (existing) return { opened: true, reused: true, pullNumber: existing.number, url: existing.html_url, claudeMode: "unknown" }; - const currentAgentsContent = await fetchExistingAgentsMdContent(octokit, owner, repo, baseBranch); + const currentAgentsContent = await fetchExistingFileContent(octokit, owner, repo, AGENTS_FILE_PATH, baseBranch); let refresh = refreshGeneratedDoc(currentAgentsContent, generatedSection, REPO_DOC_MARKERS); if (refresh.action === "manual-review-required") { // "manual-review-required" is generated-doc-refresh.ts's proxy for "this file looks hand-maintained, @@ -162,14 +179,36 @@ export async function openRepoDocPullRequest(env: Env, repoFullName: string, mod if (!manifest.repoDocGeneration.allowOverwriteExisting) return { opened: false, reason: `AGENTS.md needs manual review before it can be refreshed: ${refresh.reason}` }; refresh = { action: "generate", content: generatedSection }; } - if (refresh.action === "no-change") return { opened: false, reason: "no meaningful change since the last generated AGENTS.md" }; - const agentsContent = refresh.content; + const agentsChanged = refresh.action !== "no-change"; + // refreshGeneratedDoc never returns "no-change" for a null currentContent (that's always "generate"), so + // currentAgentsContent is guaranteed non-null here. + const agentsContent = refresh.action === "no-change" ? currentAgentsContent! : refresh.content; + + // Skill file (#3001): additive to this SAME pull request, never a parallel delivery path. A skill-only + // change can still open a PR even when AGENTS.md itself is unchanged; a skill-file conflict only excludes + // the skill from THIS run (agentsChanged is unaffected), it never blocks the AGENTS.md refresh. + let skillEntry: { path: string; content: string } | null = null; + if (manifest.repoDocGeneration.scope.includes("skills")) { + const generatedSkillSection = renderRepoSkillContent(profile); + if (generatedSkillSection) { + const skillPath = repoSkillFilePath(repoFullName); + const currentSkillContent = await fetchExistingFileContent(octokit, owner, repo, skillPath, baseBranch); + let skillRefresh = refreshGeneratedDoc(currentSkillContent, generatedSkillSection, REPO_SKILL_MARKERS); + if (skillRefresh.action === "manual-review-required" && manifest.repoDocGeneration.allowOverwriteExisting) { + skillRefresh = { action: "generate", content: generatedSkillSection }; + } + if (skillRefresh.action === "replace" || skillRefresh.action === "generate") skillEntry = { path: skillPath, content: skillRefresh.content }; + } + } + + if (!agentsChanged && !skillEntry) return { opened: false, reason: "no meaningful change since the last generated AGENTS.md" }; const branchInfo = await octokit.request("GET /repos/{owner}/{repo}/branches/{branch}", { owner, repo, branch: baseBranch }); const baseCommitSha = branchInfo.data.commit.sha; const baseTreeSha = branchInfo.data.commit.commit.tree.sha; - const { treeSha, claudeMode } = await buildRepoDocTree(octokit, owner, repo, baseTreeSha, agentsContent); + const extraEntries: DocTreeEntry[] = skillEntry ? [{ path: skillEntry.path, mode: "100644", type: "blob", content: skillEntry.content }] : []; + const { treeSha, claudeMode } = await buildRepoDocTree(octokit, owner, repo, baseTreeSha, agentsContent, extraEntries); const commit = await octokit.request("POST /repos/{owner}/{repo}/git/commits", { owner, repo, message: PR_TITLE, tree: treeSha, parents: [baseCommitSha] }); const commitSha = (commit.data as { sha: string }).sha; @@ -180,7 +219,7 @@ export async function openRepoDocPullRequest(env: Env, repoFullName: string, mod owner, repo, title: PR_TITLE, - body: repoDocPullRequestBody(repoFullName), + body: repoDocPullRequestBody(repoFullName, skillEntry?.path ?? null), head: REPO_DOC_BRANCH_NAME, base: baseBranch, maintainer_can_modify: true, diff --git a/src/review/repo-skill-render.ts b/src/review/repo-skill-render.ts new file mode 100644 index 0000000000..a937d2f896 --- /dev/null +++ b/src/review/repo-skill-render.ts @@ -0,0 +1,124 @@ +// Repo-skill content rendering (#3001, part of the repo-doc generation roadmap #2993). Extends AGENTS.md/CLAUDE.md +// generation (#3000) to conditionally propose a Claude Code / Codex skill file -- this repo's own +// `.claude/skills/contributing-to-gittensory/SKILL.md` (frontmatter `name`/`description` + a procedural body) is +// the concrete convention being replicated for OTHER repos. +// +// TRIGGER, NOT UNCONDITIONAL: a skill file is only warranted when a repo's contribution workflow is complex +// enough that folding the whole procedure into AGENTS.md would be unwieldy -- exactly the reason THIS repo has +// one. `shouldGenerateRepoSkill` decides that from RepoProfile's EXISTING fields only (no new signal derivation, +// keeping #2999's extraction primitive generation-agnostic): a blocking gate check, a strict linked-issue rule, +// and multi-stage CI are each named, independently testable sub-checks (mirroring src/signals/slop.ts's +// named-sub-signal style); two or more firing is the trigger, so no single ambiguous signal alone proposes a +// file a maintainer didn't actually need. +import type { RepoProfile, RepoProfileContributionWorkflow } from "./repo-profile"; +import type { GeneratedDocMarkers } from "./generated-doc-refresh"; + +export const REPO_SKILL_MARKER_START = ""; +export const REPO_SKILL_MARKER_END = ""; +export const REPO_SKILL_MARKERS: GeneratedDocMarkers = { start: REPO_SKILL_MARKER_START, end: REPO_SKILL_MARKER_END }; + +function hasBlockingGate(contributionWorkflow: RepoProfileContributionWorkflow): boolean { + return contributionWorkflow.gatePublishesCheck; +} + +/** A repo that both requires a linked issue AND has a policy stricter than "optional" has a real, non-obvious + * admission rule worth writing down -- mirrors the mismatch repo-policy-readiness.ts already treats as notable. */ +function hasStrictLinkedIssueRule(contributionWorkflow: RepoProfileContributionWorkflow): boolean { + return contributionWorkflow.requireLinkedIssue && contributionWorkflow.linkedIssuePolicy !== "optional"; +} + +function hasMultiStageCi(contributionWorkflow: RepoProfileContributionWorkflow): boolean { + return contributionWorkflow.ciWorkflowFiles.length >= 2; +} + +/** + * Whether this repo's contribution workflow is complex enough to warrant a generated skill file. Two or more of + * three named signals (a blocking gate check, a strict linked-issue rule, multi-stage CI) must fire -- a single + * signal alone (e.g. two CI workflow files with no real gate) is common and not, by itself, evidence of a + * non-obvious flow worth documenting. + */ +export function shouldGenerateRepoSkill(profile: Extract): boolean { + const signals = [hasBlockingGate(profile.contributionWorkflow), hasStrictLinkedIssueRule(profile.contributionWorkflow), hasMultiStageCi(profile.contributionWorkflow)]; + return signals.filter(Boolean).length >= 2; +} + +function sanitizeSkillNameSegment(value: string): string { + const sanitized = value + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); + return sanitized || "repo"; +} + +function repoOnlyName(repoFullName: string): string { + const slash = repoFullName.lastIndexOf("/"); + return slash === -1 ? repoFullName : repoFullName.slice(slash + 1); +} + +/** The skill's `name:` frontmatter value -- also the containing directory name, per this repo's own convention + * (`.claude/skills/contributing-to-gittensory/`, directory name === frontmatter `name`). */ +export function repoSkillName(repoFullName: string): string { + return `contributing-to-${sanitizeSkillNameSegment(repoOnlyName(repoFullName))}`; +} + +/** Where the generated skill file is delivered -- `.claude/skills//SKILL.md`, matching the fixed-filename, + * one-directory-per-skill convention this repo already uses. */ +export function repoSkillFilePath(repoFullName: string): string { + return `.claude/skills/${repoSkillName(repoFullName)}/SKILL.md`; +} + +function renderTriggerReasons(contributionWorkflow: RepoProfileContributionWorkflow): string { + const reasons: string[] = []; + if (hasBlockingGate(contributionWorkflow)) reasons.push("- CI publishes a required check before a pull request can merge."); + if (hasStrictLinkedIssueRule(contributionWorkflow)) reasons.push(`- A linked issue is required, with a "${contributionWorkflow.linkedIssuePolicy}" policy.`); + if (hasMultiStageCi(contributionWorkflow)) reasons.push(`- ${contributionWorkflow.ciWorkflowFiles.length} CI workflow files run on a pull request.`); + return reasons.join("\n"); +} + +function renderFrontmatterDescription(repoFullName: string): string { + const repoName = repoOnlyName(repoFullName); + return `Use when writing, testing, or preparing any code contribution or pull request to ${repoFullName}.\n This repo's contribution flow has enough structure that it is worth following exactly. Invoke for any\n "contribute to / open a PR against / fix a bug in / add a feature to ${repoName}" task.`; +} + +/** + * Render the markdown body of a generated skill file from a repo profile, or `null` when either the profile has + * no data (`present: false`) or {@link shouldGenerateRepoSkill} says this repo's workflow doesn't warrant one. + * Callers must treat `null` as "do not generate", not as an empty-but-valid file. The ENTIRE return value is the + * machine-generated section: it both starts and ends with {@link REPO_SKILL_MARKERS}, mirroring + * renderRepoDocContent's convention so refreshGeneratedDoc (src/review/generated-doc-refresh.ts) can recompute + * just this span on a later refresh, unchanged. + */ +export function renderRepoSkillContent(profile: RepoProfile): string | null { + if (!profile.present) return null; + if (!shouldGenerateRepoSkill(profile)) return null; + const { contributionWorkflow, commands } = profile; + const repoName = repoOnlyName(profile.repoFullName); + const skillName = repoSkillName(profile.repoFullName); + const runner = commands.packageManager ?? "npm"; + return `${REPO_SKILL_MARKER_START} +--- +name: ${skillName} +description: >- + ${renderFrontmatterDescription(profile.repoFullName)} +--- + +# Contributing to ${repoName} — the contribution playbook + +This repo's contribution flow has enough structure that it is worth writing down rather than folding into +AGENTS.md: + +${renderTriggerReasons(contributionWorkflow)} + +## Before you push + +- Build: ${commands.buildCommands.length === 0 ? "none detected" : commands.buildCommands.map((name) => `\`${runner} run ${name}\``).join(", ")} +- Test: ${commands.testCommands.length === 0 ? "none detected" : commands.testCommands.map((name) => `\`${runner} run ${name}\``).join(", ")} +- Lint: ${commands.lintCommands.length === 0 ? "none detected" : commands.lintCommands.map((name) => `\`${runner} run ${name}\``).join(", ")} + +## Linked issues + +- Policy: ${contributionWorkflow.linkedIssuePolicy} +- Required: ${contributionWorkflow.requireLinkedIssue ? "yes" : "no"} +${REPO_SKILL_MARKER_END} +`; +} diff --git a/test/unit/repo-doc-pr.test.ts b/test/unit/repo-doc-pr.test.ts index 5fa394a401..79f0d2865a 100644 --- a/test/unit/repo-doc-pr.test.ts +++ b/test/unit/repo-doc-pr.test.ts @@ -1,10 +1,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; import { openRepoDocPullRequest } from "../../src/github/repo-doc-pr"; -import { upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories"; import * as repositoriesModule from "../../src/db/repositories"; import * as repoDocRenderModule from "../../src/review/repo-doc-render"; import { renderRepoDocContent } from "../../src/review/repo-doc-render"; +import { renderRepoSkillContent, repoSkillFilePath } from "../../src/review/repo-skill-render"; import { extractRepoProfile } from "../../src/review/repo-profile"; import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; import { createTestEnv } from "../helpers/d1"; @@ -44,6 +45,16 @@ async function seedRepoDocGenerationConfig(env: ReturnType await upsertRepoFocusManifest(env, repoFullName, { repoDocGeneration: { enabled: true, ...overrides } }); } +// #3001: shouldGenerateRepoSkill needs 2 of 3 named signals. Seeds a strict linked-issue rule (settings + +// manifest) and 2 CI workflow files -- both in the SAME upsertRepoFocusManifest call as repoDocGeneration, since +// a second separate call would replace rather than merge with the first. +async function seedSkillTriggerRepo(env: ReturnType, repoFullName: string, scope: string[] = ["agents", "skills"], overrides: { allowOverwriteExisting?: boolean } = {}): Promise { + await upsertRepositorySettings(env, { repoFullName, requireLinkedIssue: true }); + await upsertRepoFocusManifest(env, repoFullName, { linkedIssuePolicy: "required", repoDocGeneration: { enabled: true, scope, ...overrides } }); + await seedChunk(env, ".github/workflows/ci.yml", "name: CI\non: push\n"); + await seedChunk(env, ".github/workflows/lint.yml", "name: Lint\non: push\n"); +} + // A fetch stub matching every candidate raw-content URL loadRepoFocusManifest's live fetcher tries when there is // no persisted manifest snapshot -- returning a plain 404-shaped failure degrades it to the default (disabled) // manifest, matching fetchRepoFocusManifestFile's own fail-safe "try the next candidate, then give up" behavior. @@ -397,6 +408,200 @@ describe("openRepoDocPullRequest (#3000)", () => { expect(agentsEntry?.content).not.toContain("an older lint command"); }); + it("#3001: does not include a skill file when scope excludes \"skills\", even if the trigger would fire", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedSkillTriggerRepo(env, REPO, ["agents"]); + const calls: Array<{ method: string; url: string; body: Record }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(String(init.body)) : {} }); + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/") && method === "GET") return new Response("not found", { status: 404 }); + if (url.endsWith("/branches/main")) return Response.json({ commit: { sha: "base-commit-sha", commit: { tree: { sha: "base-tree-sha" } } } }); + if (url.endsWith("/git/trees") && method === "POST") return Response.json({ sha: "tree-sha" }); + if (url.endsWith("/git/commits") && method === "POST") return Response.json({ sha: "commit-sha" }); + if (url.endsWith("/git/refs") && method === "POST") return Response.json({}); + if (url.endsWith("/repos/owner/widgets/pulls") && method === "POST") return Response.json({ number: 81, html_url: "https://github.com/owner/widgets/pull/81" }); + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result).toEqual({ opened: true, reused: false, pullNumber: 81, url: "https://github.com/owner/widgets/pull/81", claudeMode: "symlink" }); + const treeCall = calls.find((c) => c.url.endsWith("/git/trees")); + expect((treeCall?.body.tree as Array<{ path: string }>).map((entry) => entry.path)).toEqual(["AGENTS.md", "CLAUDE.md"]); + expect(calls.some((c) => c.url.includes("SKILL.md"))).toBe(false); + }); + + it("#3001: does not include a skill file when scope includes \"skills\" but the trigger condition is not met", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedProfileData(env); + await seedRepoDocGenerationConfig(env, REPO, { scope: ["agents", "skills"] }); + const calls: Array<{ method: string; url: string; body: Record }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(String(init.body)) : {} }); + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/") && method === "GET") return new Response("not found", { status: 404 }); + if (url.endsWith("/branches/main")) return Response.json({ commit: { sha: "base-commit-sha", commit: { tree: { sha: "base-tree-sha" } } } }); + if (url.endsWith("/git/trees") && method === "POST") return Response.json({ sha: "tree-sha" }); + if (url.endsWith("/git/commits") && method === "POST") return Response.json({ sha: "commit-sha" }); + if (url.endsWith("/git/refs") && method === "POST") return Response.json({}); + if (url.endsWith("/repos/owner/widgets/pulls") && method === "POST") return Response.json({ number: 82, html_url: "https://github.com/owner/widgets/pull/82" }); + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result.opened).toBe(true); + const treeCall = calls.find((c) => c.url.endsWith("/git/trees")); + expect((treeCall?.body.tree as Array<{ path: string }>).map((entry) => entry.path)).toEqual(["AGENTS.md", "CLAUDE.md"]); + }); + + it("#3001: adds a first-run skill file to the SAME tree/commit/PR as AGENTS.md when the trigger fires and scope includes \"skills\"", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedSkillTriggerRepo(env, REPO); + const calls: Array<{ method: string; url: string; body: Record }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(String(init.body)) : {} }); + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/") && method === "GET") return new Response("not found", { status: 404 }); + if (url.endsWith("/branches/main")) return Response.json({ commit: { sha: "base-commit-sha", commit: { tree: { sha: "base-tree-sha" } } } }); + if (url.endsWith("/git/trees") && method === "POST") return Response.json({ sha: "tree-sha" }); + if (url.endsWith("/git/commits") && method === "POST") return Response.json({ sha: "commit-sha" }); + if (url.endsWith("/git/refs") && method === "POST") return Response.json({}); + if (url.endsWith("/repos/owner/widgets/pulls") && method === "POST") return Response.json({ number: 83, html_url: "https://github.com/owner/widgets/pull/83" }); + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result).toEqual({ opened: true, reused: false, pullNumber: 83, url: "https://github.com/owner/widgets/pull/83", claudeMode: "symlink" }); + + const treeCall = calls.find((c) => c.url.endsWith("/git/trees")); + const tree = treeCall?.body.tree as Array<{ path: string; mode: string; content: string }>; + expect(tree.map((entry) => entry.path)).toEqual(["AGENTS.md", "CLAUDE.md", repoSkillFilePath(REPO)]); + const skillEntry = tree.find((entry) => entry.path === repoSkillFilePath(REPO)); + expect(skillEntry?.mode).toBe("100644"); + expect(skillEntry?.content).toContain("name: contributing-to-widgets"); + + const prCall = calls.find((c) => c.url.endsWith("/repos/owner/widgets/pulls") && c.method === "POST"); + expect(prCall?.body.body as string).toContain(repoSkillFilePath(REPO)); + }); + + it("#3001: opens a PR for a skill-only change even when AGENTS.md itself is unchanged", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedSkillTriggerRepo(env, REPO); + const profile = await extractRepoProfile(env, REPO); + const currentAgentsContent = renderRepoDocContent(profile)!; + const calls: Array<{ method: string; url: string; body: Record }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(String(init.body)) : {} }); + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/AGENTS.md") && method === "GET") return Response.json({ content: base64Utf8(currentAgentsContent), encoding: "base64" }); + if (url.includes("/contents/") && method === "GET") return new Response("not found", { status: 404 }); + if (url.endsWith("/branches/main")) return Response.json({ commit: { sha: "base-commit-sha", commit: { tree: { sha: "base-tree-sha" } } } }); + if (url.endsWith("/git/trees") && method === "POST") return Response.json({ sha: "tree-sha" }); + if (url.endsWith("/git/commits") && method === "POST") return Response.json({ sha: "commit-sha" }); + if (url.endsWith("/git/refs") && method === "POST") return Response.json({}); + if (url.endsWith("/repos/owner/widgets/pulls") && method === "POST") return Response.json({ number: 84, html_url: "https://github.com/owner/widgets/pull/84" }); + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result.opened).toBe(true); + const treeCall = calls.find((c) => c.url.endsWith("/git/trees")); + const tree = treeCall?.body.tree as Array<{ path: string; content: string }>; + expect(tree.map((entry) => entry.path)).toEqual(["AGENTS.md", "CLAUDE.md", repoSkillFilePath(REPO)]); + // AGENTS.md's own content is reused byte-for-byte (no-change on that side), only the skill file is new. + expect(tree.find((entry) => entry.path === "AGENTS.md")?.content).toBe(currentAgentsContent); + }); + + it("#3001: reports overall no-change when BOTH AGENTS.md and the skill file already match the last generated content", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedSkillTriggerRepo(env, REPO); + const profile = await extractRepoProfile(env, REPO); + const currentAgentsContent = renderRepoDocContent(profile)!; + const currentSkillContent = renderRepoSkillContent(profile)!; + let wroteAnything = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/AGENTS.md") && method === "GET") return Response.json({ content: base64Utf8(currentAgentsContent), encoding: "base64" }); + if (url.includes("/contents/") && method === "GET") return Response.json({ content: base64Utf8(currentSkillContent), encoding: "base64" }); + if (method === "POST") wroteAnything = true; + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result).toEqual({ opened: false, reason: "no meaningful change since the last generated AGENTS.md" }); + expect(wroteAnything).toBe(false); + }); + + it("#3001: excludes the skill file from this run (without failing the AGENTS.md refresh) when it looks hand-maintained and overwrite isn't allowed", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedSkillTriggerRepo(env, REPO); + const calls: Array<{ method: string; url: string; body: Record }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(String(init.body)) : {} }); + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/AGENTS.md") && method === "GET") return new Response("not found", { status: 404 }); + if (url.includes("/contents/") && method === "GET") return Response.json({ content: base64Utf8("# Hand-written skill notes\n\nNo markers here.\n"), encoding: "base64" }); + if (url.endsWith("/branches/main")) return Response.json({ commit: { sha: "base-commit-sha", commit: { tree: { sha: "base-tree-sha" } } } }); + if (url.endsWith("/git/trees") && method === "POST") return Response.json({ sha: "tree-sha" }); + if (url.endsWith("/git/commits") && method === "POST") return Response.json({ sha: "commit-sha" }); + if (url.endsWith("/git/refs") && method === "POST") return Response.json({}); + if (url.endsWith("/repos/owner/widgets/pulls") && method === "POST") return Response.json({ number: 85, html_url: "https://github.com/owner/widgets/pull/85" }); + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result.opened).toBe(true); + const treeCall = calls.find((c) => c.url.endsWith("/git/trees")); + expect((treeCall?.body.tree as Array<{ path: string }>).map((entry) => entry.path)).toEqual(["AGENTS.md", "CLAUDE.md"]); + }); + + it("#3001: overwrites a hand-maintained skill file with a fresh generate when allowOverwriteExisting is set", async () => { + const env = envWithKey(); + await seedInstalledRepo(env, { defaultBranch: "main" }); + await seedSkillTriggerRepo(env, REPO, ["agents", "skills"], { allowOverwriteExisting: true }); + const calls: Array<{ method: string; url: string; body: Record }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (TOKEN_URL.test(url)) return Response.json({ token: "t" }); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(String(init.body)) : {} }); + if (url.includes("/pulls?") && method === "GET") return Response.json([]); + if (url.includes("/contents/AGENTS.md") && method === "GET") return new Response("not found", { status: 404 }); + if (url.includes("/contents/") && method === "GET") return Response.json({ content: base64Utf8("# Hand-written skill notes\n\nNo markers here.\n"), encoding: "base64" }); + if (url.endsWith("/branches/main")) return Response.json({ commit: { sha: "base-commit-sha", commit: { tree: { sha: "base-tree-sha" } } } }); + if (url.endsWith("/git/trees") && method === "POST") return Response.json({ sha: "tree-sha" }); + if (url.endsWith("/git/commits") && method === "POST") return Response.json({ sha: "commit-sha" }); + if (url.endsWith("/git/refs") && method === "POST") return Response.json({}); + if (url.endsWith("/repos/owner/widgets/pulls") && method === "POST") return Response.json({ number: 86, html_url: "https://github.com/owner/widgets/pull/86" }); + return new Response("unexpected", { status: 500 }); + }); + const result = await openRepoDocPullRequest(env, REPO, "live"); + expect(result.opened).toBe(true); + const treeCall = calls.find((c) => c.url.endsWith("/git/trees")); + const tree = treeCall?.body.tree as Array<{ path: string; content: string }>; + const skillEntry = tree.find((entry) => entry.path === repoSkillFilePath(REPO)); + expect(skillEntry?.content).toContain("name: contributing-to-widgets"); + expect(skillEntry?.content).not.toContain("Hand-written skill notes"); + }); + it("reports a caught GitHub Error's message when both the symlink and copy tree attempts fail", async () => { const env = envWithKey(); await seedInstalledRepo(env, { defaultBranch: "main" }); diff --git a/test/unit/repo-skill-render.test.ts b/test/unit/repo-skill-render.test.ts new file mode 100644 index 0000000000..f6e6d19722 --- /dev/null +++ b/test/unit/repo-skill-render.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it } from "vitest"; +import { REPO_SKILL_MARKER_END, REPO_SKILL_MARKER_START, renderRepoSkillContent, repoSkillFilePath, repoSkillName, shouldGenerateRepoSkill } from "../../src/review/repo-skill-render"; +import type { RepoProfile, RepoProfileContributionWorkflow } from "../../src/review/repo-profile"; +import { REPO_PROFILE_SCHEMA_VERSION } from "../../src/review/repo-profile"; + +function contributionWorkflow(overrides: Partial = {}): RepoProfileContributionWorkflow { + return { gatePublishesCheck: false, linkedIssuePolicy: "optional", requireLinkedIssue: false, ciWorkflowFiles: [], ...overrides }; +} + +function presentProfile(overrides: Partial> = {}): RepoProfile { + return { + version: REPO_PROFILE_SCHEMA_VERSION, + present: true, + repoFullName: "owner/widgets", + generatedAt: "2026-07-04T00:00:00.000Z", + architecture: { indexedFileCount: 10, topLevelDirectories: [{ path: "src", fileCount: 10 }] }, + conventions: { fileNamingStyle: "kebab-case", testFileConvention: "dot-test-suffix" }, + commands: { packageManager: "npm", buildCommands: ["build"], testCommands: ["test"], lintCommands: ["lint"] }, + contributionWorkflow: contributionWorkflow(), + ...overrides, + }; +} + +describe("shouldGenerateRepoSkill (#3001)", () => { + it.each([ + ["no signals", contributionWorkflow(), false], + ["gate only", contributionWorkflow({ gatePublishesCheck: true }), false], + ["strict linked issue only", contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "required" }), false], + ["multi-stage CI only", contributionWorkflow({ ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), false], + ["gate + strict linked issue (2 of 3)", contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required" }), true], + ["gate + multi-stage CI (2 of 3)", contributionWorkflow({ gatePublishesCheck: true, ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], + ["strict linked issue + multi-stage CI (2 of 3)", contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "preferred", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], + ["all three signals (3 of 3)", contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], + ])("%s -> %s", (_label, workflow, expected) => { + expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(expected); + }); + + it("requireLinkedIssue alone with an \"optional\" policy does not count as a strict linked-issue rule", () => { + const workflow = contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "optional", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }); + // requireLinkedIssue+optional is a settings/policy mismatch, not a "strict rule" -- only 1 real signal + // (multi-stage CI) fires, below the 2-of-3 threshold. + expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(false); + }); + + it("exactly one CI workflow file does not count as multi-stage", () => { + const workflow = contributionWorkflow({ gatePublishesCheck: true, ciWorkflowFiles: [".github/workflows/ci.yml"] }); + expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(false); + }); +}); + +describe("repoSkillName / repoSkillFilePath (#3001)", () => { + it("derives a lowercase, dash-joined name from the repo segment", () => { + expect(repoSkillName("owner/widgets")).toBe("contributing-to-widgets"); + }); + + it("sanitizes dots, underscores, and mixed case into dashes", () => { + expect(repoSkillName("owner/My_Cool.Repo")).toBe("contributing-to-my-cool-repo"); + }); + + it("falls back to a bare 'repo' segment name when sanitization removes everything", () => { + expect(repoSkillName("owner/___")).toBe("contributing-to-repo"); + }); + + it("handles a repo full name with no slash", () => { + expect(repoSkillName("widgets")).toBe("contributing-to-widgets"); + }); + + it("builds the fixed .claude/skills//SKILL.md path", () => { + expect(repoSkillFilePath("owner/widgets")).toBe(".claude/skills/contributing-to-widgets/SKILL.md"); + }); +}); + +describe("renderRepoSkillContent (#3001)", () => { + it("renders null for an absent profile", () => { + const profile: RepoProfile = { version: REPO_PROFILE_SCHEMA_VERSION, present: false, repoFullName: "owner/widgets", generatedAt: "now", reason: "no RAG index configured or populated for this repo yet" }; + expect(renderRepoSkillContent(profile)).toBeNull(); + }); + + it("renders null when the trigger condition is not met", () => { + expect(renderRepoSkillContent(presentProfile())).toBeNull(); + }); + + it("renders the marker, frontmatter, trigger reasons, and command/linked-issue sections when the trigger fires", () => { + const profile = presentProfile({ + repoFullName: "owner/widgets", + contributionWorkflow: contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), + }); + const content = renderRepoSkillContent(profile); + expect(content).not.toBeNull(); + expect(content!.startsWith(REPO_SKILL_MARKER_START)).toBe(true); + expect(content!.trimEnd().endsWith(REPO_SKILL_MARKER_END)).toBe(true); + expect(content).toContain("name: contributing-to-widgets"); + expect(content).toContain("# Contributing to widgets"); + expect(content).toContain("CI publishes a required check before a pull request can merge."); + expect(content).toContain('A linked issue is required, with a "required" policy.'); + expect(content).toContain("2 CI workflow files run on a pull request."); + expect(content).toContain("Build: `npm run build`"); + expect(content).toContain("Test: `npm run test`"); + expect(content).toContain("Lint: `npm run lint`"); + expect(content).toContain("Policy: required"); + expect(content).toContain("Required: yes"); + }); + + it("omits the gate-check reason line when the trigger fires via linked-issue + multi-stage CI alone (no blocking gate)", () => { + const profile = presentProfile({ + contributionWorkflow: contributionWorkflow({ gatePublishesCheck: false, requireLinkedIssue: true, linkedIssuePolicy: "preferred", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), + }); + const content = renderRepoSkillContent(profile); + expect(content).not.toBeNull(); + expect(content).not.toContain("CI publishes a required check"); + expect(content).toContain('A linked issue is required, with a "preferred" policy.'); + expect(content).toContain("2 CI workflow files run on a pull request."); + }); + + it("omits the multi-stage-CI reason line when the trigger fires via gate + strict linked issue alone (single CI file)", () => { + const profile = presentProfile({ + contributionWorkflow: contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", ciWorkflowFiles: [".github/workflows/ci.yml"] }), + }); + const content = renderRepoSkillContent(profile); + expect(content).not.toBeNull(); + expect(content).toContain("CI publishes a required check before a pull request can merge."); + expect(content).toContain('A linked issue is required, with a "required" policy.'); + expect(content).not.toContain("CI workflow files run on a pull request."); + }); + + it("degrades commands to 'none detected' and uses npm as the default runner when no package manager is known", () => { + const profile = presentProfile({ + contributionWorkflow: contributionWorkflow({ gatePublishesCheck: true, ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), + commands: { packageManager: null, buildCommands: [], testCommands: [], lintCommands: [] }, + }); + const content = renderRepoSkillContent(profile); + expect(content).toContain("Build: none detected"); + expect(content).toContain("Test: none detected"); + expect(content).toContain("Lint: none detected"); + }); + + it("renders byte-identical output for the same profile facts regardless of generatedAt", () => { + const workflow = contributionWorkflow({ gatePublishesCheck: true, ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }); + const a = renderRepoSkillContent(presentProfile({ contributionWorkflow: workflow, generatedAt: "2026-01-01T00:00:00.000Z" })); + const b = renderRepoSkillContent(presentProfile({ contributionWorkflow: workflow, generatedAt: "2026-12-31T23:59:59.000Z" })); + expect(a).toEqual(b); + }); +});