Skip to content

feat(hook-manager): add step-hook-manager-shim eval suite for Step P.3b (part of #1891) - #1892

Merged
potiuk merged 2 commits into
apache:mainfrom
Kaap10:test/isolated-setup-install-none-shim-eval
Oct 30, 2026
Merged

potiuk merged 2 commits into
apache:mainfrom
Kaap10:test/isolated-setup-install-none-shim-eval

Conversation

@Kaap10

@Kaap10 Kaap10 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add behavioral eval suite step-hook-manager-shim (7 cases) under tools/skill-evals/evals/setup-isolated-setup-install/ for Step P.3b (### Step P.3b-whole-user — Install the per-repo dispatcher + hook-manager shim (dispatcher flavour only)).
  • Verify that the agent correctly skips copying shims for managers with install.strategy: "none" (simple-git-hooks, cargo-husky, git-code-format-maven-plugin, git-build-hook) and installs the shim exclusively for managers requiring an install fix (prek).
  • Increment documented case count from 21 across 5 steps to 28 across 6 steps in tools/skill-evals/README.md and tools/skill-evals/evals/setup-isolated-setup-install/README.md.

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package (tools/*/ with pyproject.toml)
  • Groovy reference impl
  • Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
  • Documentation (docs/, README.md, CONTRIBUTING.md)
  • Project template (projects/_template/)
  • CI / dev loop (prek, workflows, validators)
  • Other:

Test plan

  • prek run --all-files passes
  • For Python packages touched: uv run pytest / ruff check / mypy passes
  • For Groovy bridges touched: command-line invocation tested end-to-end
  • For skill changes: eval suite passes for the affected skill
    (PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner tools/skill-evals/evals/<skill>/)
  • For skill behaviour changes: a new or updated eval fixture is included in this PR
    (a regression test for the bug fixed / the behaviour added — see CONTRIBUTING.md)
  • Other: sync-hook-managers.py --check passes cleanly (0 drift); test_sync_hook_managers.py passes (25/25); markdownlint passes (42 files, 0 issues); all 7 cases load and self-compare cleanly in runner.

RFC-AI-0004 compliance

  • HITL — any new mutation is gated on explicit user confirmation
  • Sandbox — no new unrestricted host access; network reach declared in the adapter
  • Vendor neutrality — placeholders (<PROJECT>, <tracker>, <upstream>, <security-list>) used in all skill / tool prose (the check-placeholders prek hook is the mechanical gate)
  • Conversational + correctable — agentic-override path documented if behaviour is adopter-tunable
  • Write-access discipline — no autonomous outbound messages; drafts only, sent on confirmation
  • Privacy LLM — private content does not reach a non-approved LLM; redactor invoked where needed

Linked issues

Part of #1891 (Part 1 of 2)
Refs #1838

Notes for reviewers (optional)

Follow-up Part 1 from PR #1838 merge feedback:

"test_none_strategy_generates_no_shim_case pins that the generated shim has no case for a none adapter. The decision that matters is the install step skipping those adapters, which lives in step-p's prose, so its real coverage would be an eval case for setup-isolated-setup-install."

The suite tests 7 concrete scenarios:

  1. case-1-prek-repo: whole-user dispatcher flavour, repo with prek.toml -> installed_shims: ["prek"]
  2. case-2-simple-git-hooks-repo: whole-user dispatcher flavour, repo with simple-git-hooks -> installed_shims: []
  3. case-3-cargo-husky-repo: whole-user dispatcher flavour, repo with .git/hooks/pre-push hook marker -> installed_shims: []
  4. case-4-maven-repo: whole-user dispatcher flavour, repo with pom.xml (git-code-format-maven-plugin + git-build-hook) -> installed_shims: []
  5. case-5-mixed-repos: whole-user dispatcher flavour, mixed repos (prek + simple-git-hooks + maven) -> installed_shims: ["prek"]
  6. case-6-unmanaged-repo: whole-user dispatcher flavour, standard git repo without hook managers -> installed_shims: []
  7. case-7-non-dispatcher-scope: operator picked per-project scope -> dispatcher_installed: false, installed_shims: []

Part 2 (live empirical execution of the two Maven plugins under dispatcher) will follow in a separate PR when a live Maven runner environment is available.

@github-actions github-actions Bot added family:setup setup-* skills family:tools tools/* family:docs Docs, MISSION.md, READMEs capability:platform Framework / agent substrate skills (install, verify, doctor, override, status, setup bootstrap) substrate:framework-dev Tool substrate: build / validate / eval the framework itself labels Oct 28, 2026
@Kaap10 Kaap10 changed the title feat(hook-manager): add step-hook-manager-shim eval suite for Step P.3b (#1891) feat(hook-manager): add step-hook-manager-shim eval suite for Step P.3b (part of #1891) Oct 28, 2026

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suite is well shaped and all 7 cases pass on the head, but its output spec restates Step P.3b's shim rule and the expected answers, so it would keep passing if the prose it is meant to cover regressed.

evals catch a skill that silently starts producing different output (tools/skill-evals/evals/setup-isolated-setup-install/step-hook-manager-shim/fixtures/output-spec.md:18)

Both signals catch the same class of regression: a skill that silently starts producing different output (eval failure) or that silently became materially more expensive to run (cost-table drift).

- Per Step P.3b item 2:
  - Copy `tools/agent-isolation/hook-manager-shim.sh` into `~/.claude/bin/<tool>` once for each adapter in the hook-manager registry with an install strategy other than `none` that a repository the operator works in uses

The runner appends output-spec.md to the extracted Step P.3b section (load_step_config, runner.py:199-202), so these lines put the rule under test into the prompt a second time, and line 21 gives the answer for every none manager by name. If item 2 of Step P.3b lost its strategy filter, the suite would still pass on this copy, which is the regression #1838 asked this suite to catch. I ran the suite on the head with claude -p and all 7 cases pass, but that cannot tell the two sources apart.

The model does need to know which managers are none, since Step P.3b points at the registry rather than naming them. Could you include the registry instead ("also_include": ["tools/hook-manager/registry.json"] in step-config.json) and cut this file down to the two field definitions and the JSON-only rule? Then a check that the suite fails with the strategy sentence removed from item 2 would show it covers the prose.

Flagged by both the primary and adversarial reviewers.


This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The findings
below are observations, not blockers; an Apache Magpie
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Apache Magpie handles maintainer review:
Contributing guide.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new head does what the earlier review asked: the registry now comes in through also_include, and output-spec.md is down to the field definitions and the JSON-only rule, so the suite reads Step P.3b's own prose against the live registry. The counts (28 cases across 6 steps) and the fixtures match the registry.


This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Magpie handles maintainer review:
Contributing guide.

@potiuk
potiuk merged commit 89a03ec into apache:main Oct 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

capability:platform Framework / agent substrate skills (install, verify, doctor, override, status, setup bootstrap) family:docs Docs, MISSION.md, READMEs family:setup setup-* skills family:tools tools/* substrate:framework-dev Tool substrate: build / validate / eval the framework itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants