Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/pr-pipeline/feat-8956-docs-patch-row-3e7a91c4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: minor
---

### Added

- `patch-rules.json` has a `docs` row for a markdown-only patch. It allows only a path ending in `.md` (the new `markdown` allow rule). Besides the universal set it refuses a path under a `skills/` directory at any depth and a `SKILL.md` anywhere (the new `skills` and `skill-md` refusals, which apply only to a row that names them), each escalating `patch-protected-path`. Its cap, commit, moved-head path and reasons match `default`. No push job selects it yet: fix-docs's `apply-docs-patch` job keeps the `default` row until `pr-refine.yml` passes `patch-prepare: docs`.
10 changes: 10 additions & 0 deletions .github/actions/resolve-config/patch-rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@
"conflict-patch-protected-path"
]
},
"docs": {
"refuse": ["github-workflows", "codeowners", "gitmodules", "settings", "symlink", "gitlink", "rename-into-refused", "skills", "skill-md"],
"allow": ["markdown"],
"max-files": 300,
"requires": [],
"commit": { "parents": ["head"], "message": "Apply {activity} on {ref}", "trailer": "Lane-Activity: {lane}/{activity}" },
"reads-report": null,
"skip-moved-head": false,
"escalations": ["patch-empty", "patch-invalid", "patch-not-applied", "patch-protected-path"]
},
"default": {
"refuse": ["github-workflows", "codeowners", "gitmodules", "settings", "symlink", "gitlink", "rename-into-refused"],
"allow": ["any"],
Expand Down
13 changes: 8 additions & 5 deletions .github/actions/resolve-config/patch-rules.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://github.com/melodic-software/claude-code-plugins/.github/actions/resolve-config/patch-rules.schema.json",
"title": "Patch rules",
"description": "What the apply-patch script is to admit from a producer's patch: one row per prepare mode a patch-producing skill uses, `default` for one with no prepare mode, and `issue` for a patch that goes to a new issue-lane branch. Contract: docs/conventions/pr-pipeline/README.md, Patch rules.",
"description": "What the apply-patch script is to admit from a producer's patch: one row per prepare mode a patch-producing skill uses, `docs` for a markdown-only patch, `default` for one with no prepare mode, and `issue` for a patch that goes to a new issue-lane branch. Contract: docs/conventions/pr-pipeline/README.md, Patch rules.",
"type": "object",
"additionalProperties": false,
"required": ["default", "issue"],
Expand All @@ -11,6 +11,7 @@
"review-threads": { "$ref": "#/$defs/row" },
"dependency-review": { "$ref": "#/$defs/row" },
"merge-base": { "$ref": "#/$defs/row" },
"docs": { "$ref": "#/$defs/row" },
"default": { "$ref": "#/$defs/row" },
"issue": { "$ref": "#/$defs/row" }
},
Expand All @@ -32,17 +33,19 @@
"settings",
"symlink",
"gitlink",
"rename-into-refused"
"rename-into-refused",
"skills",
"skill-md"
]
},
"description": "Named refusals; any match refuses the whole patch. github-workflows: .github/workflows/** in any case and through any .. segment; codeowners: CODEOWNERS at the root, in .github/ or in docs/; gitmodules: .gitmodules; settings: **/settings*.json outside plugins/; symlink: mode 120000; gitlink: mode 160000; rename-into-refused: a rename whose new path another refusal matches."
"description": "Named refusals; any match refuses the whole patch. github-workflows: .github/workflows/** in any case and through any .. segment; codeowners: CODEOWNERS at the root, in .github/ or in docs/; gitmodules: .gitmodules; settings: **/settings*.json outside plugins/; symlink: mode 120000; gitlink: mode 160000; rename-into-refused: a rename whose new path github-workflows, codeowners, gitmodules or settings matches, or that holds a backslash. Optional, refused only where a row names them, in any case and through any .. segment: skills: a path under a skills/ directory at any depth; skill-md: a SKILL.md at any depth."
},
"allow": {
"type": "array",
"minItems": 1,
"uniqueItems": true,
"items": { "enum": ["any", "kept-threads", "dependency-manifests", "merge-result"] },
"description": "What a path the refusals pass must also meet. any: nothing more; kept-threads: the path of a thread the run kept; dependency-manifests: an npm or pip manifest or lock file outside .github/; merge-result: a path the base merge left conflicted, which the patch must leave with no conflict markers; the merge's own changes from the base side pass through without facing the rules."
"items": { "enum": ["any", "markdown", "kept-threads", "dependency-manifests", "merge-result"] },
"description": "What a path the refusals pass must also meet. any: nothing more; markdown: a path ending in .md, in that case; kept-threads: the path of a thread the run kept; dependency-manifests: an npm or pip manifest or lock file outside .github/; merge-result: a path the base merge left conflicted, which the patch must leave with no conflict markers; the merge's own changes from the base side pass through without facing the rules."
},
"max-files": {
"oneOf": [{ "type": "integer", "minimum": 1 }, { "type": "null" }],
Expand Down
3 changes: 2 additions & 1 deletion .github/actions/resolve-config/resolve.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1789,12 +1789,13 @@ const PATCH_RULES = JSON.parse(read("./patch-rules.json"));
const PATCH_RULES_SCHEMA = JSON.parse(read("./patch-rules.schema.json"));
const patchRules = (name) => JSON.parse(fixture(name));

test("patch rules: the shipped registry passes, with a row for each prepare mode and the default and issue targets", () => {
test("patch rules: the shipped registry passes, with a row for each prepare mode, the docs row and the default and issue targets", () => {
assert.equal(checkPatchRules(PATCH_RULES, PATCH_RULES_SCHEMA), PATCH_RULES);
assert.deepEqual(Object.keys(PATCH_RULES).sort(), [
"ci-context",
"default",
"dependency-review",
"docs",
"issue",
"merge-base",
"review-threads",
Expand Down
25 changes: 19 additions & 6 deletions .github/scripts/apply-patch/apply-patch.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ import { pathToFileURL } from "node:url";
import { BODY_HOLD, LANES_APP_LOGIN } from "../pr-merge/merge-holds.mjs";
import { fixRefusal, parseConfig } from "../pr-unblock/dependency-classify.mjs";
import { PREDICATES } from "./predicates.mjs";
import { PROTECTED_REFUSALS } from "./protected-paths.mjs";
import { PROTECTED_REFUSALS, segments } from "./protected-paths.mjs";
import { retryFetch } from "./retry-fetch.mjs";

export { segments } from "./protected-paths.mjs";
Expand All @@ -89,9 +89,19 @@ const REGULAR = "100644";
const EXECUTABLE = "100755";
const ZERO = /^0+$/;

// Path refusals a row may add to the universal set, in any case and through
// any .. segment; rename-into-refused does not read them.
const ROW_PATH_RULES = {
// A path under a skills/ directory at any depth.
skills: (s) => s.slice(0, -1).includes("skills"),
"skill-md": (s) => s.at(-1) === "skill.md",
};
const rowPathRule = (name) => (entry) => ROW_PATH_RULES[name](segments(entry.path));

// The refusal table. Every row must name every refusal but those in ALWAYS,
// which apply to every row unnamed; any match refuses the whole patch, and
// `kind` picks the row's escalation reason.
// which apply to every row unnamed, and the `optional` ones, which apply only
// to a row that names them; any match refuses the whole patch, and `kind`
// picks the row's escalation reason.
export const ALWAYS = ["backslash"];
export const REFUSALS = {
backslash: PROTECTED_REFUSALS.backslash,
Expand All @@ -102,10 +112,13 @@ export const REFUSALS = {
symlink: { kind: "invalid", test: (entry) => entry.oldMode === SYMLINK || entry.newMode === SYMLINK },
gitlink: { kind: "invalid", test: (entry) => entry.oldMode === GITLINK || entry.newMode === GITLINK },
"rename-into-refused": PROTECTED_REFUSALS["rename-into-refused"],
skills: { kind: "protected-path", optional: true, test: rowPathRule("skills") },
"skill-md": { kind: "protected-path", optional: true, test: rowPathRule("skill-md") },
};

// The names of the refusals that match one changed entry; by default all.
export const refusalsOf = (entry, names = Object.keys(REFUSALS)) => names.filter((name) => REFUSALS[name].test(entry));
// The names of the refusals that match one changed entry; by default all but
// the optional ones, which apply only to a row that names them.
export const refusalsOf = (entry, names = Object.keys(REFUSALS).filter((name) => !REFUSALS[name].optional)) => names.filter((name) => REFUSALS[name].test(entry));

// Credential shapes: any match in the patch or a patched blob refuses it all.
// GitHub prefixes from
Expand Down Expand Up @@ -273,7 +286,7 @@ export function checkRow(row) {
const fail = (message) => {
throw new Error(`patch-rules.json row \`${row.name}\` ${message}`);
};
const missing = Object.keys(REFUSALS).filter((name) => !ALWAYS.includes(name) && !row.refuse?.includes(name));
const missing = Object.keys(REFUSALS).filter((name) => !ALWAYS.includes(name) && !REFUSALS[name].optional && !row.refuse?.includes(name));
if (missing.length > 0) {
fail(`does not refuse ${missing.join(", ")}`);
}
Expand Down
114 changes: 111 additions & 3 deletions .github/scripts/apply-patch/apply-patch.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ test("a patch over max-files is refused", async (t) => {
});

// Agent instructions, plugin hooks and manifests, MCP config and .claude
// reach review on every row; settings files, submodules and code owners stay
// refused on every row.
// reach review on every row past its optional refusals; settings files,
// submodules and code owners stay refused on every row.
const INSTRUCTION_PATHS = [
["CLAUDE.md", []],
["AGENTS.md", []],
Expand All @@ -374,11 +374,119 @@ const INSTRUCTION_PATHS = [
test("every row refuses the same instruction paths: settings, submodules and code owners only", () => {
for (const [path, refusals] of INSTRUCTION_PATHS) {
for (const name of Object.keys(RULES)) {
assert.deepEqual(refusalsOf(entry(path), RULES[name].refuse), refusals, `${name} ${path}`);
const universal = RULES[name].refuse.filter((refusal) => !REFUSALS[refusal].optional);
assert.deepEqual(refusalsOf(entry(path), universal), refusals, `${name} ${path}`);
}
}
});

// --- the docs row: markdown only, nothing under skills/ and no SKILL.md ---

test("refuses a path under a skills/ directory at any depth, in any case, and a SKILL.md anywhere, only where a row names them", () => {
for (const [path, refusals] of [
["plugins/p/skills/s/SKILL.md", ["skills", "skill-md"]],
["plugins/p/skills/s/reference/r.md", ["skills"]],
["skills/x.md", ["skills"]],
["plugins/p/Skills/s/notes.md", ["skills"]],
["a/../skills/x.md", ["skills"]],
["docs/skill.md", ["skill-md"]],
["SKILL.md", ["skill-md"]],
["docs/skills.md", []],
["plugins/p/skillset/x.md", []],
]) {
assert.deepEqual(refusalsOf(entry(path), ["skills", "skill-md"]), refusals, path);
assert.deepEqual(refusalsOf(entry(path), RULES.default.refuse), [], `default ${path}`);
}
});

test("only the docs row names the skills and skill-md refusals, and no row has to", () => {
for (const name of Object.keys(RULES)) {
const named = RULES[name].refuse.filter((refusal) => REFUSALS[refusal].optional);
assert.deepEqual(named, name === "docs" ? ["skills", "skill-md"] : [], name);
}
const refuse = RULES.docs.refuse.filter((name) => !REFUSALS[name].optional);
assert.doesNotThrow(() => checkRow(row("docs", { refuse })));
assert.throws(() => checkRow(row("docs", { refuse: refuse.filter((name) => name !== "codeowners") })), /does not refuse codeowners/);
});

test("the docs row allows markdown alone, refuses the universal set, and keeps the default row's cap, commit, moved-head path and reasons", () => {
const docs = row("docs");
const fallback = row("default");
assert.deepEqual(docs.allow, ["markdown"]);
assert.deepEqual(
docs.refuse.filter((name) => !REFUSALS[name].optional),
fallback.refuse,
);
for (const key of ["max-files", "requires", "commit", "reads-report", "skip-moved-head", "escalations"]) {
assert.deepEqual(docs[key], fallback[key], key);
}
});

const docsPush = (fix, api, extra = {}) => pushWith(fix, api, { row: row("docs"), lane: "pr-refine", activity: "fix-docs", ...extra });

test("under the docs row a patch that edits only markdown outside skills/ pushes", async (t) => {
const fix = repoWithPatch(t, (repo) => {
writeFileSync(join(repo, "README.md"), "# fixed\n");
mkdirSync(join(repo, "docs"));
writeFileSync(join(repo, "docs/AGENTS.md"), "x\n");
});
const api = fakeApi(fix.repo, fix.head);
assert.deepEqual(await docsPush(fix, api), { outcome: "pushed", commit: "c".repeat(40), branch: "feat/x" });
const { createCommit } = api.calls.find((call) => call.createCommit);
assert.equal(createCommit.message, "Apply fix-docs on feat/x\n\nLane-Activity: pr-refine/fix-docs\n");
});

for (const [path, edit] of [
["scripts/x.sh", (repo) => writeFileSync(join(repo, "scripts/x.sh"), "#!/bin/sh\nfoo=2\n")],
["config.json", (repo) => writeFileSync(join(repo, "config.json"), "{}\n")],
["lib/x.mjs", (repo) => (mkdirSync(join(repo, "lib")), writeFileSync(join(repo, "lib/x.mjs"), "export {};\n"))],
["GUIDE.MD", (repo) => writeFileSync(join(repo, "GUIDE.MD"), "x\n")],
]) {
test(`under the docs row a patch that also edits the non-markdown ${path} is refused patch-protected-path before any API call`, async (t) => {
const fix = repoWithPatch(t, (repo) => {
writeFileSync(join(repo, "README.md"), "# fixed\n");
edit(repo);
});
const api = fakeApi(fix.repo, fix.head);
const lines = [];
assert.deepEqual(await docsPush(fix, api, { log: (line) => lines.push(line) }), { outcome: "escalate", escalationReason: "patch-protected-path" });
assert.deepEqual(lines, [`not allowed by markdown: ${path}`]);
assert.deepEqual(api.calls, []);
});
}

for (const [path, refusal] of [
["plugins/p/skills/s/SKILL.md", "skills"],
["plugins/p/skills/s/reference/r.md", "skills"],
["docs/SKILL.md", "skill-md"],
]) {
test(`under the docs row a patch that touches ${path} is refused patch-protected-path before any API call`, async (t) => {
const fix = repoWithPatch(t, (repo) => {
writeFileSync(join(repo, "README.md"), "# fixed\n");
mkdirSync(dirname(join(repo, path)), { recursive: true });
writeFileSync(join(repo, path), "# changed\n");
});
const api = fakeApi(fix.repo, fix.head);
const lines = [];
assert.deepEqual(await docsPush(fix, api, { log: (line) => lines.push(line) }), { outcome: "escalate", escalationReason: "patch-protected-path" });
assert.deepEqual(lines, [`refused (${refusal}): ${path}`]);
assert.deepEqual(api.calls, []);
});
}

test("under the docs row deleting a SKILL.md is refused patch-protected-path before any API call", async (t) => {
const path = "plugins/p/skills/s/SKILL.md";
const fix = repoWithPatch(t, (repo) => rmSync(join(repo, path)), {
head: (repo) => {
mkdirSync(dirname(join(repo, path)), { recursive: true });
writeFileSync(join(repo, path), "# skill\n");
},
});
const api = fakeApi(fix.repo, fix.head);
assert.deepEqual(await docsPush(fix, api), { outcome: "escalate", escalationReason: "patch-protected-path" });
assert.deepEqual(api.calls, []);
});

// fix-docs hands on its patch with no prepare mode, so the default row holds
// it: an edit to an agent instruction or a plugin hook pushes, and one to a
// code owners file is refused whole before any API call.
Expand Down
2 changes: 2 additions & 0 deletions .github/scripts/apply-patch/predicates.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { inThreadScope } from "../pr-unblock/thread-scope.mjs";

export const PREDICATES = {
any: () => () => true,
// A markdown file: a path ending in .md, in that case.
markdown: () => (path) => path.endsWith(".md"),
// A kept thread's path, or a file thread-scope.mjs places in its scope,
// its named_paths and the PR's own files the patch does not add included:
// `keptThreads` is { paths, threads, prPaths }.
Expand Down
Loading
Loading