[kyle-sexton]

Closes #8461

Hold: a human merges this

Summary

A fix request by a trusted person with write access can now name files to add to its review thread's edit scope. A named file joins the scope only when it is in the same plugins/<p>/ directory as the thread's path. The owner decided this on #8461. The case that prompted it is #8116's thread PRRT_kwDOTCGFQM6tojDu: the owner's request on write-transcript.js named run-transcript.js, and the lane refused that file at 2026-10-19T02:31Z as outside every kept thread's scope.

Fix

• address-feedback.mjs select: for each kept thread, reads the fix requests (isFixRequest) that write-access requesters (fixers) wrote. It takes each file-shaped word outside fenced code and quoted lines, with wrapping punctuation and a :line suffix removed. A word with a / is read from the repository root; a bare file name is read from the thread path's directory. A path is kept as one of the thread's named_paths when namedInScope allows it and the GraphQL tree at the gated head lists it as a regular file (mode 100644 or 100755, exact name). At most 10 per thread.
• thread-scope.mjs:
  • namedInScope requires plain ASCII segments in normal form, the thread's own plugins/<p>/ prefix (case-sensitive), no .github/ and no instruction path.
  • inThreadScope(path, threadPaths, threads) accepts a path in a thread's named_paths that namedInScope allows for that thread.
  • The instruction-path list moved here from apply-ci-fix.mjs (isInstructionPath), so fix-ci and thread scope share one list.
• The promote step and the collect step in pr-run-activity-write.yml, the reply job (publish-feedback.mjs, which validates named_paths in run.json) and the check-paths/record jq copy in feedback-thread.sh all apply the same rule.
• Hardening from the security review:
  • The promote step refuses a lane branch that adds or changes a symbolic link or gitlink, read from the lane tip's tree modes.
  • Instruction paths now match a bare hooks, .claude or .claude-plugin segment, and also cover a plugin's monitors/ and bin/ and .lsp.json.
  • Thread scope refuses any path with a character outside ASCII letters, digits and _@+.-. That closes the JS-versus-jq case-folding gap for characters such as U+212A KELVIN SIGN; no tracked path today has such a character.
  • The plugin-wide part of a thread's scope also refuses a plugin's .mcp.json, .gitmodules and CODEOWNERS, and .bash, .zsh, .ps1, .cjs, .mts and .cts files.
• Docs: docs/conventions/pr-pipeline/pr-run-activity.md (where thread scope is described), comments-ci.md, and the fix-ci instruction lists in README.md, pr-pipeline.yaml and pr-pipeline.schema.json. Fragments are in .changes/pr-pipeline/ and .changes/source-control/.

Verification

• node --test in .github/scripts/pr-unblock: 437 pass, 0 fail.
• feedback-thread.test.sh: all pass.
• scripts/check-affected.sh: 7 passed, 1 skipped (ruff, no .py), over 18 files.
• actionlint on pr-run-activity-write.yml: clean. shellcheck on both shell files: clean.
• The new tests take their expected values from the rules in pr-pipeline: let a trusted fix request widen a thread's edit scope to a named file in the same plugin #8461 and the live fix(knowledge): feed ASR transcript cues to video-digest frame densification #8116 thread:
  • a same-plugin named file is in scope (the fix(knowledge): feed ASR transcript cues to video-digest frame densification #8116 body adds run-transcript.js);
  • a name in another plugin is refused;
  • .github/ and instruction paths are refused;
  • ../ and absolute names are refused;
  • a requester without write access, or an untrusted one, adds nothing;
  • a name that is missing, a symbolic link, a gitlink or in another letter case is not added;
  • fenced and quoted names are not read;
  • the promote step refuses a lane symlink.
• Each test was broken once to confirm it fails: removing the existence check, instruction check, plugin match, traversal check, writer check, segment anchors and ASCII check each failed its tests, which pass again on restore.
• Security review (review:security-reviewer): no traversal escape and no escape at name time. It reported a symlink gap at promote time, missing instruction paths, and a non-ASCII case-folding gap; all three are fixed in the second commit.
• Left out:
  • A live run of the lane on fix(knowledge): feed ASR transcript cues to video-digest frame densification #8116; it needs the merged base.
  • Whether the lane's tools can write a symbolic link at all. The new promote check refuses one either way.

Related

• fix(knowledge): feed ASR transcript cues to video-digest frame densification #8116, thread PRRT_kwDOTCGFQM6tojDu: the failing case.
• feat(pr-pipeline): let a trusted fix request start address-feedback without an escalation #8426: the fix-request start of address-feedback that this builds on.
• Open, not changed here:
  • Any file-shaped word in a write-access fix request is read as a name, including a negated mention such as "do not touch x.sh". An explicit also edit <path> line would be stricter, but the fix(knowledge): feed ASR transcript cues to video-digest frame densification #8116 request does not use one.
  • The thread file (threads.json) is not hash-pinned between the select step and the promote step, as it was before this change.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

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-19T17:06:30.003724Z	3592533	Draft marked ready

ℹ️ 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]

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

