diff --git a/.changes/pr-pipeline/fix-admit-knowable-plugin-arm-scope-0ea8d215.md b/.changes/pr-pipeline/fix-admit-knowable-plugin-arm-scope-0ea8d215.md new file mode 100644 index 0000000000..eed9123b51 --- /dev/null +++ b/.changes/pr-pipeline/fix-admit-knowable-plugin-arm-scope-0ea8d215.md @@ -0,0 +1,11 @@ +--- +bump: minor +--- + +### Changed + +- Intake admission also takes issues whose Uncertainty is Knowable (`admission-rules.json`), which the run lane already runs research-first (owner decision 2026-10-24, amending Q51 criterion 3). + +### Fixed + +- The address-feedback scope rule (`thread-scope.mjs`) no longer lets a fix edit a plugin's control or instruction files through the plugin-wide arm; it excludes the same control paths the pull-request-files arm does. diff --git a/.changes/source-control/fix-admit-knowable-plugin-arm-scope-df25da1a.md b/.changes/source-control/fix-admit-knowable-plugin-arm-scope-df25da1a.md new file mode 100644 index 0000000000..0913a80e82 --- /dev/null +++ b/.changes/source-control/fix-admit-knowable-plugin-arm-scope-df25da1a.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- `feedback-thread.sh check-paths` and `record` no longer let a lane review-fix edit a plugin's control or instruction files (`.claude-plugin/`, `hooks/`, `monitors/`, `bin/`, `.mcp.json`, `.lsp.json`, `AGENTS.md`, `CLAUDE.md`, `.claude/`) through the plugin-wide scope of a thread on another file in that plugin. The thread's own path and named paths are unchanged. diff --git a/.github/actions/resolve-config/admission-rules.json b/.github/actions/resolve-config/admission-rules.json index 4dbb318ee9..f0c14a8ac3 100644 --- a/.github/actions/resolve-config/admission-rules.json +++ b/.github/actions/resolve-config/admission-rules.json @@ -4,7 +4,7 @@ "types-under-parent": ["Feature"], "classes": ["work-class: read-only", "work-class: mechanical", "work-class: scoped"], "sizes": ["small", "medium"], - "uncertainty": ["known"], + "uncertainty": ["known", "knowable"], "refuse-labels": ["duplicate", "needs-owner-decision", "do-not-merge", "wontfix"], "task-list": "- [ ] " } diff --git a/.github/scripts/intake-triage/admit.test.mjs b/.github/scripts/intake-triage/admit.test.mjs index 8156cba456..eda589abd6 100644 --- a/.github/scripts/intake-triage/admit.test.mjs +++ b/.github/scripts/intake-triage/admit.test.mjs @@ -60,6 +60,11 @@ describe("decide", () => { assert.deepEqual(decide(facts()), { verdict: "admit", reasons: [] }); }); + test("admits Uncertainty Knowable, which the lane researches first", () => { + const verdict = { ...VERDICT, dimensions: { ...VERDICT.dimensions, uncertainty: "knowable" } }; + assert.deepEqual(decide(facts({ verdict })), { verdict: "admit", reasons: [] }); + }); + const refusals = [ ["criterion 2: a type the rules do not list", { issue: issueOf({ type: { name: "Epic" } }) }, /type Epic/], ["criterion 2: an issue with no type", { issue: issueOf({ type: null }) }, /type none/], @@ -85,14 +90,14 @@ describe("decide", () => { /parent/, ], [ - "criterion 3: Uncertainty Knowable", + "criterion 3: Uncertainty Unknown", { verdict: { ...VERDICT, - dimensions: { ...VERDICT.dimensions, uncertainty: "knowable" }, + dimensions: { ...VERDICT.dimensions, uncertainty: "unknown" }, }, }, - /Uncertainty knowable/, + /Uncertainty unknown/, ], [ "criterion 3: Size Large", diff --git a/.github/scripts/pr-unblock/thread-scope.mjs b/.github/scripts/pr-unblock/thread-scope.mjs index c0d3d8fc10..a27a752087 100644 --- a/.github/scripts/pr-unblock/thread-scope.mjs +++ b/.github/scripts/pr-unblock/thread-scope.mjs @@ -73,8 +73,8 @@ export function namedInScope(path, threadPath) { } // A kept thread's own path, a file in the plugins/

