Skip to content

feat(han-experimental): add the han-experimental plugin and review-skill-or-agent - #119

Draft
taminomara wants to merge 21 commits into
testdouble:mainfrom
taminomara:feat/review-skill-or-agent
Draft

feat(han-experimental): add the han-experimental plugin and review-skill-or-agent#119
taminomara wants to merge 21 commits into
testdouble:mainfrom
taminomara:feat/review-skill-or-agent

Conversation

@taminomara

@taminomara taminomara commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a new opt-in plugin, han-experimental, and its first skill, /review-skill-or-agent. The skill reviews a finished Claude Code skill or agent against the han-plugin-builder authoring guidance and a set of quality dimensions, then produces a severity-ranked, code-review-shaped report. Bloat and restatement are first-class corrective findings.

The orchestrator never reads the artifact itself. It resolves the target's type and guidance with a deterministic detector script, runs a short haiku triage, then dispatches a signal-scaled panel of fresh-context reviewers that each read the artifact as untrusted data. It consolidates and de-duplicates the findings, validates them with an adversarial pass, and renders the report from a template. It can halt (returning a "Review Halted" block instead of a report) when the guidance or the target is not reviewable.

han-experimental is opt-in: it depends on han-core (the reviewer agents it dispatches) and han-plugin-builder (the guidance it grounds against), and the han meta-plugin does not bundle it.

Key file changes

  • han-experimental/.claude-plugin/plugin.json: new plugin manifest (v0.1.0; depends on han-core and han-plugin-builder).
  • han-experimental/skills/review-skill-or-agent/SKILL.md: the skill itself, a seven-step review protocol with roster selection, three reviewer prompt blocks, and a halt procedure.
  • han-experimental/skills/review-skill-or-agent/references/: finding-classification.md (the consequence-class severity spine), bloat-classification.md (bloat tiers and the two-pass process), review-checklist.md (the per-lens checklist), and template.md (the report template).
  • han-experimental/skills/review-skill-or-agent/scripts/: detect-guidance-and-type-context.sh (type and guidance resolution) plus its test suite.
  • Wiring: .claude-plugin/marketplace.json, CLAUDE.md, docs/skills/README.md, and the long-form doc docs/skills/han-experimental/review-skill-or-agent.md.

Test Plan

  • detect-guidance-and-type-context.test.sh passes (48/48).
  • Ran /review-skill-or-agent against itself and applied the surviving findings.
  • Confirmed the detector resolves guidance against an installed han-plugin-builder and reports guidance-complete: true.

Results so far:

The skill performs better than a simple invocation of guidance skill with an instruction to review something. It spawns a panel of reviewers, similar to code-review. Additionally, it has instructions on finding bloat and duplication, which addresses common issues left after building/refactoring skills using LLMs.

This skill is work in progress:

  • Reviewing agents is untested, I've been focusing on skills at the moment.
  • Reviewing diffs is untested, size classification is not implemented yet, I've been focusing on reviewing the entire skill, not a change.
  • For now, skill dispatches up to 9 reviewers, 3 are mandatory. I plan to shrink the roster after size classification is implemented.
  • Process is not optimal at the moment: dedup is inefficient, adversarial review gets swamped with findings. I'm planning a rework of this area.
  • Issue grouping is to be revised: I'm not certain that both new issue tiers - legibility and bloat - earn their keep.
  • Initial triage is wobbly: runs on haiku and doesn't have verbatim prompt for sub-agent.
  • I didn't review docs or readme changes yet.

Question for @mxriverlynn and other maintainers:

I've put this skill into new han-experimental plugin because I didn't want to add han-core as dependency of han-plugin-builder. Plugin builder can be vendored into other repos; vendoring review-skill-or-agent would require vendoring agents from han-core. Are we ok with this? If not, are we ok with adding han-core to han-plugin-builder's dependencies without vendoring it?

Until this is resolved, the skill is fragile: it uses claude plugin list --json to find installation path of han-plugin-builder and read guidance, and it depends on output format of --json flag, which is not formally specified.

🤖 Generated with Claude Code

https://claude.ai/code/session_01L76cznjxv6sCjxk6AYTQNY

@taminomara taminomara mentioned this pull request Jul 12, 2026
Merged
@mxriverlynn

mxriverlynn commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

🤔 i need to think about the structure and dependencies a bit... this might be pointing to a more foundational layer than han-core, with a bit more restructure

