Skip to content

feat(app-control): add the drive affected action and --trusted-root - #11404

Merged
melodic-automation-lanes[bot] merged 1 commit into
mainfrom
work-resolve-issue/issue-11384-41141897199-1
Oct 30, 2026
Merged

melodic-automation-lanes[bot] merged 1 commit into
mainfrom
work-resolve-issue/issue-11384-41141897199-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #11384

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Adds /app-control:drive affected --base <ref> \[--head <ref>\], backed by the new scripts/affected_features.py. It maps git diff --name-only --no-renames <base>...<head> to the control skill's features through the hub index, and through a matched module's ## Sub-features lines. It writes affected.json in the issue's C1 shape and launches nothing. It also adds --trusted-root <dir>, which limits control-skill discovery to <dir> and to plugins under it (the script refuses a hub outside it). The control-skill contract now fixes the sources glob syntax (** crosses /, * and ? do not, every other character is literal) and says where the globs resolve. I made the means choices from the code. The drive skill does discovery and passes the chosen hub to the script, so the script stays a pure matcher. For a project skill, globs resolve against --project-dir, matching the contract's 'relative to the project directory that holds .claude/'. Consumers that parse sources today, which this slice does not switch: plugins/testing/skills/run-e2e/SKILL.md:118 and :195, plugins/verification/skills/confirm/SKILL.md:168, and plugins/app-control/skills/map/scripts/check-map.sh:159-187.

Not verified

  • bash plugins/app-control/scripts/affected_features.test.sh was not executed: this lane runs only the named wrappers. It was hand-traced by the review subagent, and the run is left to the PR's CI test lane.
  • /skill-quality:check drive was not run: the skill-quality plugin is not loaded in this lane, and check-affected skipped changed-skills because it runs checked-tree code. Left to the PR's CI.
  • The new evals 26-29 were not run (no eval runner in this lane). check-evals-quality passed on them.
  • ci-status green on the PR head is a PR check, left to the PR's CI.
  • Tidy skipped: the change is about 560 changed lines (the two new files counted whole), over the 300-line tidy cutoff.
  • The changelog fragment was written by hand: new-changelog-fragment refused because HEAD is detached. It follows the existing work-resolve-issue fragment naming, and check-changelog-fragments accepts it.

Fix

  • plugins/app-control/scripts/affected_features.py: new script. Reads the diff and the hub index (module README Sub-features recursion, fenced blocks skipped), applies the fixed glob syntax and parses each entry point's driven/not driven marker. Refuses a hub outside --trusted-root. Writes affected.js
  • plugins/app-control/scripts/affected_features.test.sh: new tests against throwaway git repos. Covers plugins/*/SKILL.md not matching plugins/a/b/SKILL.md, plugins/**/x matching nested and zero-folder paths, project feature and module matches, a rename's old and new paths, entry-point parsing,
  • plugins/app-control/skills/drive/SKILL.md: affected and --trusted-root rows in Actions, --trusted-root in Find the control skill steps 1-2, step 6 routes affected to the new ## Affected section (script arguments, no fallback), description, and argument-hint (kept within the 100-character b
  • plugins/app-control/skills/drive/reference/evidence.md: new affected.json section (fields, notes, glob syntax) and the trusted_root report.json field.
  • plugins/app-control/reference/control-skill.md: Hub paragraph. sources resolve against the project directory, or for a plugin control skill the repository under verification. States the glob syntax, names the script, and gives the control skill present, <n> files unmatched note.
  • plugins/app-control/skills/drive/evals/evals.json: evals 26-29 cover a project skill match, a plugin skill match with repo-root globs, no control skill (empty with note), and a PR-checkout control skill ignored under --trusted-root.
  • .changes/app-control/work-resolve-issue-issue-11384-a7c3e915.md: minor changelog fragment (Added: affected, --trusted-root; Changed: glob syntax in the contract).

Verification

