feat(han-experimental): add the han-experimental plugin and review-skill-or-agent - #119
Draft
taminomara wants to merge 21 commits into
Draft
feat(han-experimental): add the han-experimental plugin and review-skill-or-agent#119taminomara wants to merge 21 commits into
taminomara wants to merge 21 commits into
Conversation
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
## 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
…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
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
…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
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
…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
added a commit
to taminomara/han
that referenced
this pull request
Jul 28, 2026
…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
…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
force-pushed
the
feat/review-skill-or-agent
branch
from
July 28, 2026 14:26
9a843ab to
f2c2034
Compare
taminomara
added a commit
to taminomara/han
that referenced
this pull request
Jul 28, 2026
…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
…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
…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
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 |
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. |
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 thehan-plugin-builderauthoring 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
haikutriage, 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-experimentalis opt-in: it depends onhan-core(the reviewer agents it dispatches) andhan-plugin-builder(the guidance it grounds against), and thehanmeta-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), andtemplate.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..claude-plugin/marketplace.json,CLAUDE.md,docs/skills/README.md, and the long-form docdocs/skills/han-experimental/review-skill-or-agent.md.Test Plan
detect-guidance-and-type-context.test.shpasses (48/48)./review-skill-or-agentagainst itself and applied the surviving findings.han-plugin-builderand reportsguidance-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:
Question for @mxriverlynn and other maintainers:
I've put this skill into new
han-experimentalplugin because I didn't want to addhan-coreas dependency ofhan-plugin-builder. Plugin builder can be vendored into other repos; vendoringreview-skill-or-agentwould require vendoring agents fromhan-core. Are we ok with this? If not, are we ok with addinghan-coretohan-plugin-builder's dependencies without vendoring it?Until this is resolved, the skill is fragile: it uses
claude plugin list --jsonto find installation path ofhan-plugin-builderand read guidance, and it depends on output format of--jsonflag, which is not formally specified.🤖 Generated with Claude Code
https://claude.ai/code/session_01L76cznjxv6sCjxk6AYTQNY