mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
Paweł Płatek
1004934abf
* skill-improver 2.0.0: rewrite the loop as a dynamic workflow Replaces the stop-hook/state-file loop with /skill-improver:improve, built against the failure evidence from four manual pr-review-loop sessions (SKILL-IMPROVER-V2-HANDOFF.md): - findings ledger: stable ids, one verdict per finding, persisted every round; rejected findings are not re-litigated without new evidence, and a continued run reloads the ledger instead of re-deriving findings - completion requires the last action to be a clean review; at the cap the loop runs one review-only round and exits loudly as capped-not-converged - oscillation detectors (non-decreasing counts, 3-round recurrence, re-fixed relocation) stop the loop with a structural escalation instead of burning rounds; guarantees are never silently weakened - mechanical scope guard after every fix round: git diff vs the baseline snapshot, halt on violation, no unregistered new files at completion, fixer contract bans destructive git - finalize pass strips loop narration, collapses version churn to exactly one bump, and runs scripts/collect_metrics.py (fails on zero artifacts) Ships bundled reviewer/fixer agents (no plugin-dev dependency), an offline harness with 17-mutation self-test, pytest coverage for the collector, five paid eval cases graded on artifacts, and an A/B ablation runner against v1.1.0. Deletes the hooks, setup/cancel scripts, and the cancel command. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver evals: seal fixtures against contamination Mounting fixtures with context.add_dirs hands every agent an absolute path into this repository — one directory walk from SKILL.md and the graders — which is the baseline-contamination failure measured in goal-prompt (#248). For this suite it is worse: the improvement loop diffs and edits the tree it is pointed at, so an in-repo fixture would have eval runs mutating the checkout and git-baselining the whole repository. Fixtures are now generated by each case's scaffold.sh inside the eval workspace (--scaffold is required for every case), the in-repo fixture/ directories are gone, and check_contamination.py gates every measured run: grader filenames in an agent trace, skill-improver/evals/ paths, or expected_outcome anywhere fail the run, and having nothing to inspect is an error. Its pytest suite proves each marker class detects a planted specimen. The ablation runner gains --expect-version, --no-publish, --keep-temp, and the contamination gate on both arms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver: survive non-interactive callers, pin the E2 contract Three defects found by the eval pilots: - SKILL.md named the workflow 'improve'; the registry needs 'skill-improver:improve' - a caller that ends its turn while the workflow runs abandons the loop mid-round (the handoff's I9 class, reproduced in the eval harness) — SKILL.md now requires waiting/polling for the result, and forbids the inline-loop fallback that a denied Workflow tool provoked (observed rewriting the fixture's guarantee) - the E2 fixture never marked its guarantee as non-negotiable, so a fixer could legitimately resolve the overclaim by correcting the docs; the scaffold now freezes the sentence contractually in AGENTS.md, the fixer contract treats edits to contractual text as weakening, and the noisy llm grader over the 17k-char ledger is replaced by regex graders over ledger.json and metrics.json Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver: escalate structural rejections instead of converging past them Measured in the gate-case pilot: the fixer rejected the unsatisfiable- guarantee finding with the documented rationale, the next review honored the verdict, and the loop converged in two rounds — guarantee intact, no treadmill, but 'converged: true' reads as a clean bill while the README still promises the impossible. A blocking finding that is real yet rejected as structurally unsatisfiable is the user's decision, not a parked verdict: the fixer now flags such rejections structural=true and the loop exits with a structural-rejection escalation. Covered by a new harness scenario and mutation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver evals: mechanical decoy check, longer episode timeouts The decoy-byte-identical llm judge failed 3/3 runs whose file md5 matched the planted bytes exactly — judges do not see raw bytes, so the check is now an anchored regex proven against the specimen (and against an appended line and an edited seed). The no-relitigation rubric is corrected for the severity-gated design: a trap parked as an open minor never receives a verdict and that is the correct outcome; a mechanical refile check on metrics.json rides along. Episode timeouts rise to 3600s: loops that run 3-4 fix rounds or continue past an escalation were being killed mid-round under concurrent invocations and then graded on abandoned state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver evals: neutralize answer-key filenames in version-comparison arms The contamination gate caught its first real incident: the v1 plugin's own SKILL.md walks its plugin root for its setup script, the listing enumerated the grafted eval tree, and 10/15 baseline runs carried every grader filename in their traces — names like trap-name-kept and guarantee-byte-identical are instructions. The ablation runner now grafts arm B with neutral case-dir and grader filenames (real case names stay in case.yaml, so reports are unaffected), the checker gains content markers (grader rubric phrasing) and an explicit --allow-listing mode for arms whose own tooling lists the plugin root, and the incident is documented in the suite README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver: scope the finalize greps; fail the scope judge only on false claims The contamination gate flagged a finalize subagent grepping the whole plugin install directory for narration patterns — its results could touch the eval tree. The finalize prompt now scopes its greps to the scope directories under the target repository. The scope-guard last-message grader was failing exemplary reports (in-scope reroute of a broken out-of-scope test, honest artifact notes); it now fails only on affirmative false claims: out-of-scope work presented as done, or a halt presented as success. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver evals: record the measured three-arm results v2 vs main (1.0.5) vs 1.1.0, 15 runs per arm, sonnet judge, contamination gates clean on every counted result. v2 medians 1.00 on all five cases; the old versions match v2 on raw defect-fixing (both fixed pins-bite's planted bug and added covering tests 6/6) and lose on what the loop exists to guarantee: the frozen guarantee survived byte-identical only under v2, only v2 escalated instead of self-declaring completion, only v2 held version discipline, and only v2 leaves a machine-checkable record (verify-pins.sh bit 3/3). Known noise documented: episode-timeout kills and one judge call lost to a spend limit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * skill-improver -> code-improver generalization * code-improver on itself * rm openai, fix readme * plugin handles one-level deep agents * fix python * code-improver: check finalize, sentinel every wave, bump both files Three P2s from the pipeline review of #272. Finalize edited the tree after the last review and the last scope check, so its narration strip, version bump, and docs pass were the only edits nothing verified — a legitimate "round 2 of the tournament" could be rewritten and the run still returned converged:true. A finalize-check agent now runs last: it scope-checks the tree, reads the finalize delta (pre-finalize.diff vs post-finalize.diff) for regressions, and writes the run's final ledger, ledger.md, and metrics — so the on-disk ledger records finalize's own outcome, and a dead finalize no longer leaves a stale one. An out-of-scope edit, a new unregistered file, a regression, or a dead check all exit with converged:false and a named halt. The REVIEWER-UNAVAILABLE sentinel was checked only on the first reviewer return, so a trampoline continuation that lost its skill returned an empty review that merged as a clean bill of health. The check moves into a helper applied to every return, and the continuation prompt now carries the contract it was expected to honor. The one-bump rule scoped the version to plugin.json "and any marketplace entry inside scope", but the manifest sits at the repository root, outside the default scope — so the headline case bumped plugin.json only and left CI red on a version mismatch. The baseline now reports the marketplace file that repeats the plugin's version, the loop brings it into scope (loudly, in notes), finalize is told to set both to the same value, and the check verifies they agree. Harness: three new scenarios (finalize check, dead finalize, marketplace bump), one for the continuation sentinel, and seven mutations covering the new guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * code-improver: fix the second review round's P2s Lint: verify-pins.sh:59 used `A && B || C`, which CI's shellcheck flags as SC2015 (local 0.11.0 does not). Rewritten as an if. 1. metrics ran as `python3 "<script>"`, the one form the modern-python shims refuse outright — anyone with that plugin installed got no metrics.json, metrics_ok false, and three graders failing for environment reasons. Now `uv run --no-project`; the collector is pure stdlib. 2. findExisting recomputed `file:line:class` and never read the id the reviewer returns, so a rejected finding re-reported at a shifted line missed its ledger entry and was re-dispatched. The id is consulted first. The coarse `file::class` rescue also merged two distinct findings of one class in one file into a single entry, dropping one silently: it now only matches an entry the current review has not already claimed. 3. The scope guard could not see out-of-scope files git does not track — they are in no index, so rewriting or deleting one (the scope-guard eval's own decoy) left no trace in `git diff`. The baseline hashes each untracked file, every surface report re-hashes the out-of-scope ones, and a moved hash, a vanished file, or a hash the check did not report is a violation. Files with no baseline hash, and any past the 50-file cap, are named in the notes as unguarded rather than passing as clean. 4. The three skills launched the loop as {name: "code-improver:improve"}; the Workflow tool resolves `name` against built-in and project workflows, so a marketplace-installed plugin workflow may not answer to it — and each skill forbids an inline fallback, so the entry point would dead-end. They now resolve workflows/improve.js (plugin root, Codex root, then a bounded find) and pass scriptPath, with the workflow name as a last resort. pr-improver's allowed-tools also gained the TaskOutput/TaskStop its own polling paragraph requires. Harness: scenarios for the shifted-id match, the untracked-content guard, and the collector invocation; 46 mutations, all biting. README/skill claims updated to what the guard now checks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com>