/ directory a kept -// thread's path sits in that is no instruction path and no script or code -// file, a test file in the same directory as a kept thread's path, +// thread's path sits in that is no instruction path, no control-plane or +// agent-instruction surface and no script or code file, a test file in the same directory as a kept thread's path, // a path in a kept thread's named_paths (in `threads`, each { path, // named_paths }) that namedInScope allows for that thread, or a path the pull // request already changes (`prPaths`, prPathsOf's) that the fix does not add @@ -96,7 +96,13 @@ export function inThreadScope(path, threadPaths, threads = [], { prPaths = new S } const kept = [...threadPaths]; const plugin = pluginOf(path); - if (plugin !== undefined && !CODE_FILE.test(path) && !isInstructionPath(path) && kept.some((p) => p.startsWith(plugin))) { + if ( + plugin !== undefined && + !CODE_FILE.test(path) && + !isInstructionPath(path) && + !CONTROL_PATH.test(path) && + kept.some((p) => p.startsWith(plugin)) + ) { return true; } const dir = dirOf(path); diff --git a/.github/scripts/pr-unblock/thread-scope.test.mjs b/.github/scripts/pr-unblock/thread-scope.test.mjs index 01c0eecbd2..fd7d73efb0 100644 --- a/.github/scripts/pr-unblock/thread-scope.test.mjs +++ b/.github/scripts/pr-unblock/thread-scope.test.mjs @@ -60,7 +60,7 @@ test("permission config and non-test code in the same plugin are refused", () => } }); -test("hooks, manifests, agent rules, monitors, bin and MCP config in the same plugin are in scope", () => { +test("hooks, manifests, agent rules, monitors, bin and MCP config in the same plugin are out of scope", () => { const named = new Set(["plugins/p/skills/s/SKILL.md"]); for (const path of [ "plugins/p/hooks/hooks.json", @@ -73,7 +73,7 @@ test("hooks, manifests, agent rules, monitors, bin and MCP config in the same pl "plugins/p/.mcp.json", "plugins/p/.lsp.json", ]) { - assert.equal(inThreadScope(path, named), true, path); + assert.equal(inThreadScope(path, named), false, path); } }); diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh index 2d220c10bf..71584f2b3c 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh @@ -47,8 +47,9 @@ scratch_dir=${FEEDBACK_SCRATCH:-} workspace=${FEEDBACK_WORKSPACE:-} # Not under .github/workflows/ in any letter case, with no empty, . or .. segment, of # plain ASCII segments, and in a kept thread's scope: its path, a file in the # plugins/

/ directory it sits in that is no instruction path (.gitmodules, -# CODEOWNERS, a settings*.json, a package manifest or lockfile) and no script -# or code file, a test file in its directory, or a path in its named_paths +# CODEOWNERS, a settings*.json, a package manifest or lockfile), no control +# surface (CLAUDE.md, AGENTS.md, .mcp.json, .lsp.json, .claude-plugin/, hooks/, +# monitors/, bin/) and no script or code file, a test file in its directory, or a path in its named_paths # (the files a write-access fix request named) in that same plugins/

/ # directory that is no instruction path; or, when the gated head holds it as a # regular file, a file in the thread file's pr_paths (the files the pull request already @@ -67,7 +68,8 @@ allowed() { def plugin: first(capture("^(?plugins/[^/]+/)").c) // null; ($p | test("^[A-Za-z0-9_@+.-]+(/[A-Za-z0-9_@+.-]+)*$")) as $plain | ($p | test("(^|/)(\\.gitmodules|codeowners|settings[^/]*\\.json|package(-lock)?\\.json|requirements[^/]*\\.txt)$"; "i")) as $instruction - | (if $instruction or ($p | test("\\.(sh|bash|zsh|ps1|py|mjs|cjs|js|ts|mts|cts)$"; "i")) + | ($p | test("^\\.github/|(^|/)(CLAUDE\\.md|AGENTS\\.md|\\.mcp\\.json|\\.lsp\\.json)$|(^|/)\\.claude(-plugin)?/|^plugins/[^/]+/(hooks|monitors|bin)/"; "i")) as $control + | (if $instruction or $control or ($p | test("\\.(sh|bash|zsh|ps1|py|mjs|cjs|js|ts|mts|cts)$"; "i")) then null else $p | plugin end) as $plugin | ($p | sub("^.*/"; "") | test("^(test_[^/]*\\.py|[^/]*_test\\.py|[^/]*\\.(test|spec)\\.[^/]+)$")) as $test | (if $instruction then null else $p | plugin end) as $named diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh index cecafc4730..2b283d2861 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh @@ -110,13 +110,13 @@ jq '.threads += [{"id":"PRRT_e","path":"plugins/p/evals/c/case.yaml","root_comme "$STUB/threads.json" >"$STUB/t2" && mv "$STUB/t2" "$STUB/threads.json" for path in plugins/p/evals/c/samples/y.json plugins/p/evals/c/samples/deep/z.json \ plugins/p/evals/other/samples/y.json plugins/p/skills/s/SKILL.md plugins/p/evals/x.json \ - plugins/p/hooks/hooks.json plugins/p/.claude-plugin/plugin.json plugins/p/AGENTS.md \ - plugins/p/skills/s/CLAUDE.md plugins/p/.claude/rules/r.md \ plugins/planning/README.md plugins/planning/surface/test_round.py plugins/planning/surface/round_test.py src/a.test.js src/a.spec.ts; do run check-paths "$path" >/dev/null 2>&1 assert_exit "check-paths allows $path" 0 "$?" done for path in plugins/q/evals/c/samples/y.json plugins/pp/README.md plugins/README.md \ + plugins/p/hooks/hooks.json plugins/p/.claude-plugin/plugin.json plugins/p/AGENTS.md \ + plugins/p/skills/s/CLAUDE.md plugins/p/.claude/rules/r.md \ plugins/p/scripts/run.sh plugins/p/.claude/settings.json plugins/p/package.json plugins/p/lib/X.TS \ plugins/planning/surface/helper.py plugins/planning/test_round.py \ src/lib/a.test.js src/helper.js src/test_a.sh test_root.py \ @@ -180,11 +180,12 @@ for path in plugins/other/x.js .github/workflows/x.yml \ done # check-paths: more code files, package manifests and non-ASCII letters stay out of the -# plugin-wide scope; plugin monitors, bin and .lsp.json are in it -for path in plugins/knowledge/README.md plugins/knowledge/monitors/monitors.json plugins/knowledge/bin/tool \ - plugins/knowledge/.lsp.json; do +# plugin-wide scope, and so do plugin monitors, bin and .lsp.json (control surfaces) +run check-paths plugins/knowledge/README.md >/dev/null 2>&1 +assert_exit "check-paths allows plugins/knowledge/README.md in the named thread's plugin" 0 "$?" +for path in plugins/knowledge/monitors/monitors.json plugins/knowledge/bin/tool plugins/knowledge/.lsp.json; do run check-paths "$path" >/dev/null 2>&1 - assert_exit "check-paths allows $path in the named thread's plugin" 0 "$?" + assert_exit "check-paths refuses control surface $path in the named thread's plugin" 1 "$?" done for path in plugins/knowledge/package-lock.json plugins/knowledge/lib/x.cjs plugins/knowledge/lib/x.ps1 "plugins/knowledge/hooKs/hooks.json" "plugins/knowledge/AGENTſ.md"; do run check-paths "$path" >/dev/null 2>&1 @@ -228,6 +229,22 @@ echo 'guarded' >"$STUB/ws/src/a.js" mkdir -p "$STUB/ws/src/nested" && : >"$STUB/ws/src/nested/package.json" run record PRRT_a "$STUB/scratch/d.json" >/dev/null 2>&1 assert_exit "an empty stub name below the top of the checkout refuses the VALID record" 1 "$?" +# A plugin's control files stay out of the plugin-wide scope of a thread on another of its files. +setup +mkdir -p "$STUB/ws/plugins/p/.claude-plugin" "$STUB/ws/plugins/p/hooks" +echo 'r' >"$STUB/ws/plugins/p/README.md" +echo '{}' >"$STUB/ws/plugins/p/.claude-plugin/plugin.json" +echo '{}' >"$STUB/ws/plugins/p/hooks/hooks.json" +echo 'a' >"$STUB/ws/plugins/p/AGENTS.md" +echo 'n' >"$STUB/ws/plugins/p/notes.md" +fixture_git add -A && fixture_git commit -q -m plugin +jq '.threads = [{"id":"PRRT_p","path":"plugins/p/README.md","line":1,"review_commit":.head_sha,"root_comment_id":300,"comments":[{"id":300,"body":"x"}]}]' "$STUB/threads.json" >"$STUB/t2.json" && mv "$STUB/t2.json" "$STUB/threads.json" +for path in plugins/p/.claude-plugin/plugin.json plugins/p/hooks/hooks.json plugins/p/AGENTS.md; do + run check-paths "$path" >/dev/null 2>&1 + assert_exit "check-paths refuses $path through the plugin-wide scope" 1 "$?" +done +run check-paths plugins/p/notes.md >/dev/null 2>&1 +assert_exit "check-paths allows a plain file in the thread's plugin" 0 "$?" # The lane's sandbox masks a dotfile in the checkout with /dev/null, a device git # will not add: it is no fix, while a symlink to /dev/null still is one. if bwrap --bind / / --dev /dev true 2>/dev/null; then