mxriverlynn added a commit that referenced this pull request Jul 14, 2026
@mxriverlynn
chore(CI): lint, Bats tests, CI, and Dependabot (#120)
7dcba3b 
## Summary

Adds an npm-managed lint and test toolchain, CI, and Dependabot for the
han repo. One `npm install` pins every tool
locally (no global installs, no cross-project version drift) and
installs the git hook via the `prepare` script.

- **prek** (`@j178/prek`) runs the pre-commit hooks (drop-in for
`pre-commit`).
- **Prettier** formats Markdown, JSON, YAML, and JS: 120-column prose
wrap, ordered-list numbering preserved, `.github` PR/issue templates
unwrapped (GitHub renders newlines as `<br>`), and `docs/plans`,
`docs/research`, and the vendored assets excluded.
- **ShellCheck** (via `shellcheck-py`, no Docker) lints shell scripts.
- The `pre-commit-hooks` hygiene checks cover the rest; their
whitespace/EOL fixers exclude md/json/yaml/js so Prettier owns those
with no overlap, and `check-yaml`/`check-json` are dropped because
Prettier already validates those types.
- **Bats** runs `test/*.bats` in CI (not on commit); a sanity test
proves the harness.
- **CI**: a `lint` job (`npm run lint`) and a `test` job (`npm test`),
both on the pinned lockfile.
- **Dependabot**: one monthly PR bundling npm + GitHub Actions +
pre-commit updates via a multi-ecosystem group, with a 30-day cooldown
on new releases to reduce supply-chain risk (security updates are exempt
from the cooldown).

Mitigates #113 for this repo. Should be merged before #119.

## Note on the `lint` CI job

This PR adds the tooling only. Running `npm run lint` reflows the
existing Markdown across the repo; that mechanical reflow is
intentionally **not** included here, so the `lint` job will report
changes until it is applied in a separate pass. The `test` job is green.

## Follow-ups for a maintainer

- Run `npm run lint` and commit the repo-wide Prettier reflow (and
re-run so the `lint` job goes green).
- Resolve two ShellCheck findings, or add `# shellcheck disable`
directives: `init-guidance.sh:55` (SC2016) and
`upload-screenshots.sh:136` (SC2034). Both look benign.
- Fix the broken symlink `.claude/rules/plugin-entity-taxonomy.md` that
`check-symlinks` flags.
- Enable *Settings → Code security → Dependabot security updates* so
security patches bypass the 30-day cooldown.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01KFKuiUmQ9QinkSt9LbE7hc
taminomara and others added 19 commits July 15, 2026 04:10
@taminomara @claude
feat(han-experimental): add the han-experimental plugin and review-sk…
4c2e75e 
…ill-or-agent

Introduce the opt-in han-experimental plugin carrying review-skill-or-agent: a
skill that reviews a finished Claude Code skill or agent against the
plugin-authoring guidance and quality dimensions (bloat and restatement first)
and produces a severity-ranked, code-review-shaped report. It resolves the
target's type, grounds against the type-appropriate guidance (halting when it is
absent or incomplete), and dispatches a signal-scaled roster of fresh-context
reviewers handed the artifact as untrusted data, then validates the findings
with adversarial-validator.

Wiring:
- register han-experimental in .claude-plugin/marketplace.json (v0.1.0)
- describe it in CLAUDE.md (plugin family, repo layout, shipping/deps, index rule)
- add its section and skill entry to docs/skills/README.md
- add the long-form doc docs/skills/han-experimental/review-skill-or-agent.md

Depends on han-core (the reviewer agents it dispatches) and han-plugin-builder
(the authoring guidance it grounds against). Opt-in: not bundled by the han
meta-plugin.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L76cznjxv6sCjxk6AYTQNY
@taminomara @claude
fix(review-skill-or-agent): stabilize triage and right-size the roster
0265723 
Rework Step 3 so the same artifact yields the same reviewer roster and
small skills stop drawing specialists they do not need:

- Replace the five free-form triage signals with a pinned rubric
  (references/triage-rubric.md): each signal has a floor that excludes
  the trivial baseline, plus a fixed per-signal yes/no output format.
- Recalibrate the roster gates toward exclusion (information-architect
  at reference-count >= 2, security default-out, a "fewer is better"
  tie-break) and resolve the line-84/93 and line-97 contradictions.
- Wire the conformance reviewer's specialist-backstop deterministically:
  the orchestrator hands it the list of specialist lenses left off the
  roster, so it backstops exactly those instead of conditioning on a
  roster it never sees. No added load when the specialist is present,
  and it no longer contradicts the Step 5 de-dup rule.
