feat(source-control): exempt test-only changes under floored script paths from pr-merge's path floor

Problem

pr-merge's HIGH path floor holds every PR that touches a floored script path (plugins/*/scripts/**, plugins/*/skills/*/scripts/** and the other floored script paths in babysit_risk.py), including PRs that change only tests there. 21 of 22 floor-held PRs merged unchanged (GitHub Support research, owner-desk causes, 2026-10-21).

Desired behavior

A PR whose changes under floored paths are ALL test-only does not take the floor hold. Test-only means a path matching *.test.mjs, *.test.sh, *.test.js, test_*.py, */tests/** or */fixtures/**.
Any non-test file under a floored path still holds.
Paths are normalized before matching, so tests/../run.sh and similar still hold.
A test-pattern file that a plugin manifest, hooks.json, .mcp.json, .lsp.json or monitors.json actually invokes still holds (reuse the declared-paths reading in babysit_risk.py).
Every other floor reason (.github/**, .claude/**, lib/**, hooks, declarations, mode changes) is unchanged.
Tests: the exemption case merges; the disguised path, the declared test file, and a mixed test plus non-test change all still hold.
S&C passes the PR. The owner merges it (merge-gate code).

Decision recorded. Owner, 2026-10-21, in the poteto session ("Yes, go with recommendations", also given in GitHub Support's session): build this. It widens unattended merge with the safeguards in the body. S&C passes the PR and the owner merges it.

Proposed work class: none

Thanks for the report. Automated triage handed this issue to a maintainer.

Unconfirmed model suggestion: work-class: scoped
Basis: automated intake triage; a maintainer confirms or changes it.

_{Automated triage. A maintainer reviews it.}

@poteto

@poteto: two more HIGH-floor narrowings, from an audit of lane PRs (the 60 newest lanes-App PRs, #8185-#8870; 16 rated HIGH). Owner-approved ask, relayed by Issue Triage. Both are next to this issue's test-only exemption, in babysit_risk.py, which you own:

**/.claude-plugin/** (babysit_risk.py:168): a plugin.json whose parsed base and head differ only by added userConfig keys need not be HIGH (e.g. feat(session-flow): add a since-your-last-message catch-up scope to session-flow:orient #8808 added only userConfig.orient_catch_up_word_cap). Keep HIGH for a modified or removed userConfig key (hooks read CLAUDE_PLUGIN_OPTION_* defaults), for any other key (hooks, mcpServers, lspServers, monitors), and for marketplace.json.
high-capability (babysit_risk.py:646): disable-model-invocation: true only removes a capability, so it should not count.

Expected effect: small (16 to 15 in that window). This issue's exemption clears none of the observed HIGH PRs alone, because each of them also changed a non-test script. The volume comes from changes to plugins/*/skills/*/scripts/**. Whether to exempt those is a risk call for the owner, routed through the hub; I am not proposing it here.

feat(source-control): extend the test-only floor exemption to scripts…

test(source-control): pin a mode-100755 test-only change under a scri…

fix(source-control): narrow the test-only floor exemption

@poteto

@poteto: owner decision 2026-10-21 (in session, "go with recommendations"): manifest spec for the HIGH floor. This replaces the narrower "metadata-only allowlist" from my earlier comment.

**/.claude-plugin/** (babysit_risk.py:168) stops being a blanket HIGH path. A manifest change is HIGH only when the parsed base→head diff hits one of these:

a userConfig.<key>.default changed, or a userConfig key removed (silently flips behaviour for users who never set it; c35f64b flipped a guard default and claude plugin validate passed it);
a userConfig.<key>.options enum added or changed (clients before v2.1.271 can't load the plugin: https://code.claude.com/docs/en/plugins-reference);
hooks, mcpServers, lspServers, monitors or dependencies added or changed (these run code or pull in another plugin; context-guard: every hook is inert — the manifest redeclares the auto-loaded hooks/hooks.json #1985, the only historical breakage, was this class);
a marketplace.json plugin source added or changed (chooses where code is fetched from; all 91 entries are relative today);
a parse failure or a duplicate JSON key (becomes a CI lint failure separately; treat as HIGH until that lint exists).

Everything else in a manifest (version, description, keywords, author, homepage, added userConfig keys with a default, defaultEnabled) leaves the floor. CI already runs claude plugin validate on touched plugins and --strict on the catalog: .github/workflows/pr-require-checks.yml:1960-1985 → scripts/validate-plugins.sh:167,193,197 (#8901 closed as already satisfied).

Evidence: history of 2,824 manifest commits on main, 2,525 version/metadata only; one consumer-facing breakage (#1985, hooks). Automation Lanes is also adding a duplicate-key CI lint and manifest criteria to the review lanes.

@poteto

@poteto: owner refinement (same day, in session): the five classes in the comment above do not route to the owner. They route to an agent review gate:

The deterministic parsed-diff classifier (the five classes above) marks the PR manifest-review-required instead of HIGH.
A dedicated manifest-review agent check judges it against the manifest criteria (default flips need a stated reason, and a guard *_enabled going true→false is a finding; anything added that runs or fetches code names what runs and from where; new settings have a default and a reader; the bump level fits; the description and marketplace entry match what ships). It returns a structured verdict.
pass → normal pr-merge path (auto-merge through the usual gates). flag or no verdict → needs-human.
The verdict check must fail closed: a session that posts no verdict is a failure, not a pass (the existing review lanes conclude success even when they post nothing).
The reviewer treats PR text as data. Its config stays under clause 5 (review workflows), so a PR cannot edit its own reviewer.

So the floor change for **/.claude-plugin/** is: remove the blanket HIGH; the five classes → manifest review gate; everything else → normal path. A parse error or duplicate key stays a hard CI failure.

@poteto

@poteto: two corrections to the two comments above (both verified):

Duplicate keys are already a hard CI failure. scripts/check-manifest-duplicate-keys.py (ci: no gate detects a duplicate key in a plugin manifest — schema validation cannot, and every consumer is last-wins #1498) runs in check-plugins (.github/workflows/pr-require-checks.yml:1919-1923), which ci-status needs. Class 5 needs nothing from the floor.
No new verdict check is needed. pr-merge already refuses unless the review lanes completed for the head with zero open findings (babysit_review_evidence.py, babysit_merge.py:1820-1830), and ADR 0038 bars making review checks required. So the floor change is just: drop the blanket HIGH on **/.claude-plugin/**. Classes 1-4 merge only on a clean review, and the review lanes' manifest criteria (draft feat(review): add plugin manifest criteria to the CI review lanes #8909) are what make that review meaningful. Land feat(review): add plugin manifest criteria to the CI review lanes #8909 before or with the floor change.

@poteto

@poteto: precondition before the floor change ships. #8909 (now a75086b) makes every manifest flag a counted finding, since pr-merge holds on the findings total (babysit_review_evidence.py), not on summary text. After #8909 merges, Automation Lanes opens a throwaway test PR that flips a guard *_enabled default true→false and confirms: (1) the security lane records findings>0 for the head, (2) pr-merge refuses it. Remove **/.claude-plugin/** from the floor only after that passes; I'll post the result here.

@poteto

@poteto: S&C review of #8909 adds two more preconditions before **/.claude-plugin/** leaves the floor: (1) #8909's criteria fixes now in progress (guard keys judged by what their reading code does, not by *_enabled names; a stated reason never clears a guard flip; the full run/fetch field list; flag lines exempt from verifier dropping), merged after a fresh S&C pass; (2) ci-workflows#847, so the review job cannot report fewer findings than flag lines. Then the end-to-end test PR. I'll post when all three are done.

Decision recorded. Owner, 2026-10-21, in the poteto session ("Do whatever is most efficient", accepting the recommendations put to them):

Q6 (feat(source-control): exempt test-only changes under floored script paths from pr-merge's path floor #8891): the test-only exemption from pr-merge's path floor covers files named as tests only (*.test.mjs, *.test.sh, *.test.js, test_*.py). Files that only a tests/ or fixtures/ directory names keep the floor hold. Reason: plugins/code-metrics/skills/setup/scripts/setup-check.sh:153,205 copies scripts/fixtures/sources/* into a work folder and runs a collector on them on the consumer's machine, and a lone conftest.py under tests/ runs in CI without review. The directory patterns can return once the scorer can read the head tree and tell runnable fixtures from inert ones.
Q18 (feat(source-control): exempt dmi-only skill edits and eval data from the path floor and judge manifests by their parsed change #8926, manifest allow-list): added userConfig keys and defaultEnabled changes do not leave the floor; they rate manifest-review-required like the five classes above. Reason: S&C showed a new skip_review key (default true) plus skill text reading ${user_config.skip_review} scored LOW, and whether a new default is restrictive cannot be decided mechanically. This narrows the "Everything else" list in the owner's earlier comment.

E2E manifest gate test: result (#8941, head `41a8e33`)

Test PR #8941 flipped block_dangerous_git_enabled (guardrails; gates the block-dangerous-git hook) default from true to false. It carried the do-not-merge label from creation, was never merged, and is closed. Its branch is deleted.

Check	Result	Evidence
(a) The security lane flags the widening guard default, and the findings count is at least 1 and at least the number of flag lines	PASS	The annotation on run 39689656523 reads `claude-lane-review-complete lane=claude-security-review sha=41a8e3317f… findings=1 inline=1 scope=whole`. The reviewed-head record has one flag line, `manifest userConfig.block_dangerous_git_enabled.default: flag`, posted inline. N=1 and there is 1 flag line.
(b) The risk rating is HIGH with `manifest-review-required`	PASS (scorer run directly, not the lane)	`babysit_risk.score` was run on the PR's real diff, using base text (`4bf9806`) and head text (`41a8e33`) from `git show`, at class C2. It returned `HIGH` with two reasons: `manifest-review-required` with detail `plugins/guardrails/.claude-plugin/plugin.json: userConfig.block_dangerous_git_enabled.default`, and `high-path:plugins/guardrails/**`. Both manifest sides were read and parsed, so the code comes from the key change and not from the fail-closed parse path. Only the manifest reason is the claim under test. The class, lanes and config inputs were synthetic.
(c) pr-merge refuses	PASS, but it refuses before the risk gate	The gate ran check-only, with no `--merge`: `babysit_merge.py melodic-software/claude-code-plugins#8941 --allowed-owners melodic-software --unattended --authority pr-merge --lane-kill-switch proceed --lane-rung-cap ''`. It exited 10 with `ready: false`. Blockers: the body hold, the `do-not-merge` label, `mergeStateStatus=BLOCKED`, 1 unresolved review thread, failing `ci-status`, and `unattended merge refused: acting-login-mismatch`. It refused before scoring, so the refusal is not `risk:HIGH`.

Why no lane  comment exists. The pr-merge workflow only lists PRs opened by the lanes App (gh pr list --author app/<slug>, plus Dependabot's). It also prefilters out do-not-merge PRs before the gate runs (.github/workflows/pr-merge.yml, "List the candidate PRs" step). A PR authored by a person, as this one was, is never scored by the lane. To exercise (b) and (c) through the live lane, the test PR would have to be opened by the lanes App and carry no hold label, and that would make it mergeable.

Pre-push checks. The first push was refused for two reasons: changelog-fragments (no fragment under .changes/guardrails/) and plugin-options-docs (stale README options row). I added a bump: minor fragment and ran scripts/sync-plugin-options-docs.py, and the second push passed.

Side observation. The code-review lane also flagged that the hook's own fallback still defaults to on (${CLAUDE_PLUGIN_OPTION_BLOCK_DANGEROUS_GIT_ENABLED:-true}, plugins/guardrails/hooks/block-dangerous-git.sh:78). A manifest-only flip therefore disagrees with the hook's runtime default.

Navigation Menu

feat(source-control): exempt test-only changes under floored script paths from pr-merge's path floor #8884

Problem

Desired behavior

E2E manifest gate test: result (#8941, head `41a8e33`)

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

feat(source-control): exempt test-only changes under floored script paths from pr-merge's path floor #8884

Description

Problem

Desired behavior

Activity

kyle-sexton commented on Oct 21, 2026

github-actions commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

kyle-sexton commented on Oct 21, 2026

E2E manifest gate test: result (#8941, head 41a8e33)

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions

E2E manifest gate test: result (#8941, head `41a8e33`)