🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

Codex review: needs maintainer review before merge. Reviewed August 28, 2026, 8:45 PM ET / August 29, 2026, 00:45 UTC.

ClawSweeper review

What this changes

Moves the r: skill auto-close decision from generic path labeling to added bundled skill manifests, while clearing stale automated labels and preventing duplicate responses.

Regression provenance

Possible regression — probable (reviewed change; known regression link). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep open. Current main still has the broad path-based close classification, while this focused maintainer-authored PR moves the terminal decision to Barnacle and addresses the prior grouped-skill finding.

Priority: P2
Reviewed head: ecd696adabbcca0aaab01471df2fb6ea3c524510

Review scores

Verification

How this fits together

Barnacle is the GitHub event handler that labels and can close pull requests. It receives a pull-request event and changed-file list, classifies the work, then applies labels and any matching close response.

flowchart LR
  A[Pull request event] --> B[Changed-file list]
  B --> C[Candidate label classifier]
  C --> D[Skill manifest decision]
  D --> E[Label cleanup or application]
  E --> F[Barnacle close response]

Before merge

• Resolve merge risk (P1) - This changes a write-capable automatic-close decision: an untested future manifest layout could either falsely close a core PR or fail to route a new bundled skill, although the focused cases cover direct and grouped manifests plus label-event cleanup.
• Resolve merge risk (P1) - Production automation grows by net 13 lines; the added classifier and label-event guards are necessary to distinguish terminal close eligibility from generic routing and are covered by focused regressions.
• Resolve merge risk (P1) - Exact-head CI was still in progress in the captured review state.
• Complete next step (P2) - No repair lane is needed: this is an active, focused maintainer-authored implementation; the remaining merge prerequisite is exact-head CI completion.

Security

None.

PR surface

Tests +110, Config -10, Other +23. Total +123 across 3 files.

Review metrics

Root-cause cluster

Relationship: canonical
Canonical: #132225
Summary: This PR is the canonical repair for the false-positive auto-close behavior; the linked Windows test reports are evidence of that routing failure but are distinct from the underlying symlink defect.

Members:

• adjacent_distinct: [Bug] Workspace-load symlink tests are not Windows-safe #127931 - It tracks the Windows symlink test problem rather than the automation that closed attempted fixes.
• same_root_cause: test(skills): fix workspace-load Windows symlink typing #127941 - It was closed with the erroneous skill-routing label while attempting the linked test fix.
• same_root_cause: test(skills): stabilize workspace-load Windows symlink typing #127946 - It was another attempted test fix closed through the same erroneous classification path.
• same_root_cause: test(skills): use platform-safe symlink type in workspace-load fixtures #132199 - Its body records the same false-positive skill close despite being a test-only repair.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Complete exact-head automation validation (recommended)
   Wait for the remaining checks on this head to finish successfully before merging a change that can label and close pull requests.

Technical review

Best possible solution:

Keep generic path labels non-terminal and let Barnacle assign the close label only when an added repository skills/**/SKILL.md manifest proves the PR adds a bundled skill.

Do we have a high-confidence way to reproduce the issue?

Yes—source provides a high-confidence event path: a change under src/skills/** receives the broad label, then Barnacle selects its existing close rule. Current main retains that path while the PR's focused tests model its removal.

Is this the best way to solve the issue?

Yes. The close decision belongs in Barnacle because it has file status and can distinguish an added skill manifest from incidental paths; deleting only the labeler rule would not preserve the intended handling for new bundled skills.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 80944da38fc8.

Labels

Label changes:

• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This maintainer-authored automation PR is exempt from the external-contributor real-behavior gate; its body reports focused before/after test evidence, while this read-only review did not execute repository code.
• remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
• remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

• P2: This is a contained automation defect that can incorrectly close otherwise valid pull requests.
• merge-risk: 🚨 automation: The PR changes Barnacle's label and automatic-close routing for pull-request events.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This maintainer-authored automation PR is exempt from the external-contributor real-behavior gate; its body reports focused before/after test evidence, while this read-only review did not execute repository code.

Evidence

What I checked:

• Automation entry point: The pull-request-target workflow loads Barnacle from the checked-out default-branch revision and invokes its event handler with write-scoped issue and pull-request permissions. (.github/workflows/auto-response.yml:58, ecd696adabbc)
• Current defect and current-main status: The introduced-diff baseline shows the existing r: skill path rule matched all of src/skills/**; git diff --quiet found no change to the three relevant files between the pinned PR base and fetched main, so current main has not already absorbed this repair. (.github/labeler.yml:304, d53f57c3d250)
• Owner-boundary repair: The classifier now derives the terminal label from an added manifest under the repository skill root, rather than from a generic path label; the matcher accepts nested organizational directories. (scripts/github/barnacle-auto-response.mjs:554, ecd696adabbc)
• Event lifecycle coverage: The label application path removes stale automated labels, preserves a human-applied close label, and suppresses the automation label-event duplicate response; focused tests cover each outcome. (scripts/github/barnacle-auto-response.mjs:785, ecd696adabbc)
• Prior finding resolved: The prior review required grouped manifests. The follow-up commit adds a recursive path segment and a skills/group/weather-helper/SKILL.md regression case. (test/scripts/barnacle-auto-response.test.ts:298, ecd696adabbc)
• Skill-layout contract: Repository documentation states that skills under skills/ are discovered recursively and may be nested for organization, matching the broadened manifest matcher. Public docs: docs/install/migrating-hermes.md. (docs/install/migrating-hermes.md:67, ecd696adabbc)

Likely related people:

• Peter Steinberger: Introduced the custodian-skills change associated with the original broad label rule and has recent history in the Barnacle automation path. (role: feature owner and recent automation contributor; confidence: high; commits: 44736749eb0c, cb696a1bb3b6; files: .github/labeler.yml, scripts/github/barnacle-auto-response.mjs)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Let the remaining exact-head automation checks complete before merge.

Rating scale

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

• reviewed 2026-08-29T00:24:42.592Z sha cdc663f :: needs changes before merge. :: [P1] Classify grouped bundled skill manifests

Merged via squash.

• Prepared head SHA: ecd696adabbcca0aaab01471df2fb6ea3c524510
• Landed commit: 6886458244bb1a835c913273b2a49256e080145b