- Drop the hard-coded haiku model override at the triage dispatch so it
  inherits the session model (skills pass no model override).
- Correct the skill/tool-seam checks to the current context-injection
  loader model: load-blockers are the refused constructs, unallowlisted
  pipe/chain stages, and missing non-zero-exit guards, not output size;
  "| head" is a truncation nit, not a guard.
- Fix a self-load hazard: the literal bang-backtick pattern in SKILL.md
  prose is executed by the loader, so reword it to "bang-backtick".

Add a seam checklist item for bang-backtick invocation safety, and keep
the long-form doc in sync.

Runtime-validated: triage signals stable 5/5 across two targets; lean
rosters confirmed; the always-on conformance backstop caught both
planted seam defects with the seam reviewer deselected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxVNiTYhXQtAoBDy4GRJQw
@taminomara
Skill reviewer: harden triage
6ba93b7
@taminomara
Skill reviewer: remove triage and diff sub-agents
77e69f0
@taminomara @claude
feat(review-skill-or-agent): discover the review target and load bran…
3c54d2c 
…ch context

Give /review-skill-or-agent the branch-context and artifact-discovery front end
that /code-review already has. When no target is named, discover the skill or
agent under review from the current branch's committed or uncommitted changes,
a named pull request, or a candidate list, and load branch-level intent context
(PR description, commit messages, a matching planning doc, a repo-root PR-body
file) to hand reviewers as untrusted data.

- Add detect-git-context.sh, byte-identical to code-review's shared Layer-1
  detect-review-context.sh contract, plus a crafted-fixture test.
- Rewrite Step 1 for target resolution by precedence (named artifact -> named
  PR -> branch discovery) with named Mode A/B/C routing and changed-file ->
  artifact mapping.
- Add Step 1.5 to load and bound branch context, gated so it loads only when
  the target is one the branch changed, with the untrusted-data discipline
  governing the read-and-condense act.
- Deliver branch context as a distinct Block D under BEGIN/END untrusted
  markers, kept distinguishable from the artifact.

No new tool grant; candidate discovery uses git ls-files under the existing
Bash(git *) grant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hdh6DsXbsUYgPv58m5ZF4D
@taminomara @claude
test(review-skill-or-agent): port detect-git-context test to Bats
186605a 
Move the hand-rolled crafted-fixture detect-git-context.test.sh to
test/detect-git-context.bats, where `npm test` (bats --recursive test/) picks
it up alongside test/sanity.bats. The six Layer-1 contract behaviors and their
git fixtures are preserved verbatim; only the runner changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hdh6DsXbsUYgPv58m5ZF4D
@taminomara @claude
test(review-skill-or-agent): port detect-guidance-and-type-context te…
812bda2 
…st to Bats

Move the hand-rolled crafted-fixture detect-guidance-and-type-context.test.sh
to test/detect-guidance-and-type-context.bats, where `npm test`
(bats --recursive test/) picks it up. All 48 cases are ported verbatim: the
frontmatter-shape classification, guidance-root resolution across the sibling /
installed / vendored layouts, id-keyed selection, emit() flattening, and the
wedged-claude liveness bound. The four environment-conditional cases (root,
missing timeout, claude-on-PATH) use Bats `skip`. Only the runner changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hdh6DsXbsUYgPv58m5ZF4D
@taminomara
WIP
723038b
@taminomara
wip
3bc603b
@taminomara
WIP
2896d42
@taminomara
WIP
9582950
@taminomara
WIP
c7b595b
@taminomara
WIP
daa2573
@taminomara
WIP
196e7d4
@taminomara
WIP
901b76a
@taminomara
WIP
41511cd
@taminomara
WIP
5ff2db4
@taminomara
WIP
f92e955
@taminomara
WIP
a36b238
@taminomara taminomara mentioned this pull request Jul 28, 2026
Merged
taminomara added a commit to taminomara/han that referenced this pull request Jul 28, 2026
@taminomara @claude
feat(code-review, test-planning): detect the diff base across multipl…
a24a802 
…e remotes and git-flow

The code-review and test-planning git-context detectors chose the branch to
diff changes against from origin/HEAD alone. On a fork whose origin default is
stale, on a git-flow branch cut from an integration branch, or on a branch that
merged another branch in, that base was wrong, so the review or test plan was
scoped to an inflated, empty, or mis-attributed diff.

