Multi-harness support: skill adapters + pluggable scan engines (Hermes, Copilot CLI, Codex)

[unsupportedpastels]

What

Make VulnHunter harness-agnostic while leaving the Claude Code path byte-for-byte identical. The same scanner skill, batch harness, and headless runtime can now be driven by other agent CLIs, selected per run:

Harness	Skill install	Headless driver	Status
claude-code	./install.sh (unchanged)	Claude Agent SDK (unchanged)	reference
hermes	./install.sh --target hermes	hermes chat -Q -s vulnhunt	experimental
copilot	./install.sh --target copilot	copilot -p …	experimental (flag surface unverified — CLI not available during development)
codex	./install.sh --target codex	codex exec -C … -s workspace-write	experimental (flags verified on codex-cli 0.147.0)

Design

Three layers, each independently extensible:

1. Skill rendering — the repo-root skill directories stay the single source of truth. adapters/<name>/adapter.json declares declarative transforms (substitutions, terminology overlays, frontmatter additions); scripts/render_skills.py (stdlib-only) renders dist/<name>/. The claude-code adapter is the identity transform and tests/test_render_skills.py enforces byte-identity with the sources, so any source edit is a deliberate prompt change every adapter inherits. Missing find-strings fail the render loudly, so an upstream prompt edit that breaks an adapter shows up in tests, not mid-scan.

2. Headless engines (vulnhunter-agent/agent/engines/) — a ScanEngine protocol with the existing SDK path (claude-code) plus subprocess engines (hermes, copilot, codex). All engines share the SDK path's pre-staging contract (pre-created results dir, git context, pre-resolved-metadata kickoff, audit events) and judge success by the results-directory contract, never stdout. The claude-code path is behaviorally untouched — __main__ still calls the module-level run_vulnhunt, so existing test seams hold. Selected via [scan] engine in the agent TOML (default claude-code).

3. Benchmark harness engines — VULNHUNT_HARNESS_ENGINE parametrizes batch scans; VULNHUNT_HARNESS_JUDGE_ENGINE parametrizes the judge independently (scan under one engine, judge under another to avoid self-preference bias). The claude argv is byte-identical to before (snapshot tests).

Notable adaptation details:

• Hermes runs every top-level delegate_task in the background; a headless session that concludes early kills in-flight subagents mid-scan. The hermes kickoff/skill therefore teaches an explicit wait protocol (poll action: "list" until every child completes). Found and fixed during live validation.
• Codex has no subagent tool, so its adapter rewrites Phase 2 fan-out into sequential class-group passes (the phase files are already class-partitioned) and corrects the ORCHESTRATOR role accordingly.
• No model enforcement: engine runs never gate on model family, and the non-Claude renders turn the interactive model check into a proceed-on-selected-model calibration notice. The claude-code render keeps the upstream STOP gate byte-identically.

Untouched by design: scan_manifest.json schema, results-directory layout, publish/issues/audit/verify stages, ground-truth data, and the vulnhunter-fix package (its validators are already provider-neutral).

How I validated

• All existing suites green: harness 177 passed, vulnhunter-agent 1147 passed (incl. 20 new engine tests), vulnhunter-fix 549 passed / 10 skipped, plus a new stdlib-only root test suite (12 golden/drift tests) wired into CI as a skill-renderer job.
• Renderer --check --baseline <dir> verifies a fresh render matches a supplied baseline (dist/ is gitignored and rendered fresh, so there is no committed tree to diff against); dist/claude-code byte-identity to the sources is enforced by tests/test_render_skills.py.
• Live on a local Hermes v0.20.1 install: skill discovered via hermes skills list, headless -Q contract verified, skill_view resolves ${HERMES_SKILL_DIR}, and a scoped Phase-1 recon run on a synthetic repo produced a methodology-conformant phase1_output.md (all planted sinks found, test dirs excluded, partition table correct). A full interactive scan of a real repo on a non-Opus model completed Phases 1–2 at the full 7×3+1 fan-out with the merged candidate inventory matching the methodology.
• Codex bundle install + headless codex exec skill discovery verified against codex-cli 0.147.0.
• Copilot adapter ships documented-but-unverified flags (no binary available); marked experimental with a verification note.

