Skip to content

feat(pr-pipeline): admit Knowable issues and keep lane fixes out of plugin control files - #9559

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/admit-knowable-plugin-arm-scope
Oct 24, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/admit-knowable-plugin-arm-scope

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: owner decisions Q1 and Q3 of 2026-10-24 (lane-stop audit answers, recorded in Issue Triage's session: "accept all").

Summary

  • Q1: intake admission took only Uncertainty Known (admission-rules.json:7), while the run lane already runs Knowable issues research-first (pr-pipeline.yaml:664-669). That cut supply for no lane reason. It now admits both.
  • Q3: address-feedback's plugin-wide scope arm (thread-scope.mjs:99) let a review-fix edit a plugin's control and instruction files (.claude-plugin/plugin.json, .mcp.json, hooks/hooks.json, AGENTS.md, CLAUDE.md, bin/) whenever a kept thread sat on any other file in that plugin. The pull-request-files arm already excluded these (prPathAllowed, CONTROL_PATH). The plugin arm now excludes them too. A thread on a control file still reaches that file through its own path, and named paths are unchanged.

Fix

  • .github/actions/resolve-config/admission-rules.json: uncertainty is ["known", "knowable"].
  • .github/scripts/pr-unblock/thread-scope.mjs: the plugin arm adds !CONTROL_PATH.test(path).
  • plugins/source-control/skills/pull-request/scripts/feedback-thread.sh: the jq mirror gets the same $control exclusion.
  • Tests:
    • admit.test.mjs adds a positive Knowable admit case, and its criterion-3 refusal case now uses Unknown.
    • thread-scope.test.mjs asserts that the control files are out of the plugin-wide scope.
    • feedback-thread.test.sh moves those paths to the refused list and adds a check-paths case on a committed plugin.
  • Fragments: source-control patch, pr-pipeline minor.

Verification

  • node --test .github/scripts/pr-unblock/thread-scope.test.mjs .github/scripts/intake-triage/admit.test.mjs: 113/113 pass.
  • bash plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh: 0 failures.
  • shellcheck on both shell files: clean.

Related

#9212 (Q51 admission criteria), #9444 (control paths in the PR-files arm), #9551

🤖 Generated with Claude Code

…lugin control files

Owner decisions 2026-10-24 (Q1, Q3):
- admission-rules.json takes Uncertainty knowable as well as known; the run
  lane already runs Knowable research-first (pr-pipeline.yaml:664-669).
- thread-scope.mjs's plugin-wide arm, and feedback-thread.sh's mirror, now
  exclude CONTROL_PATH (.claude-plugin/, hooks/, monitors/, bin/, .mcp.json,
  .lsp.json, AGENTS.md, CLAUDE.md, .claude/), as prPathAllowed already did.
  A thread's own path and named paths are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T03:34:58.459123Z 7dffb12 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 7dffb123723a9f10633a2184bc111b83381df291: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

critical-findings: 0

What I checked (diff in .claude-lane/pr.diff, 8 files, plus the PR context):

  • Scope-rule hardening (thread-scope.mjs, feedback-thread.sh). The change narrows the plugin-wide arm: it now excludes control and instruction files (.claude-plugin/, hooks/, monitors/, bin/, .mcp.json, .lsp.json, AGENTS.md, CLAUDE.md, .claude/, .github/). I compared the new jq $control regex in feedback-thread.sh with CONTROL_PATH in thread-scope.mjs:38-39. They are the same pattern, so the shell mirror and the JS rule cannot disagree on this arm. This lowers risk and widens no access. The thread's own path and named paths are unchanged, as the PR says. I did not re-audit the existing same-directory test-file and named-path arms. They sit outside this diff.
  • Admission rule (admission-rules.json). Adding "knowable" to uncertainty admits more issues into the automated lane. This is a deliberate, recorded owner decision, per the PR description. It is not a plugin userConfig guard, and the file is not a plugin manifest. The refuse-labels, size, class and type gates are unchanged. The needs-owner-decision and do-not-merge refusals still apply to the newly admitted issues. I found no exploitable path in this diff. Unknown uncertainty stays refused, and the updated test asserts it.
  • Instruction-surface deletions. No standing instruction is removed or softened. Only comments and tests change, and the tests move toward stricter refusal.
  • Trust-gate paths. The diff touches no .github/standards/trusted-actors/**, .github/actions/check-trusted-trigger/**, CODEOWNERS or .gitmodules path, so no hold applies.
  • Manifests. The diff touches no plugin.json or marketplace.json, so there are no manifest <field> lines.
  • PR context. It contains no embedded instructions aimed at the review lane.

I did not run the tests or read the instruction exception register. The review rests on reading the diff and the CONTROL_PATH definition.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 7dffb123723a9f10633a2184bc111b83381df291: 0 finding(s), 0 as inline comments.

No findings on this pull request.

Checked

  • thread-scope.mjs and feedback-thread.sh use the same control-path pattern. The jq $control regex at plugins/source-control/skills/pull-request/scripts/feedback-thread.sh:71 matches CONTROL_PATH at .github/scripts/pr-unblock/thread-scope.mjs:38-39 character for character, so the two stay in sync. $control is added only to the plugin-wide $plugin arm. The thread's own path, the pull-request-files arm and the named-paths arm are unchanged, as the description says.
  • The Knowable admission change is a one-line rules edit. The tests cover both a positive Knowable admit and a refusal on Unknown.
  • The updated tests assert the new behavior in both implementations: the control paths move to refused, and a plain file in the thread's plugin is still allowed.
  • The change fragments' bump levels match the change: minor for pr-pipeline and patch for source-control.
  • No closing issue is linked. The description cites owner decisions Q1 and Q3, which I could not read, so I took them as stated.
  • No plugin manifests (plugin.json or marketplace.json) are changed, so there are no manifest lines.
  • I did not run the tests. The 113/113 and 0-failure results in the description are author-claimed and unverified by me.

Hold: none. The change doesn't add a model call, schedule or trigger, and it doesn't touch trusted-actors, CODEOWNERS or .gitmodules. Admitting Knowable issues may increase how many issues the existing run lane picks up. That is a volume change on an existing path, not new spend infrastructure, so I did not hold on it.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit a682359 Oct 24, 2026
34 checks passed
@kyle-sexton
kyle-sexton deleted the fix/admit-knowable-plugin-arm-scope branch October 24, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant