Add the hookdeck-cli role-reviewer override - #420
leggetter wants to merge 2 commits into
Conversation
Sorry, something went wrong.
Installs the reviewer override that was written and verified in agent-control-room, so reviews of this repo pick up its repo-specific checks instead of only the generic role. The file was written against release/v3.0.0 at 3be28c6. Re-verified against main at 5c6948c and corrected where it had drifted: pkg/mcpcore and the tool-prefix scheme do not exist on main, run_parallel.sh has three slices rather than four, the outpostlive tag and its workflow are release-branch only, and tool_event.go is actually tool_events.go. The skill is named role-reviewer rather than repo-prefixed like hookdeck-cli-release, because the base role resolves its override by role name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Harnesses index .agents/skills/ and register this file as a selectable skill named role-reviewer, alongside the base role. The description read like a self-contained reviewer, so picking it got you repo specifics with none of the method, read-only rule or report format. Rewords the description to lead with what it is not, and adds the same warning to the body for anyone who opens the file directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review feedback addressed — SKILL.md:3 (the description field)
Checked it, and the answer splits in two. On content, it is a strict delta. It shares zero section headings with the base role: base has Before anything else / You do not write / What to check / What to leave alone / Report format / Related skills; this file has Running the checks / Additional checks / Do not ask for / Scope. Nothing is restated. But the concern behind the question is real, and I could reproduce it. Harnesses index .agents/skills/ and register this file as a selectable skill named role-reviewer, sitting alongside the base role. My own session printed it back: The following skills are available for use with the Skill tool: - role-reviewer: Project overrides for the role-reviewer role in hookdeck-cli. Delta only; ... Selecting that entry gets you repo specifics and none of the method, read-only rule or report format — a reviewer-shaped hole. The old description's first clause was readable as "this is the reviewer role for hookdeck-cli", which does not help. name: cannot change (it is how the base role resolves the override by path), so the description now carries the warning. Fixed in 43c7a89:
Re-registered and confirmed the new text is what the picker shows. Checks re-run after the changego build -o hookdeck . exit 0 go vet ./... exit 0 go test ./... exit 0 (17 packages ok, 0 FAIL) Frontmatter re-parsed as valid YAML (name, description) after moving to a >- block. Still no Go files touched; the 29 gofmt -l files and 116 lint issues remain pre-existing and untouched. One thing I did not change, and think you should decide## Scope during the v3.0.0 beta is the one section that genuinely is not a hookdeck-cli override. It is about the board — the Review column, Backlog, feeds, a human promoting a card — and it cites "the control room's README". None of that is a property of this repository, and it would read identically in any repo the reviewer works on. It is the strongest evidence for your "generic reviewer" reading. I left it because the task was explicit not to rewrite the file, and because that section records a dated decision (2026-09-21) with its reasoning — deleting it off a one-line comment felt like over-reach. If you want it lifted into the base role or the control room README instead, say so and I will move it; it is a two-minute change. |
Sorry, something went wrong.
|
Closing this - wrong artifact for a public repo. It was written as a delta on a reviewer role that lives in a private repo, so anyone cloning this one gets a skill that declares itself incomplete and points at a base they can't fetch. It also carried MCP guidance that only holds on release/v3.0.0; pkg/mcpcore doesn't exist on main at all. Replacing it with a self-contained review skill that stands on its own, named like its siblings. New PR to follow. |
Sorry, something went wrong.
What the task was
Install the already-verified role-reviewer override into this repository at .agents/skills/role-reviewer/SKILL.md, copied from agent-control-room (examples/hookdeck-cli/.agents/skills/role-reviewer/SKILL.md), re-verifying its claims against the current tree rather than rewriting it, and list it in AGENTS.md § Agent skills.
What I changed
Two files:
On the skill name
It is role-reviewer, not hookdeck-cli-reviewer. This repo's own skills are repo-prefixed (hookdeck-cli-release), so this looks inconsistent on purpose: the base role resolves its override by looking for .agents/skills/role-reviewer/SKILL.md, keyed on the role name. Repo-prefixing it would mean nothing ever loads it. The AGENTS.md bullet says this inline so the next reader does not "fix" it.
On duplication
Only .agents/skills/ gained a file. .claude/skills and .cursor/skills are already committed symlinks (mode 120000) to ../.agents/skills, so both harness paths resolve to the single canonical copy with no second file — verified by ls through both paths and by git status, which shows only .agents/skills/role-reviewer/. This is the rule AGENTS.md states as "never copy a skill into a second location".
Framing
Dropped the example-of-an-override framing. It is the real override now.
Drift I corrected
The source was written against release/v3.0.0 at 3be28c6. This PR targets main at 5c6948c (v2.6.0), and release/v3.0.0 is 76 commits ahead of main — so the file was verified against a tree that is ahead of this one, not behind it. Several claims describe code that is not on main:
Rather than delete the MCP guidance — it is verified work and applies the moment v3.0.0 lands — I moved those two bullets under a heading that states plainly they are release/v3.0.0-only and that raising a finding from them on main would be wrong. The other four corrections are made in place. The provenance header now records both the original check and this re-verification.
Claims I re-verified as still true
.claude/.cursor symlink modes and targets; .github/skills/ absent; the §5/§10 commands; test/acceptance/.env gitignored (via .gitignore:18); go test ./... excluding tagged acceptance tests; the eight CLITelemetry fields (exact match); pkg/cmd/helptext.go and Annotations["cli.arguments"]; the manual tag on project_use_manual_test.go; the -tags=telemetry job in both CI and run_parallel.sh; d0104e8 ("refactor: make .agents/skills the canonical skills location"); CHANGELOG.md no longer maintained; and AGENTS.md's #336 citation, which the override still flags as miscited.
What I verified, and what that does and does not cover
This change touches no Go code — the diff is one new Markdown file and one Markdown bullet. Zero .go files, confirmed via git status. So the green suite above says the tree still builds; it does not exercise anything this PR changed, and there is nothing here a Go test could assert on. No tests added, for that reason.
Pre-existing and untouched by this PR:
Both were already in this state on main. I did not fix them — out of scope for this change.
Not verified
Observation, not changed here
AGENTS.md § Agent skills cites #336 for the "never copy a skill into a second location" rule. Per the override, that issue is actually about a release-skill CI gate reporting pending on every commit, and the duplication it records is the bug travelling into hookdeck/n8n-nodes-hookdeck. The citation looks wrong, but correcting it is a separate change and I left it alone.
Draft, targeting main. Not merging.
🤖 Generated with Claude Code