Caveats / follow-ups

• Non-Claude adapters are experimental until benchmarked against the ground-truth corpus (VULNHUNT_HARNESS_ENGINE now makes that a config choice).
• docs/ADAPTER_GUIDE.md documents how to add the next harness and the invariants to preserve.

Thanks for considering this — happy to split or reshape the commits if maintainers prefer a different slicing.

---

Update — review remediation (commits 81dcb41, f15bb39)

Addresses @schenksj's review. All 11 inline threads replied to and resolved.

Blocking

1. Engine failures no longer pass as clean scans. Success is now contents-based — runner._results_dir_is_complete() requires the results dir to contain the skill's README.md report (the engines pre-create the dir, so existence proves nothing); any non-zero exit is a failure. Lands once in the new SubprocessEngine base.
2. COPY_IGNORE fixed — correct (dir, [name]) shape via render_skills.is_ignored(); __pycache__/*.pyc/.venv no longer leak into bundles. Regression-tested.
3. --check reworked to compare against a required --baseline (dist/ is gitignored); errors without one; docstring corrected.
4. hermes/copilot/codex collapsed onto SubprocessEngine (agent/engines/_subprocess.py); ~270 duplicated lines removed; EngineError moved to engines/__init__.

Non-blocking follow-ups also done: engine_extra_args as a native TOML list (shlex.split fallback); harness config validates at use, not import; scan.py/judge.py read the engine at call time (env-var default path now tested); engine_timeout_seconds <= 0 = "no timeout"; dead code removed (ScanOutcome, copilot _EFFECTIVE_TOOLS); drift tripwire now requires count on every non-optional substitute; _apply_drop fails loudly; docs/engine-matrix.md + ENGINES.md document the totals/timeout/success-contract limitations.

Tests: vulnhunter-agent 1167 passed, harness 180 passed, root renderer 18 passed. Engine contract (success / empty-dir / non-zero / timeout / extra-args) parametrized across all three subprocess engines.

[CLAassistant]

All committers have signed the CLA.

[schenksj]

First off — thank you for this. It's a genuinely thoughtful contribution: the three-layer split (skill rendering / headless engines / benchmark harness) is the right decomposition, the byte-identity golden test on claude-code is a clever invariant that protects the existing path far better than a review checklist would, and the PR description is unusually honest about what's verified versus what isn't. Documenting Copilot's flag surface as unverified rather than quietly shipping it is exactly the right call, and docs/ADAPTER_GUIDE.md means the next harness won't have to reverse-engineer your reasoning. I really enjoyed reading it.

I'm requesting changes for one correctness issue and a couple of one-liners, plus a duplication concern that I think is worth resolving before merge specifically because it multiplies the cost of fixing the correctness issue. None of it is structural — the design holds up.

---

Evidence of passing

The one thing I can't verify from here: the repo's own test workflow never ran on this PR. The only checks present are Mend License, Mend Security, Socket ×2, and CLA. The tests workflow (the pytest matrix from #27) is being held for maintainer approval since this is a fork PR from a first-time contributor — and that includes the new skill-renderer job this PR adds. A maintainer will need to approve the run before the claimed numbers are CI-backed.

I did reproduce them locally (Python 3.12, uv), and they hold up:

Suite	PR claim	Reproduced
harness	177 passed	✅ 177 passed
vulnhunter-agent	1147 passed	✅ 1146 passed, 1 skipped
root renderer tests	12	✅ 12 passed
render_skills.py --adapter all	clean	✅ clean
vulnhunter-fix	549 / 10 skipped	not run — untouched by the diff

So the test claims are accurate. Thanks for putting real numbers in the description; it made this much faster to check.

---

Blocking

1. Non-zero engine exits are reported as successful scans. Details inline on hermes.py. Short version: the engines pre-create the results directory, and _find_results_dir judges success by that directory's existence — so it always finds the one the engine just made. The returncode != 0 and found is None guard is unreachable in production, and a crashed or OOM-killed engine returns an empty results dir as a clean scan. For a security scanner, "found nothing" and "died before looking" have to be distinguishable, so I'd like this one settled before merge.

2. COPY_IGNORE never fires (render_skills.py:224) — a one-line argument-shape bug that lets __pycache__/.venv/.pyc into rendered bundles. Inline, with a repro.

3. --check can't work as documented. This PR gitignores dist/, but check_render compares a fresh render against the committed dist/<name> — with nothing committed it always reports "not rendered yet" and exits 1. CI correctly doesn't use it, but the module docstring and the PR's validation section both cite it as a verification mechanism. Either point it at a supplied baseline or drop it; right now it's a promise the repo layout can't keep.

4. Please collapse the three subprocess engines before merge. Flagged inline on hermes.py:107. This is normally a "nice to have," but here hermes.py / copilot.py / codex.py contain the same ~90-line run_scan three times, which means issue #1 above has to be fixed three times and can regress independently in each. A SubprocessEngine base with _binary_name / _skill_path / _build_command / _build_kickoff hooks removes ~200 lines and makes the fix land once.

---

Non-blocking, worth a follow-up

Two independent engine registries. The agent has ENGINE_NAMES + get_engine; harness/local_harness/config.py has its own ENGINES dict; and the per-engine argv knowledge is duplicated between harness/scan.py/judge.py and the agent engine modules (hermes' file,delegation toolsets, codex's -s workspace-write, etc. appear in both places). Skill paths ~/.{claude,hermes,copilot,codex}/skills/vulnhunt are hardcoded in four files plus install.sh. Adding a fifth harness means editing 6+ locations, which works against the extensibility the PR is aiming for. One shared table (name → binary, skills dir) would fix it.

Reference engine is unexercised in production. __main__.py branches on engine.name == "claude-code" and calls run_vulnhunt directly, so ClaudeCodeEngine only ever runs in tests. The monkeypatch-seam rationale is sound and I'd keep the behavior for this PR — but it means the new protocol's reference implementation has no production consumer. Worth moving the test seam later so the abstraction is actually load-bearing.

Test coverage gaps (none blocking, but each is a small addition):

• harness/scan.py does from .config import ENGINE — bound by value at import, so importlib.reload(config) in test_default_engine_is_claude_code doesn't propagate to scan.ENGINE. The tests don't notice because they always pass engine= explicitly, leaving the env-var → scan_folder() default path untested.
• Copilot and Codex have 2–3 tests each versus Hermes' 7 — no timeout, non-zero-exit, or extra-args coverage, despite being character-for-character the same code paths. (Folding them into a shared base per #4 would let one test class cover all three.)
• totals_out is accepted and ignored by all three subprocess engines, so cost/token totals silently report zero for non-Claude runs. Worth a line in docs/engine-matrix.md even if implementing it is out of scope.
• The timeout path's proc.kill() reaches only the direct child; hermes/codex subagent processes are orphaned. Also engine_timeout_seconds = 0 currently means "time out instantly" rather than "no timeout" and isn't validated — the timeout test depends on that reading.
• Only 1 of hermes' 11 transforms sets count. Literal find/replace over prompt prose without an expected count means a source edit that duplicates or partially rewords a phrase renders cleanly but transforms wrongly. Requiring count on every non-optional substitute would make that tripwire actually trip — which is the whole point of the drift tests, and they're a good idea worth strengthening.

Small dead-code items: ScanOutcome and _EFFECTIVE_TOOLS (copilot) are defined but never used — inline notes on both. _HERMES_SKILL_CANDIDATES is a one-element tuple named "candidates".

---

Thanks again for the care that went into this, and for offering to reshape the commits — the current slicing reads well, so I wouldn't change it. Happy to re-review quickly once the results-contract issue and the two one-liners are addressed; I don't think any of this is far from done.