The runner ran each check from the base copy against 1e7baec:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • .lane/checks/bin/check-changelog-fragments: all 20 fragments valid
  • .lane/checks/bin/check-affected: 13 passed, 5 skipped (typos not installed; validate-plugins sub-checks and changed-skills that run checked-tree code). Earlier runs failed on shellcheck SC2016, an argument-hint over 100 characters and markdownlint MD038; all three are fixed.
  • .lane/verify/bin/check-evals-quality plugins/app-control/skills/drive/evals/evals.json: PASS, 0 warnings
  • .lane/verify/bin/cant-fail-scan --file plugins/app-control/scripts/affected_features.test.sh --check: PASS, no gating findings
  • fresh-context review subagent: hand-traced all 13 test cases against the script and found each passes. Fixed 3 of its findings: project-dir wording for a skill from the main checkout, the driven marker now needs a ': ' or space separator, and the note row wording. Left a 4th (sections end only at '#
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41141897199

Left out: the items under Not verified.

Related

#11384

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

Copy link
Copy Markdown
Contributor

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

No security findings in this pull request.

I read the full diff and the PR context. Nothing changed matches a trusted path, and no plugin.json or marketplace.json changed, so there are no manifest flags. Nothing touches a trust control, a token's scope, a kill switch or a merge hold. The only code that runs is the local affected_features.py and its test. No workflow changed.

What I checked in plugins/app-control/scripts/affected_features.py:

  • --trusted-root: The script resolves the hub and the skill directory with realpath. It refuses either one when commonpath against the resolved root differs from the root, at lines 252-255. A symlink or .. cannot escape the check. Index paths read from the hub must match features/.+\.md, and the resolved file must sit inside the skill directory (line 172), so a hostile index line cannot read outside the control skill.
  • Git invocation: The script strips GIT_* from the environment and passes --no-replace-objects, -c core.fsmonitor=false, --no-ext-diff, --no-textconv, -z, --end-of-options and a trailing --. It also rejects refs starting with -. This blocks option injection, and a checkout under test cannot run a configured external diff, textconv or fsmonitor command through the git diff call.
  • Glob matching: Globs are turned into regexes with re.escape on every literal character. The glob text cannot inject regex syntax. The only risk is backtracking from many repeated **/ segments. The hub supplies the globs, and a control skill already directs what the lane runs, so that is not a security issue.
  • --project-dir: The script requires it to be inside the repository top. Files outside the prefix map to None and never match.
  • SKILL.md changes: The drive skill's discovery steps only narrow when --trusted-root is given: one root in step 1, and step 2 limited to plugin folders under the root. Step 6 adds an affected route. No standing rule is removed or softened.

Notes, none counted as findings:

  • The --trusted-root restriction on discovery is written as instructions to the model in SKILL.md. The script enforces it only for the hub and skill directory it is handed. That layering fits the PR's stated design ("the script stays a pure matcher").
  • Text from an unrestricted control skill, such as index paths, can end up in the note field of affected.json. A control skill already directs what the drive skill does, so this adds no new reach.
  • Spend: none. The change adds no billed resource and no turn, schedule or model setting.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 1e7baec91aa5d1fcfcc607d01ee50e34ae7872f5: 0 finding(s), 0 as inline comments.

Code review: PR #11404 (/app-control:drive affected, --trusted-root)

Result: no findings. I read the full diff (7 files) and the PR context. I did not run any tests: this lane has no shell. Every statement about the tests below comes from reading the code and tracing it by hand. The PR's own "Verified" claims are author-claimed, and I did not confirm them.

What I checked

  • affected_features.py glob semantics.
    • glob_re turns **/ into (?:.*/)?, ** into .*, and * and ? into single-segment matches. It matches with fullmatch.
    • So plugins/*/SKILL.md does not match plugins/a/b/SKILL.md. plugins/**/x matches both plugins/x and plugins/a/b/x. This is the sanity case the issue names.
  • Diff and git safety.
    • The diff is git diff --name-only --no-renames -z --no-ext-diff --no-textconv --end-of-options <base>...<head> --, so a rename counts its old and new path.
    • GIT_* environment variables are stripped and core.fsmonitor=false is set. A checkout under test has no obvious way to run code through git config.
    • Refs starting with - are rejected.
  • --trusted-root enforcement.
    • The script resolves both the hub and the skill directory with realpath and refuses either one outside the root.
    • Feature files are read only when the path matches features/.+\.md and its real path stays inside the skill directory. That blocks .. and symlink escapes.
    • Because the script checks this itself, it holds even if the drive skill's discovery text were followed loosely.
  • Module recursion. A matched module line goes through its ## Sub-features, and a seen set prevents cycles. A file that the module matches but no sub-feature matches stays in unmatched_files. The tests encode this on purpose.
  • Tests. I traced the 13 cases in affected_features.test.sh by hand against the script: ordering, unmatched_files, the --head $BASE empty-diff note, the refusal exit code with no affected.json written, and --project-dir prefix stripping. All came out as asserted.
  • Entry-point marker. The parser accepts a line that ends driven, not driven, or not driven: <reason>, as reference/control-skill.md:195 defines. An unmarked line defaults to driven: false, which is the conservative default.
  • Docs and evals. The SKILL.md Actions rows, the ## Affected section, evidence.md, the contract paragraph, and evals 26-29 agree with the script's arguments and the AffectedJson shape in the issue. The one deviation from the issue is deliberate and stated in the PR. For a project control skill, globs resolve against --project-dir, and the contract's existing wording ("relative to the project directory that holds .claude/") already says that.
  • Changelog fragment. bump: minor fits an added action and flag.

Acceptance criteria from #11384

  • affected row and tests. The affected row is in the Actions table. The two glob cases in the sanity check are in the test file and would fail if the glob rule broke. Running the test file and /skill-quality:check drive is left to CI, as the PR says.
  • Evals. Evals 26-29 cover a project skill match, a plugin skill match with repo-root globs, no control skill (empty with note), and a PR-checkout control skill ignored under --trusted-root.
  • Changelog. The fragment covers the version and changelog criterion.
  • ci-status. The ci-status criterion is a PR check I cannot see.

Notes, not findings

  • Section parsing ends a ## section only at the next ## heading, so ### headings inside ## Entry points stay in it. The PR author already named this as a known limit.
  • The other consumers of sources (run-e2e, confirm, check-map.sh) still parse it themselves. The PR states this is out of scope for this slice. AC3 ("confirm and run-e2e call it instead of parsing control skills") is therefore not met yet by this slice alone, and the issue's plan does not claim it is.
  • Spend: the added evals will spend prepaid API credits when run. That is not a hold.

No hold.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 1e7baec91aa5d1fcfcc607d01ee50e34ae7872f5.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit b0b41ac Oct 30, 2026
44 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-11384-41141897199-1 branch October 30, 2026 23:56
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.

feat(app-control): add the drive affected action and --trusted-root

0 participants