Security review (incremental, 3b1f117 → 6d6ed2d)

No security issues found.

The incremental change adds TREE_READ_CAP (20) to namedPaths in .github/scripts/pr-unblock/address-feedback.mjs. The cap bounds the distinct directories whose trees are read at headSha for one thread. A comment cannot make the lane read an unbounded number of trees by naming paths in many different directories.

• It narrows the surface. Before this change a comment could name many paths in many directories, and each new directory cost one listTree call. Now at most TREE_READ_CAP (20) trees are read per thread. Candidates in directories past the cap are skipped with continue, and no further path is added without an existence check.
• The trust checks are unchanged. The author must be a trusted human and a write/admin fixer. The comment must be a fix request. namedInScope and the regular-file mode check (100644/100755, so no symlinks or gitlinks) still apply. The skip path only drops candidates, never admits one without the check, so scope cannot widen.
• The test change (address-feedback.test.mjs) only adds coverage for the cap and has no security surface.

No instruction-surface deletions in this diff, so the protected-class register was not needed.

[github-actions]

claude-review reviewed 6d6ed2d922d4f02a1b63281696cdd4d47210e0a7: 0 finding(s), 0 as inline comments.

No findings. I read the incremental diff (address-feedback.mjs and address-feedback.test.mjs) and found nothing to block. I could not run the test suite: the sandbox needs approval for node --test, and it was not granted. The checks below come from reading the code and the test fixtures.

Change reviewed. namedPaths now stops reading new directory trees once TREE_READ_CAP (2 × NAMED_CAP, so 20) distinct directories have been read. After that, a candidate in an unread directory is skipped with continue. A candidate in a directory that was already read is still checked against the cached tree (address-feedback.mjs:191-199).

What I checked.

• Cap applies only to new directories. The cap test sits inside if (!trees.has(parent)), so it limits only first reads. A name in a directory already read still counts toward NAMED_CAP.
• Skipped candidates cannot be named. The continue runs before trees.get(parent), so a skipped candidate never reaches the .find call. Skipping fails closed: a path in an unread directory is never added to the scope.
• Loop exits are unchanged. The NAMED_CAP early break still works as before.
• New test (address-feedback.test.mjs:732-743). I traced it against the TREES fixture (lines 639-644) and the named helper (lines 645-655).
  • With 25 missing directories plus run-transcript.js, the lane makes 20 reads (the 20 missing directories, in candidate order) and skips the rest. named_paths is undefined and reads.length is 20, as asserted.
  • With run-transcript.js listed first, the lane reads T and then 19 missing directories (20 reads). It skips the remaining missing directories. run-transcript.test.js is then found in the cached T tree, so both names are kept and reads.length is still 20, as asserted.

I did not review the other files of this pull request, which are unchanged since the last review.

[melodic-automation-lanes]

The pr-unblock lane stopped on this pull request and needs a person: the merge stages a change under .github/workflows/, which the lane does not push.

• Reason: workflow-staged
• Resume key: 3592533dc8028c4d411151b79ba658cd57a057a8 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=8470 and activity=resolve-conflicts:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=8470 -f activity=resolve-conflicts

[kyle-sexton]

Review note for the merge (from GitHub Security & Cost's pass at 9c2af24, P5, not blocking): pathTokens reads every line of the fix request, so a line like "don't touch scripts/x.sh" adds that path to the named scope. A follow-up should read paths only from the fix-request line or a marked paths: line.