Port review-skill-or-agent's base selection to both detectors: the base is now
the candidate whose fork point is nearest the current commit, measured along the
branch's own first-parent line, chosen across a multi-remote candidate pool.

- Candidate pool: every remote's declared default branch plus well-known
  trunk/integration names (main, master, trunk, mainline, next, develop, devel,
  development, default, dev), looked up as local and per-remote branches;
  nearest own-line coincidence wins, declared defaults break ties.
- A candidate the branch merely merged in cannot win on its absorbed commits.
- Output contract shape unchanged; fail-open to `default-branch: none` preserved.
- Adds test/detect-review-context.bats and test/detect-test-context.bats
  (16 tests each) under test/, which these detectors previously lacked.

Completes the OI-1 follow-up flagged in review-skill-or-agent (PR testdouble#119),
which intentionally ran ahead of these copies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NfTexc8iv7XAqt3ksH9zNP
taminomara added a commit to taminomara/han that referenced this pull request Jul 28, 2026
@taminomara @claude
feat(code-review, test-planning): detect the diff base across multipl…
80244a8 
…e remotes and git-flow

The code-review and test-planning git-context detectors chose the branch to
diff changes against from origin/HEAD alone. On a fork whose origin default is
stale, on a git-flow branch cut from an integration branch, or on a branch that
merged another branch in, that base was wrong, so the review or test plan was
scoped to an inflated, empty, or mis-attributed diff.

Port review-skill-or-agent's base selection to both detectors: the base is now
the candidate whose fork point is nearest the current commit, measured along the
branch's own first-parent line, chosen across a multi-remote candidate pool.

- Candidate pool: every remote's declared default branch plus well-known
  trunk/integration names (main, master, trunk, mainline, next, develop, devel,
  development, default, dev), looked up as local and per-remote branches;
  nearest own-line coincidence wins, declared defaults break ties.
- A candidate the branch merely merged in cannot win on its absorbed commits.
- Output contract shape unchanged; fail-open to `default-branch: none` preserved.
- Adds test/detect-review-context.bats and test/detect-test-context.bats
  (16 tests each) under test/, which these detectors previously lacked.

Completes the OI-1 follow-up flagged in review-skill-or-agent (PR testdouble#119),
which intentionally ran ahead of these copies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NfTexc8iv7XAqt3ksH9zNP
@taminomara
taminomara force-pushed the feat/review-skill-or-agent branch from 9a843ab to f2c2034 Compare July 28, 2026 14:26
taminomara added a commit to taminomara/han that referenced this pull request Jul 28, 2026
@taminomara @claude
feat(code-review, test-planning): detect the diff base across multipl…
d46c1f8 
…e remotes and git-flow

The code-review and test-planning git-context detectors chose the branch to
diff changes against from origin/HEAD alone. On a fork whose origin default is
stale, on a git-flow branch cut from an integration branch, or on a branch that
merged another branch in, that base was wrong, so the review or test plan was
scoped to an inflated, empty, or mis-attributed diff.

Port review-skill-or-agent's base selection to both detectors: the base is now
the candidate whose fork point is nearest the current commit, measured along the
branch's own first-parent line, chosen across a multi-remote candidate pool.

- Candidate pool: every remote's declared default branch plus well-known
  trunk/integration names (main, master, trunk, mainline, next, develop, devel,
  development, default, dev), looked up as local and per-remote branches;
  nearest own-line coincidence wins, declared defaults break ties.
- A candidate the branch merely merged in cannot win on its absorbed commits.
- Output contract shape unchanged; fail-open to `default-branch: none` preserved.
- Adds test/detect-review-context.bats and test/detect-test-context.bats
  (16 tests each) under test/, which these detectors previously lacked.

Completes the OI-1 follow-up flagged in review-skill-or-agent (PR testdouble#119),
which intentionally ran ahead of these copies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NfTexc8iv7XAqt3ksH9zNP
@taminomara @claude
fix(review-skill-or-agent): freeze HEAD and cover second-parent base …
0465a4e 
…detection

Address code-review findings on the base-branch detector:
- Freeze HEAD to a commit up front (HEAD_SHA) so every candidate and the
  changed-files diff measure against the same point, not a live ref re-read
  per candidate (SUGG-002).
- Expand the selection comment with the reasoning the count rests on:
  first-parent monotonicity, and that "^candidate" excludes by the candidate's
  full reachability while "--first-parent" limits only the walk from HEAD
  (SUGG-001).
- Add a regression test for a candidate that reaches an own-line commit only
  through its own merge second parent, the topology that distinguishes
  full-reachability exclusion from a first-parent-limited one (WARN-001).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NfTexc8iv7XAqt3ksH9zNP
mxriverlynn added a commit that referenced this pull request Jul 29, 2026
@mxriverlynn
feat(code-review, test-planning): detect the diff base across multipl…
9a50331 
…e remotes and git-flow (#151)

## Summary

The `code-review` and `automated-test-planning` git-context detectors
pick the base branch to diff against from `origin/HEAD` alone. On a fork
whose `origin` default is stale, on a git-flow branch cut from an
integration branch, or on a branch that merged another candidate in,
that base is wrong, so the review or test plan is scoped to an inflated,
empty, or mis-attributed diff.

This ports `review-skill-or-agent`'s base detection to both detectors.
The base is now the branch's own line of work: the candidate whose fork
point is nearest the current commit, measured along first-parent
history, chosen across a multi-remote candidate pool.

**Rebased onto `han-v5.0.0-alpha-1`** (`main` is frozen until v5). On
the v5 line the `test-planning` skill was split, so the test detector
now lives at
`han-coding/skills/automated-test-planning/scripts/detect-test-context.sh`;
a small adaptation commit repoints the moved script's Bats test and the
detector sync-note comments to the new path.

## What changed

- **`detect-review-context.sh` (code-review), `detect-test-context.sh`
(automated-test-planning).** Replace the single `origin/HEAD` lookup
with a candidate pool: every remote's declared default branch, plus
well-known trunk and integration names (`main`, `master`, `trunk`,
`mainline`, `next`, `develop`, `devel`, `development`, `default`,
`dev`), looked up as local branches and as each remote's tracking
branch. The nearest own-line coincidence wins; a declared default breaks
an equal-distance tie; refs are deduplicated. A candidate the branch
merely merged in cannot win on its absorbed commits, because those
commits are not on the branch's first-parent line.
- **Output contract unchanged.** The scripts emit the same `key: value`
keys, still fail open to `default-branch: none`, and always exit 0. No
consuming skill prose or long-form docs change.
- **Tests.** Add `test/detect-review-context.bats` and
`test/detect-test-context.bats` (16 cases each), which these detectors
previously lacked: stale-origin-vs-fresh-remote, git-flow integration
branch, local trunk, merged-in demotion, unrelated-candidate exclusion,
none / unborn / detached fallbacks, and the declared-default tie-break
priority.

## Scope

The `build-target-diff.sh` change-scope helper and the `SKILL.md`
PR-base override are `review-skill-or-agent`-specific and stay on #119.
This PR is the shared detector plus its tests only.

## Testing

- Both Bats suites pass (16 + 16 cases) against the moved script path,
alongside the existing `sanity.bats`.
- `prek run --all-files`: every hook passes.

## Related

Completes the OI-1 follow-up flagged in #119, whose
`detect-git-context.sh` intentionally ran ahead of these copies. Once
both land, all three detectors — code-review, automated-test-planning,
and review-skill-or-agent — match, and #119's "intentionally ahead"
header note gets reconciled to name all three.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_017NEQjJUi7ZHVXGfVGXKUVJ
@mxriverlynn

mxriverlynn commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

@taminomara i'm looking at this again, in the context of discussion in #152 - and i really want to get the review-skill-or-agent merged in. but i agree with your desire to not have it depend on han-core for the agent dispatch.

i'm wondering: maybe there's a small set of custom agents that can be defined in han-plugin-builder, specific to doing this review? or maybe there's another means of accomplishing the same thing without having to bring in custom agent definitions? i'm not sure about the solution, but i'm definitely sure we need that review skill in the plugin

@taminomara

taminomara commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

This is a possibility. The skill is early alpha, though, and I'll need more usage data and AB testing to swap agents without loosing depth.

Right now, this skill spawns up to 8 reviewers, which is far too many. I've tried shrinking the roster, and managed to remove 1 agent without loosing quality, and one more can be removed with little quality degradation. Maybe having custom agents can help to reduce the roster even more.

There's one concern with this approach, though. Right now, junior-developer yields a lot of findings, on par with the agent that checks the skill against the guidance. So, this one I'd like not to lose.

@taminomara

taminomara commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

In any case, I'd like to close some feature gaps and merge this skill to han-experimental first, so that people can try it. We can refine the skill and move it to han-plugin-builder later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@taminomara @mxriverlynn