fix(plugin): correct the skill-reachability path — every generated activation suite reports recall 0.0 - #143
Conversation
… measured descriptions Salvages the parts of the closed PR #109 that stand alone — no dependency on its `optimize/` subsystem or `Dataset.split_field`. ## The reachability bug (the reason this branch exists) Six surfaces told users to point `SKILL_SOURCE_PATH` at `.claude/skills`. A local plugin path must be a plugin ROOT — a directory holding `skills/` — so the skill resolves at `<path>/skills/<name>/SKILL.md`. One level too deep loads nothing at all. Every activation suite `check-skill` generates today therefore reports recall 0.0, which the template's own comment calls "reads exactly like a broken skill". `ci` was writing the broken path into users' scheduled CI workflows, where it produces a permanent red indistinguishable from the drift the schedule exists to detect. Verified against the real CLI rather than taken from the closed PR, two layouts over one probe skill at `<root>/.claude/skills/probe-alpha/SKILL.md`: claude --plugin-dir <root>/.claude/skills -> NONE (never loads) claude --plugin-dir <root>/.claude -> `.claude:probe-alpha` Corrected in all six: `check-skill`, `ci`, the `activation.yaml` template, `docs/PLUGIN.md`, and tutorial 07 (guidance + its troubleshooting row, which blamed only an unset variable). ## Bare-name collision hazard (check-skill) `skill_triggered` matches on the bare name and strips `plugin:` prefixes, and Claude Code ships its own unscoped `init`. A colliding `skill_name` does not error — it credits whichever skill fires. Ported PR #109's warning to say so. ## Descriptions Took PR #109's six-skill set. Two carry evidence: `analyze` is its promoted `a-regression` variant (train 1.000 vs 0.667 non-overlapping, test 1.000 vs 0.909) and adds the "what regressed" trigger it deterministically missed; `lint-tasks`' trim was measured at ceiling (F1 1.000 both splits). The `check-skill`/`init`/`task` trims are tighter prose, NOT individually A/B'd — recorded here so nobody later reads them as validated. Listing budget 1,576 -> 1,351 of 1,600. That headroom is the point: the budget is shared with every skill the user has installed. ## Deliberately not taken PR #109's `--split` guidance, its `run_limits`/`setting_sources` template notes, its threshold-currency warning, and its "plan expands the dataset" claim all describe code that PR added. Each was checked against `main` and is absent, so porting the prose would document behavior this tree does not have. Verified: `make lint` exit 0 (371 passed), `claude plugin validate --strict`, `coder-eval plan` on the edited template. `make test` shows 10 pre-existing failures in `tests/test_judge_litellm.py` that reproduce on clean `main` — the repo's own `litellm/` directory shadows the installed package when pytest runs from the root. This diff is Markdown and YAML only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V5WZ1BbtygK9YVvcgPfNGh
…st from ordering Three follow-ups to the skill-reachability fix in this branch's first commit. ## CE045 — a claude-code plugin path must be a plugin root (Authored as CE044; renumbered on rebase — #141's merge claimed that id for the manifest-parity rule, a different check on the same plugin tree.) Repo convention says a fixed bug should become a lint rule when its root cause is mechanically detectable. This one is: `SKILL_SOURCE_PATH` must never name a directory whose last segment is `skills`, because a plugin's skills live at `<path>/skills/<name>/SKILL.md` and one level deeper loads nothing at all. Six surfaces drifted to the wrong value together precisely because nothing held them in agreement. The rule globs the surfaces rather than enumerating them, so a seventh is caught by existing rather than by being remembered. It carries a mutation guard pinning both the rejected and the accepted forms, and it was verified to fail with file and line when the pre-fix value is reintroduced. ## The plugin-path harness divergence `docs/agents/HARNESS_PARITY.md` gains a section for it. claude-code REQUIRES a plugin root; `codex_agent._setup_skills` and `antigravity_agent._resolve_skills_paths` both scan the bare and the nested layout and take whichever holds a `<skill>/SKILL.md`. So `.claude/skills` works on two backends out of three and fails silently on the third — which is how the wrong value survived in six places. Per the parity rule a divergence is either fixed or documented, and this one was neither. That is also why CE045 keys on `SKILL_SOURCE_PATH` and leaves the Codex docs' `PLUGIN_PATH` alone. ## test_install_failure_is_not_latched A latent ordering hazard, found while investigating an unrelated schedule change. It does not fail on today's `-n auto` sharding, and it is not a flake: any change to how tests distribute across workers can surface it, and one did. `signal.signal` was patched module-wide for the test's whole duration, so the async teardown's own `signal.signal(SIGINT, default_int_handler)` restore hit the refusing mock and raised `ValueError` out of teardown — failing the test for something it does not test. The patch is now lifted as soon as the two calls under test are made. The count assertion is also replaced with a comparison of the two calls' signal SETS. A running total of 4 cannot distinguish a latch bug from an unrelated call landing in the same list; the set form tolerates a stray duplicate while still proving the retry, since a latched install would record nothing on the second call. ## Not in this PR The repo's `litellm/` directory is importable as a namespace package, so `import litellm` succeeds and returns an empty module — which is why `tests/test_judge_litellm.py` fails with `ModuleNotFoundError: No module named 'litellm.types'` for anyone without the optional `litellm` extra, instead of skipping. Ten failures on a clean checkout, invisible to CI because CI installs the extra. Deliberately left for a separate change; the work is parked on `fix/litellm-namespace-shadowing`. Verified: `make lint` exit 0 (CE044 and CE045 both green); `make test` shows the same pre-existing litellm failures as clean main and nothing else; ruff check and format clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V5WZ1BbtygK9YVvcgPfNGh
|
Claude finished @uipreliga's task in 1m 52s —— View job Code Review in Progress
|
uipreliga
left a comment
There was a problem hiding this comment.
Review: coder_eval — pr:143
Scope: pr:143 · branch feat/reuse-pr109-cheap-wins · fa68920 · 2026-08-27T22:03Z · workflow variant
Change class: simple — corrects a documented plugin path (.claude/skills → .claude) across six surfaces and adds a test-only CE045 lint rule that pins it; no src/ code path changes, and the rule's matcher carries its own mutation guard
Types, security, error handling, and API surface are effectively clean (four axes at 10.0) and the change itself is a genuine improvement, but the measurement layer is the real risk — the prescribed .claude plugin root injects sibling subagents into the evaluated sandbox, a misconfigured plugin path stays silent on claude-code alone, and the new CE045 rule's regex, globs, and tasks-walk each have a verified blind spot at the exact surface it was written to guard — so the bottom line is a healthy codebase whose eval-fidelity and lint-coverage gaps should be closed before more activation suites are generated from these templates.
Summary
| Axis | Score | 🔴 | 🟠 | 🟡 | 🔵 | Top Issue |
|---|---|---|---|---|---|---|
| 1. Code Quality & Style | 8.8 / 10 | 0 | 1 | 0 | 2 | CE045's _ASSIGNMENT regex truncates at whitespace, so the ci skill's GitHub Actions ${{ ... }} form is never checked (and matcher tests bake the gap in) |
| 2. Type Safety | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 3. Test Health | 8.9 / 10 | 0 | 0 | 2 | 1 | CE045's "globbed, not enumerated" claim overclaims: _GLOBS omits root-level *.md and .github/workflows, so canonical SKILL_DOC_SURFACES go unscanned |
| 4. Security | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 5. Architecture & Design | 9.5 / 10 | 0 | 0 | 1 | 0 | Plugin-path ripple stopped at HARNESS_PARITY: CODEX.md and the per-agent config references a task author reads still describe a bare skills dir, and CE045's exemption keys on the variable NAME ($PLUGIN_PATH also feeds a claude-code experiment) |
| 6. Error Handling & Resilience | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 7. API Surface & Maintainability | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 8. Evaluation Harness Quality | 7.9 / 10 | 0 | 1 | 2 | 1 | The prescribed plugin root (.claude) loads the entire plugin tree — commands/, agents/, hooks/, workflows/ — into the evaluated sandbox, and none of the six changed surfaces carries a caveat |
Overall Score: 9.4 / 10 · Weakest Axis: Evaluation Harness Quality at 7.9 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 5 · 🔵 4 across 8 axes.
Blockers
- [Axis 1] CE045's
_ASSIGNMENTregex truncates at whitespace, so theciskill's GitHub Actions${{ ... }}form is never checked (and matcher tests bake the gap in) (tests/test_custom_lint.py:3392) — The rule's value extractor is_ASSIGNMENT = re.compile(r"""SKILL_SOURCE_PATH\s*=\s*[\"']?([^\"'\s]+)[\"']?""")(line 3392).[^"'\s]+stops at the first whitespace, so the Actions formSKILL_SOURCE_PATH=${{ github.workspace }}/.claudeyields the captured value${{— never the path. Replaying the rule verbatim against the pre-fix tree (origin/main) proves it: it CATCHES docs/PLUGIN.md:112, docs/tutorials/07-plugin-in-claude-code.md:137, plugins/coder-eval/reference/templates/activation.yaml:17 and plugins/coder-eval/skills/check-skill/SKILL.md:172, and MISSESplugins/coder-eval/skills/ci/SKILL.md:156—SKILL_SOURCE_PATH=${{ github.workspace }}/.claude/skills— which the class docstring itself names as the highest-cost surface ('ciwrote the same path into users' SCHEDULED workflows'). Two tests then cement the hole rather than expose it: line 3461 assertsself._is_skills_dir("${{ github.workspace }}/.claude/skills"), a string the matcher can never hand to_is_skills_dir; and line 3473 asserts the Actions line's captured value merelystartswith("${{"), i.e. the truncation is written down as correct. Fix: strip${{ ... }}expressions before capture (or make the value pattern whitespace-tolerant up to end-of-line / quote), then re-point line 3461 through the real pipeline — assert that_ASSIGNMENT.findall('SKILL_SOURCE_PATH=${{ github.workspace }}/.claude/skills')produces a value_is_skills_dirrejects, so the mutation guard exercises regex + predicate together instead of the predicate alone. Also consider adding.github/workflows/**to_GLOBS, since that is where the emitted snippet lands. - [Axis 8] The prescribed plugin root (
.claude) loads the entire plugin tree — commands/, agents/, hooks/, workflows/ — into the evaluated sandbox, and none of the six changed surfaces carries a caveat (plugins/coder-eval/reference/templates/activation.yaml:13) — The template now instructs# broken skill. \path` must be a PLUGIN ROOT: a directory holding a `skills/`(line 13) and# export SKILL_SOURCE_PATH=/abs/path/to/.claude(line 19), feedingpath: "$SKILL_SOURCE_PATH"(line 26).--plugin-dirloads an entire plugin, not just itsskills/. Verified against the real CLI (2.1.248): with a root containingskills/probe-beta/,agents/probe-subagent.mdandcommands/probe-cmd.md,claude --plugin-dir -p "list subagents and /probe commands"returnedroot:probe-subagentunder the Task tool and/root:probe-cmd. A standard user repo's.claude/holdsagents/andcommands/next toskills/, so every generated activation suite now offers the evaluated agent every project subagent. Concrete false negative: a repo with.claude/agents/pdf-expert.mdmeasuring.claude/skills/pdf-forms— the agent delegates toroot:pdf-expertinstead of callingSkill(pdf-forms),skill_triggeredrecordsobserved='no', and recall drops for a reason that has nothing to do with the skill's description. That is the same 'reads exactly like a broken skill' failure the fix is chasing, made partial rather than total, and it makes the recall number repo-dependent and non-comparable across suites. None of the six updated surfaces mentions it. Fix: havecheck-skillstage a MINIMAL plugin root — a scratch dir containing onlyskills/(symlink or copy) — and pointSKILL_SOURCE_PATHat that, so the plugin root contains exactly the unit under test; at minimum, state in the template comment and incheck-skill/SKILL.md` that everything else under the chosen root (subagents, commands, hooks) also becomes visible to the evaluated agent.
Non-blocking, but please consider before merge
- *[Axis 3] CE045's "globbed, not enumerated" claim overclaims:
_GLOBSomits root-level .md and .github/workflows, so canonical SKILL_DOC_SURFACES go unscanned (tests/test_custom_lint.py:3396) —_GLOBS = ("plugins/coder-eval/**/*.md", "plugins/coder-eval/**/*.yaml", "docs/**/*.md")sits under the comment "Globbed, not enumerated: a seventh surface must be caught by existing, not by remembering to add it here" (lines 3394-3395), but the generated artifact lands outside all three patterns.plugins/coder-eval/skills/check-skill/SKILL.md:142-149("## Step 5 — Write the suite … Copy the two template files into the user's task tree") copiesreference/templates/activation.yaml— including its# export SKILL_SOURCE_PATH=/abs/path/to/.claudecomment at line 19 — into the repo's task tree. In this repo that istasks/, which the first test never reads (the second test readstasks/but only parsesagent.plugins[].path, never theSKILL_SOURCE_PATHcomment)..github/workflows/**and root-level*.mdare likewise unscanned. Fix: addtasks/**/*.yaml,tasks/**/*.yml,.github/workflows/*.y*mland*.mdto_GLOBS— the scan is a cheap line-regex, so widening it costs nothing and makes the "globbed, not enumerated" claim true. - [Axis 3] CE045 only lints repo-shipped strings; a user-supplied
SKILL_SOURCE_PATHpointing at a skills dir (or at a path that does not exist) stays silent on claude-code, while codex and antigravity both warn (tests/test_custom_lint.py:3357) — CE045 checks strings the repo itself ships. The failure it describes — "every activation suite the plugin generated reported recall 0.0" (docstring line 3369) — is triggered by a value the USER supplies viaSKILL_SOURCE_PATH, and nothing in the harness detects it.src/coder_eval/utils.py::process_plugins(lines 70-82) warns only for an undefined env var:log.warning(f"Plugin path contains undefined environment variable ${var_name}: {path}"). A path that resolves and exists but has noskills/subdirectory — precisely.claude/skills— passes through to the SDK with no signal at all. The other two backends already do the loud thing for the same condition:src/coder_eval/agents/codex_agent.py:1136logsf"0 skills linked into {agents_skills_dir} despite …", andsrc/coder_eval/agents/antigravity_agent.py:302logs"Plugin skills path did not resolve: …". Fix (additive, testable in one unit test): inprocess_plugins, when atype: localplugin path exists butPath(expanded)/"skills"is not a directory, emit a warning naming the plugin-root contract — then assert it with atmp_pathtest that builds<root>/skills/demo/SKILL.md(no warning) and<root>/demo/SKILL.md(warning). That covers every user, which the lint rule structurally cannot. - [Axis 5] Plugin-path ripple stopped at HARNESS_PARITY: CODEX.md and the per-agent config references a task author reads still describe a bare skills dir, and CE045's exemption keys on the variable NAME (
$PLUGIN_PATHalso feeds a claude-code experiment) (docs/agents/HARNESS_PARITY.md:139) — The new parity section closes with an exemption whose premise is false in this repo.docs/agents/HARNESS_PARITY.md:138-140:
`SKILL_SOURCE_PATH` — the variable `/coder-eval:check-skill` emits — is held to this by
lint rule CE045; `PLUGIN_PATH` in the Codex docs is deliberately outside that rule,
since a skills directory is valid there.
(the same claim is repeated in the lint rule's docstring, tests/test_custom_lint.py:3382-3384). PLUGIN_PATH is not Codex-only: experiments/plugin-comparison.yaml:22 sets type: claude-code in defaults.agent, and its with-plugin variant at line 37 sets path: "$PLUGIN_PATH". A reader who follows this page and exports PLUGIN_PATH=~/repo/.claude/skills loads no plugin at all on that experiment, and the with-plugin vs without-plugin comparison silently measures two identical arms — the same invisible failure the page was written to prevent, one env var over.
Two fixes, both cheap: (a) drop the blanket name-based exemption and scope it to the consuming harness (tasks/agents/codex_skills_test.yaml:15 is Codex; experiments/plugin-comparison.yaml:37 is not); (b) close the class at the shared runtime seam instead of in prose — utils.process_plugins (src/coder_eval/utils.py:42-86, reached from claude_code_agent.py:1184) already warns on an undefined env var but never checks the resolved directory, while BOTH tolerant harnesses already log this exact condition (codex_agent.py:1137-1142 "0 skills linked ... check the plugin path points at a skills repo root"; antigravity_agent.py:320-323 "0 skills discovered under %s"). claude-code — the one harness where the deep path is fatal — is the only one with no such warning, and a lint rule that runs only in this repo cannot help the user repos where /coder-eval:check-skill actually writes these suites.
4. [Axis 8] CE045's tasks/ walk checks nothing: the blanket $-in-path exemption skips the only eligible entry, so the whole YAML walk is unexercised and a literal .../.claude/skills tail behind a variable passes (tests/test_custom_lint.py:3446) — test_literal_plugin_paths_in_tasks_are_plugin_roots skips on if "$" in value: with the comment # an env var's value is covered by the assignment check above (lines 3446-3447), but the assignment check's globs are _GLOBS = ("plugins/coder-eval/**/*.md", "plugins/coder-eval/**/*.yaml", "docs/**/*.md") (line 3396) — neither tasks/** nor .github/workflows/** is scanned, so the claimed coverage does not exist. Two concrete recurrences pass CE045 today: (a) a task YAML writing path: "$REPO_ROOT/.claude/skills" — the literal /skills tail is visible and _is_skills_dir would flag it, but $ short-circuits the check; (b) a task YAML with path: "$SKILL_SOURCE_PATH" (the template's own shape, activation.yaml:26) whose assignment lives in a Makefile, a workflow, or a README — no surface CE045 reads. Fix: in the tasks walk, only skip when the value is a BARE variable reference (re.fullmatch(r'\\$\\{?\\w+\\}?/?', value)) and otherwise strip the $... prefix and apply _is_skills_dir to the literal tail; and add .github/workflows/**/*.yml to _GLOBS so a workflow-level assignment is covered.
5. [Axis 8] The HARNESS_PARITY reproduction command gives a false negative when run from a repo root — Claude Code discovers ./.claude/skills/ natively regardless of --plugin-dir, so both probes list the skill (docs/agents/HARNESS_PARITY.md:133) — The doc says Probe it in one command and gives claude --plugin-dir "$(pwd)/.claude" # loads .claude:<skill> / claude --plugin-dir "$(pwd)/.claude/skills" # loads nothing (lines 132-133). Run from the natural place — a repo root whose cwd contains .claude/skills/ — the second command still lists the skill, because Claude Code's own project-scoped skill discovery picks it up independently of --plugin-dir. Verified: in a dir containing .claude/skills/probe-alpha/SKILL.md, claude --plugin-dir "$(pwd)/.claude" listed both probe-alpha and .claude:probe-alpha, while claude --plugin-dir "$(pwd)/.claude/skills" still listed probe-alpha. Only with the skill moved outside a .claude cwd (root/skills/probe-beta, run from a sibling dir) does the contrast hold: root:probe-beta vs. nothing. So an engineer who doubts their config runs the documented probe, sees the skill under the deeper path, and concludes .claude/skills works — the opposite of what the page exists to teach. Fix: tell the reader the signal is the NAMESPACE prefix, not mere presence — e.g. # expect .claude:; a bare is project discovery, not the plugin — or make the probe run from a directory that is not the skill's parent repo.
Nits
- [Axis 1] Mid-test
monkeypatch.undo()is not exception-safe, so the ordering/masking hazard it fixes reappears on any failing path (tests/test_reference_permissions.py:391) — The fix is a baremonkeypatch.undo()at line 391, after bothregistry.ensure_crash_handlers()calls and before the assertions at 394-398. Its comment states the purpose: 'Lift the patch HERE, not at teardown ... the async-test teardown restores SIGINT by callingsignal.signal(SIGINT, default_int_handler)— which would hit_refuseand raiseValueErrorout of teardown, failing the test for a reason it does not test.' That holds only when everything before line 391 succeeds. Ifregistry.ensure_crash_handlers()(line 378 or 381) raises — the regression shape this test exists to catch — line 391 is skipped, teardown hits_refuse, and the real error is masked by a teardownValueError, which is precisely the symptom being fixed. Scope the two patches instead:with monkeypatch.context() as mp:around lines 360-382 (mp.setattr(...)foratexit.registerandsignal.signal), leaving the assertions outside the block — the restore then runs on every exit path with no explicitundo(). - [Axis 1] CE045 class docstring's incident record is inaccurate — tutorial 07 carried the wrong path once, not twice (
tests/test_custom_lint.py:3368) — Line 3367-3368 reads: 'Six surfaces once said.claude/skillsin unison:check-skill,ci, the bundledactivation.yaml,docs/PLUGIN.md, and tutorial 07 twice.'git grep -n "\.claude/skills" origin/main -- docs pluginsshows tutorial 07 contained the wrong value on exactly one line (docs/tutorials/07-plugin-in-claude-code.md:137); the file's second hunk in this PR edits the troubleshooting row, which on main said only 'SKILL_SOURCE_PATHis unset, so the skill was never offered' and never named.claude/skills. Commit 3b64a26 gets this right ('tutorial 07 (guidance + its troubleshooting row, which blamed only an unset variable)'); the docstring compresses it into a false claim. Since this docstring is the rule's rationale-of-record, restate it as the verified count — five wrong-value lines across five files (docs/PLUGIN.md:112, tutorial 07:137, activation.yaml:14 and :17, check-skill:168 and :172, ci:156), of which one form the matcher cannot see. - [Axis 3] The de-flake immunizes one test but leaves the process-wide
signal.signalpatch it diagnosed in three sibling tests, two of which read a signum-keyed dict (tests/test_reference_permissions.py:391) — The new comment at lines 371-377 correctly diagnoses the cause — "signal.signalis patched module-wide for the duration of this test, so anything else in the process that reaches fs_permissions while it runs lands inattemptstoo" — becausemonkeypatch.setattr("coder_eval.fs_permissions.signal.signal", _refuse)resolvescoder_eval.fs_permissions.signalto the stdlibsignalmodule object and mutates its.signalattribute globally. The fix (set comparison +monkeypatch.undo()at line 391) is local to this one test; the same global patch remains at line 338 (_fake_signal), line 415 and line 443 (lambda s, h: captured.__setitem__(s, h)). The last two are the exposed shape: they later invokecaptured[signal.SIGTERM](...)(line 424) andcaptured[signal.SIGINT](...)(line 452), so a stray in-process call landing in the same window overwrites the entry and the test exercises the wrong handler. Also,monkeypatch.undo()at line 391 is unguarded — ifregistry.ensure_crash_handlers()raised, undo is skipped and the teardownValueErrorthis change exists to prevent masks the real failure. Fix: givefs_permissionsa module-local indirection to patch (or usepytest.MonkeyPatch.context()/try: … finally: monkeypatch.undo()) and apply it to all four sites, so the hazard is closed for the class rather than for one method. - [Axis 8]
skill_triggeredname collisions are mitigated by advisory prose only, though the criterion already receives the namespace and throws it away (plugins/coder-eval/skills/check-skill/SKILL.md:190) — The new paragraph (lines 190-196) —**Because matching is by bare name, it cannot survive a name collision.** ... A collision does not error; it measures the wrong skill.— is accurate and a real improvement over silence, but it is a model-discretion instruction, so nothing in the harness enforces it and a colliding suite still emits a confident recall/precision number. The information needed to disambiguate is already present and discarded:src/coder_eval/criteria/skill_triggered.py:68doesnames.add(skill.split(":")[-1])on theSkilltool'sparameters['skill'], which carries the fullroot:probe-betaform (confirmed against the real CLI). Since this PR makes a namespaced plugin skill the standard shape for every generated suite, the collision is now reachable in the default configuration rather than theoretical. Follow-up (out of this PR's scope, but the right harness answer): letSkillTriggeredCriterion.skill_nameaccept a namespaced value and match exactly when one is given, keeping bare-name matching as the fallback — then a colliding suite can be pinned instead of merely warned about.
What's Missing
Parallel paths:
- 🟠 docs/AB_EXPERIMENTS.md:182-183 — the canonical "Recipe: A/B a Skill" still writes
path: "../skills"underdefaults.agent.type: claude-code(line 167). That is the exact broken depth this PR removed everywhere else: thewith-skillarm loads nothing, so the A/B silently compares two identical bare arms. CE045 cannot see it (it scans onlySKILL_SOURCE_PATH=assignments plustasks/**/*.yamlvalues, never YAML inside docs code fences). (trigger: docs/agents/HARNESS_PARITY.md) (restates: Axis 5: Plugin-path ripple stopped at HARNESS_PARITY) - 🟡 docs/agents/CLAUDE_CODE.md — the per-harness reference a claude-code task author actually reads — was not updated: line 101 still defines
pluginsas "Local plugin/skill directories;$VARinpathis expanded…" and the "Skills & plugins" section (lines 179-190) repeats it, with no plugin-root requirement and no link to the new HARNESS_PARITY section. The one page stating the contract is the parity page nobody reaches from the field table. (trigger: docs/agents/HARNESS_PARITY.md) (restates: Axis 5: Plugin-path ripple stopped at HARNESS_PARITY) - 🟡 experiments/default.yaml:62 — the baseline layer every experiment inherits still documents
# Example: [{"type": "local", "path": "/path/to/skills"}], i.e. the skills-dir form, for a file whose default agent is claude-code.experiments/is outside CE045's_GLOBSand outside itstasks/**YAML walk, so this example can be copied indefinitely. (trigger: plugins/coder-eval/reference/templates/activation.yaml) (restates: Axis 5: Plugin-path ripple stopped at HARNESS_PARITY) - 🟡 CLAUDE.md:147 is the repo's index of harness divergences ("Known unfixed divergences:
permission_modeon Codex and Antigravity,disallowed_toolson Codex,allowed_tools/disallowed_toolson Antigravity,turn_timeouton Antigravity"). The PR adds a sixth, higher-cost divergence to HARNESS_PARITY.md but does not add it to that list, so the index the next contributor greps is now incomplete for exactly the field that shipped broken across six surfaces. (trigger: docs/agents/HARNESS_PARITY.md) - 🔵 The three code sites that implement the divergence carry no back-reference to the new contract section, unlike the
max_turnsprecedent (codex_agent.py:820andantigravity_agent.py:539both cite docs/agents/HARNESS_PARITY.md). Add the same one-line pointer atclaude_code_agent.py'sprocess_pluginscall site (line ~1184),codex_agent._setup_skills, andantigravity_agent._resolve_skills_paths, so an edit to either tolerant scanner knows a documented contract exists. (trigger: docs/agents/HARNESS_PARITY.md) - 🔵 CE045 is implemented inline in tests/test_custom_lint.py against a hardcoded
REPO_ROOT, while every sibling doc-surface rule factors its logic into a root-parameterized module under tests/lint/ (CE044 → tests/lint/plugin_manifest_parity.py, plus doc_env_parity.py, doc_schema_parity.py, plugin_reference.py, workflow_outputs.py). The divergence is what makes CE045's walk untestable against a fixture tree (see the Tests bucket). (trigger: tests/test_custom_lint.py) - 🟡 The runtime seam was left asymmetric:
utils.process_plugins(src/coder_eval/utils.py:69-86) warns only on an undefined env var, while codex (codex_agent.py:1137-1142) and antigravity (antigravity_agent.py:302,320-323) both log a 0-skills/unresolved-path warning — so the one harness where the wrong depth is fatal is the only one that is silent, and no user repo (wherecheck-skillactually writes these suites) is reachable by a repo-scoped lint rule. (trigger: docs/agents/HARNESS_PARITY.md) (restates: Axis 3: CE045 only lints repo-shipped strings; no runtime guard on claude-code)
Tests:
- 🟡 CE045's surface walk has no fixture test and no non-vacuity guard:
_surfaces()globs the live tree andtest_no_surface_points_skill_source_path_at_a_skills_dirpasses trivially if the globs ever match nothing (docs move, plugin dir renamed). The mutation guard at line 3455 exercises_is_skills_diralone, never the regex+walk pipeline. Add a tmp_path tree containing one offending file and assert the walk reports it, plusassert self._surfaces()covers the known six surfaces. (trigger: tests/test_custom_lint.py) - 🟡 Nothing executable proves the plugin-root contract itself. tests/test_plugin_processing.py (11 tests) covers env-var expansion only — no case for a resolvable path whose layout has no
skills/— and the pr-checksplugin-validatejob (.github/workflows/pr-checks.yml:253-278) copies the template but asserts only row count and labels, never theagent.plugins[0].pathsemantics. A fixture root laid out<root>/skills/<name>/SKILL.mdplus a negative<root>/<name>/SKILL.mdcase would turn the entire fix from prose+regex into a real gate. (trigger: plugins/coder-eval/reference/templates/activation.yaml) - 🟡 CE045's second test (
test_literal_plugin_paths_in_tasks_are_plugin_roots) has zero eligible inputs intasks/today and skips every$-bearing value, so the YAML walk is unexercised and cannot regress-fail; it needs at least a tmp_path fixture proving it flagspath: ".claude/skills"andpath: "$REPO_ROOT/.claude/skills". (trigger: tests/test_custom_lint.py) (restates: Axis 8: CE045's tasks/ walk checks nothing (blanket$exemption)) - 🟠 The GitHub Actions surface — the one the class docstring calls the highest-cost (
ciwrites it into users' scheduled workflows) — has no passing-path test:test_the_matcher_sees_every_framing_the_surfaces_use(line 3473) asserts the truncated${{capture as correct, so the Actions form is documented as covered while being structurally unreachable by_is_skills_dir. (trigger: plugins/coder-eval/skills/ci/SKILL.md) (restates: Axis 1: CE045's_ASSIGNMENTregex truncates at whitespace) - 🔵 The de-flake in tests/test_reference_permissions.py:391 lifts the patch imperatively rather than scoping it, and three sibling tests (lines 338, 415, 443) keep the same process-wide
signal.signalpatch — so neither the exception path nor the class-wide hazard is covered by the change. (trigger: tests/test_reference_permissions.py) (restates: Axis 1: Mid-testmonkeypatch.undo()is not exception-safe)
Downstream consumers:
- 🟡 docs/TASK_DEFINITION_GUIDE.md §
skill_triggered(lines 1275-1298) is where a suite author learns how the criterion matches, and it still says nothing about how the skill becomes reachable — no plugin-root note, no link to the new parity section. An author who writes askill_triggeredsuite from that page alone reproduces the recall-0.0 bug the PR is fixing. (trigger: docs/agents/HARNESS_PARITY.md) - 🔵 Six skill
description:frontmatters were rewritten, but the paraphrase tables that advertise the same skills were not: docs/PLUGIN.md:53 still promises lint-tasks reports "fixtures with no cleanup and near-duplicates — each with a severity and a fix" and plugins/coder-eval/README.md:41-45 carries its own variants, while the trimmed descriptions dropped those claims. The derived surface tests check only name presence and the count word — nothing enforces description/blurb parity, so this drifts unobserved. (trigger: plugins/coder-eval/skills/lint-tasks/SKILL.md) - 🔵 The descriptions are described as "measured" (PR #109) but the measurement leaves no in-repo artifact: no activation suite, run record, or comment records the before/after triggering numbers, and
SKILL_LISTING_BUDGET_CHARS(tests/test_custom_lint.py:1250) was not re-baselined after the total shrank. A future edit has no evidence to compare against. (trigger: plugins/coder-eval/skills/analyze/SKILL.md)
Daily/nightly:
- 🟡 Blast radius on already-emitted user artifacts is unstated. Every suite
check-skillscaffolded and every scheduled workflowciemitted before this fix still carries.claude/skills, and the PR adds no migration note anywhere (docs/PLUGIN.md, tutorial 07, the skills themselves): those users' weekly jobs stay permanently red, and anysuite_thresholdsthey lowered against a recall-0.0 baseline is now mis-calibrated in the other direction once they fix the path. (trigger: plugins/coder-eval/skills/ci/SKILL.md) - 🟡 No recurring in-repo guard exists for the plugin's own activation behavior: there is no
skill_triggeredtask or scheduled workflow covering the six shipped skills (onlyplugin-validate, which is offline and schema-only), so a change to sixdescription:fields — the field activation keys on — ships with no signal at all until users report mis-triggering. (trigger: plugins/coder-eval/skills/check-skill/SKILL.md)
Harness & Lint Improvements
Static checks (lint / type):
- [ce-lint] CE046 — a text-scanning lint rule must carry a machine-checked known-bad corpus that exercises the FULL extract→predicate pipeline. New doc-surface-style
@pytest.mark.lintclass intests/test_custom_lint.py(not aBaseRule; the runner'scheck_pathsonly walkssrc/, see tests/test_custom_lint.py:22,31). It walks the AST oftests/test_custom_lint.pyitself and, for every lint class that defines a class-levelre.compile(...)value extractor, requires (a) aKNOWN_BAD_LINES: tuple[str, ...]attribute holding the verbatim pre-fix source lines — one per historical offending surface — and (b) a test that feeds each line throughextractor.findall(line)→ predicate and asserts every one is flagged. Today CE045's mutation guard (test_the_rule_would_catch_the_bug_it_was_written_for, line 3458) calls_is_skills_diron hand-written strings the matcher can never produce, so the predicate is proven and the matcher is not. Prevents: Finding 1 (_ASSIGNMENTat tests/test_custom_lint.py:3392 truncates at whitespace): the entrySKILL_SOURCE_PATH=${{ github.workspace }}/.claude/skillswould fail immediately —_ASSIGNMENT.findallyields${{and_is_skills_dirreturns False. It also kills the companion anti-test at line 3473 that writes the truncation down as expected. Finding 3 (docstring's "tutorial 07 twice" incident record) becomes moot: the corpus IS the incident record, so the count is derived rather than asserted in prose. - [ce-lint] CE047 — a text-scanning lint rule's file globs must cover every tracked file containing the token it guards. For each such rule, take its guarded token (
SKILL_SOURCE_PATH), rungit grep -l <token>over the tracked tree, and assert every hit matches at least one entry in the rule's_GLOBS(excluding the rule's own file). This makes CE045's comment at lines 3394-3395 — "Globbed, not enumerated: a seventh surface must be caught by existing, not by remembering to add it here" — true by construction instead of aspirational: a new surface either falls inside the globs or fails the build. Prevents: Finding 4:_GLOBS = ("plugins/coder-eval/**/*.md", "plugins/coder-eval/**/*.yaml", "docs/**/*.md")(line 3396) misses root-level*.md(README.md, CONTRIBUTING.md),.github/workflows/**,tasks/**andexperiments/**..github/workflows/pr-checks.ymlalready dogfoods the activation template (lines 261-262), so an in-repo workflow surface is a realistic next drift. - [ce-lint] CE048 — a file-walking lint scan must assert non-vacuity. AST rule over
tests/test_custom_lint.py: a test method that walks files (for ... in ...glob(...)) and ends inassert not offendersmust also assert a non-zero examined counter (assert scanned, ...). Mirrors CE036's existing registry-derived coverage requirement — a rule that examines nothing passes for the wrong reason and reads as green forever. Prevents: Finding 8:test_literal_plugin_paths_in_tasks_are_plugin_rootscurrently examines ZERO entries (the only local-plugin task,tasks/agents/codex_skills_test.yaml, is Codex and filtered out), so itsif "$" in value: continueexemption (lines 3446-3447) removed the one shape the walk could ever see and nothing noticed. A non-vacuity assert forces either a fixture or a widened scan at authoring time. - [ce-lint] CE049 — no process-global stdlib patching or mid-test
monkeypatch.undo()in tests. NewBaseRuleAST rule, wired via a dedicatedcheck_paths([TESTS], rules=[NoProcessGlobalPatch])test (the parametrizedtest_no_violationsonly walksSRC). Two shapes: (a)monkeypatch.setattr("<pkg>.<mod>.<stdlib>.<attr>", ...)where the second-to-last dotted segment is a stdlib module (os,signal,time,socket,subprocess,shutil) — pytest resolvescoder_eval.fs_permissions.osto the shared stdlib module object and mutates it process-wide, not module-locally; require a module-local indirection ormonkeypatch.context(). (b) anymonkeypatch.undo()outside afinally:block — usewith monkeypatch.context() as mp:so the restore runs on every exit path. Prevents: Findings 2 and 6. Would have flagged 11 sites intests/test_reference_permissions.py(:176, :338, :367, :414-416, :442-444, :470, :1044) — including the twocaptured[signum]handler dicts at :415/:443 that the de-flake diagnosed but did not fix — plus both bareundo()calls at :183 and :391, where a raise fromregistry.ensure_crash_handlers()(:378/:381) skips the undo and masks the real failure behind a teardownValueError. - [ce-lint] CE050 — every agent must resolve
agent.plugins[].paththrough the single sharedutils.process_pluginsseam. AST rule oversrc/coder_eval/agents/**: readingplugin["path"]/plugin.get("path")and then callingexpand_env_vars(...)orPath(...).resolve()on it, outsideutils.process_plugins, is a violation (scoped to path resolution, not to each backend's skill-linking, which legitimately differs). One seam means one place to add the path diagnostic, so a warning added there covers all three backends at once. Prevents: Finding 5 (and the runtime half of finding 7): the divergence exists precisely because each backend resolves its own path —codex_agent.py:1137-1142andantigravity_agent.py:302,320-323both warn on a 0-skills root, while claude-code (claude_code_agent.py:1184→utils.py:69-86) warns only on an undefined env var. claude-code is the one harness where the wrong path is fatal and the only one with no signal. - [ce-lint] CE045 widening — key the exemption on the CONSUMING harness, not the variable name, and stop short-circuiting on
$. (i) Derive the guarded variable set instead of hardcodingSKILL_SOURCE_PATH: scantasks/**andexperiments/**foragent.type == claude-code(including inheriteddefaults.agent) and collect the$VARnames appearing inplugins[].path; require every assignment of those names, anywhere in the repo, to be a plugin root. (ii) In the tasks walk, replaceif "$" in value: continuewith a bare-reference-only skip (re.fullmatch(r"\$\{?\w+\}?/?", value)) and apply_is_skills_dirto the literal tail otherwise. (iii) Strip${{ ... }}before capture so the GitHub Actions form is seen. Prevents: Finding 7:experiments/plugin-comparison.yamlsetstype: claude-code(line 22) andpath: "$PLUGIN_PATH"(line 37), whiledocs/agents/CODEX.md:266documentsexport PLUGIN_PATH=~/path/to/skills— the exact banned shape, exempted only because the rule keys on a name. Also finding 8 case (a):path: "$REPO_ROOT/.claude/skills"in a task passes CE045 today (reproduced: 5 passed), even though the/skillstail is plainly visible and the sibling assignment walk would flag it. - [ce-lint] CE051 — a docs probe command that asserts an observable outcome must be pinned to a runner that executes it. In
docs/agents/**(anddocs/tutorials/**), a fenced shell block whose lines carry an expectation comment (# loads ...,# expect ...,# -> ...) must be preceded by a<!-- verified-by: <pytest nodeid | CI job> -->marker that resolves to a real test id or workflow job — same resolution style as CE035'sneeds.<job>.outputswalk and the same pinning pattern CE026 already uses for Action snippets (action-dogfood). The check verifies the pin exists; the claim's truth comes from the pinned runner (see harness item H2). Prevents: Finding 9:docs/agents/HARNESS_PARITY.md:133statesclaude --plugin-dir "$(pwd)/.claude/skills" # loads nothing, which is FALSE when run from a repo root — project-scoped discovery lists the skill anyway (reproduced against claude 2.1.248). An unpinned expectation comment is an untested assertion in the one document written to stop people from mis-diagnosing this.
Harness improvements (not statically reachable):
- Add the plugin-path diagnostic at the shared runtime seam (
utils.process_plugins), with acaplog+tmp_pathunit test. Warn when the expanded path (a) does not exist, or (b) exists, has noskills/subdirectory, AND directly contains a<child>/SKILL.md— the precise skills-dir shape the bug produces. The narrow (b) condition matters: a legitimate plugin root may hold onlycommands/,agents/, or an MCP server (tests/test_plugin_processing.pyalready exercises a.../mcproot), so an unconditional "no skills/" warning would fire on valid configs. Test both arms:<root>/skills/demo/SKILL.md→ silent,<root>/demo/SKILL.md→ warns. Why not static: The offending value is supplied by the USER at run time viaSKILL_SOURCE_PATH; no repo-scoped scan can ever see it. CE045 runs only in this repo, while/coder-eval:check-skillwrites activation suites into downstream user repos where no lint of ours executes. Detecting the wrong layout requires stat-ing the resolved directory. Prevents: Findings 5 and 7 — and the entire failure class CE045 was written for, at the one place that reaches every user. It also closes the parity gap the PR's own HARNESS_PARITY row documents as "fails without an error". - Turn the plugin-path probe into an executable artifact the docs cite, not a hand-typed command. Add a
@pytest.mark.livetest (ormake probe-plugin-dir) that builds a fixture skill in a tmp dir OUTSIDE any.claudecwd, runsclaude --plugin-dir <root>and--plugin-dir <root>/skills, and asserts on the NAMESPACE prefix (<root-name>:<skill>present vs. absent) rather than on mere presence. Rewrite HARNESS_PARITY.md:129-133 to state the namespace signal and point at the pinned runner. Why not static: Needs the realclaudeCLI and a live agent invocation. The false negative is cwd-dependent — project-scoped skill discovery masks the failure only when the probe runs from the skill's own repo root, which is exactly where a reader runs it — so it is invisible to any text scan. Prevents: Finding 9. It also stops the CE045 docstring's "Probed against the real CLI" claim from rotting silently as the CLI evolves. - Have
check-skillstage a MINIMAL scratch plugin root, and record the loaded plugin inventory intask.json. (i) The skill should create<tmp>/skills/<skill-name>(symlink or copy) and pointSKILL_SOURCE_PATHthere, so the plugin root contains exactly the unit under test. (ii) The claude-code agent should record, per--plugin-dirroot, which components it contributed (skills/,agents/,commands/,hooks/,.mcp.json), so a confound is visible after the fact and two suites' recall numbers are comparable. At minimum, state the wider load in the template comment and incheck-skill/SKILL.md. Why not static: What a plugin root contains is a property of the user's filesystem at run time — nothing in this repo can be scanned to learn that a given.claude/also holdsagents/. Verified against claude 2.1.248: a root withskills/,agents/andcommands/registers all three (probe-root:probe-beta,probe-root:probe-subagent,/probe-root:probe-cmd); auto-delegable subagents are the load-bearing confound, slash commands are inert in a non-interactive eval. Note CE045 does not block the fix — a scratch root whose last segment is notskillspasses. Prevents: Finding 11 — recall becoming repo-dependent and non-comparable across generated activation suites, for reasons unrelated to the skill's description. - Let
SkillTriggeredCriterion.skill_nameaccept a namespaced value and match exactly, with bare-name matching as the fallback; add a collision note.criteria/skill_triggered.py:68doesnames.add(skill.split(":")[-1])on aSkillparameter that already carries the full<plugin-root>:<name>form — the disambiguating information is received and discarded. Also emit a criterion-level note when the trajectory shows one bare name under more than one namespace. Why not static: The collision is only observable in a recorded trajectory'sSkilltool calls — it is a property of the user's installed skill set at run time, not of any repo text. The current mitigation is a prose paragraph incheck-skill/SKILL.md(~line 190), i.e. model discretion with nothing enforcing it; a colliding suite still emits a confident recall/precision number. Prevents: Finding 10. This PR makes a namespaced plugin skill the DEFAULT shape for every generated suite, so the collision moved from theoretical to reachable in the standard configuration. - Add a session-level shared-state leak detector for the test suite. An autouse fixture that snapshots
signal.getsignal(SIGINT/SIGTERM)and the identity of hot stdlib attributes (os.chmod,signal.signal,os.kill) before each test and fails the test that leaves any of them changed — attributing the leak to its source instead of to the next victim. Why not static: CE049 catches the patch SHAPE at the call site; only a runtime guard catches a leak that arrives through a helper or fixture, or an escape via a background thread — andfs_permissionsowns exactly such a watchdog thread, which is what made the original failure-n auto-ordering-dependent rather than reproducible. Prevents: The residual half of findings 2 and 6 — the three sibling tests still holding a process-widesignal.signalpatch, two of which read a signum-keyed dict (captured[SIGTERM]at :424,captured[SIGINT]at :452) that a stray in-process call can overwrite. - Add a
would-have-caughtreplay target for new lint rules:make lint-replay RULE=<id> REV=<pre-fix-sha>. Check the pre-fix tree into a throwaway git worktree, run only the named rule against it, and require at least one violation — then paste the reported offender lines into the rule'sKNOWN_BAD_LINEScorpus (static check CE046). This is exactly the manual step that exposed the${{ ... }}miss during this review; automating it makes the corpus a byproduct of authoring the rule rather than a discipline. Why not static: After the fix lands, the repo tree is clean by construction, so a new rule passes trivially against the only tree a static check can see. Proving a rule catches its motivating bug requires materializing a second, historical worktree — git state, not source text. Prevents: Findings 1, 4 and 8 share one root shape: the rule passes because it sees nothing (a truncated capture, an uncovered glob, an exemption that skips the only candidate). A replay makes the sighting mandatory before the rule is accepted.
Top 5 Priority Actions
- Stage a minimal plugin root in
/coder-eval:check-skill— a scratch dir holding onlyskills/<skill>— instead of prescribing the user's whole.claude(plugins/coder-eval/reference/templates/activation.yaml:13,19,26), because--plugin-diralso loads siblingagents/, whose auto-delegable subagents can make the agent delegate instead of calling the skill, flippingskill_triggeredtoobserved='no'and making recall repo-dependent; if the wider load is intentional, say so in the template and in check-skill/SKILL.md. - Add a runtime guard in
src/coder_eval/utils.py::process_plugins(lines 69-86; only warning is the undefined-env-var one at :77, reached from agents/claude_code_agent.py:1184) for a plugin path that does not exist, or that has noskills/subdir while directly containing<child>/SKILL.md— today claude-code, the one harness where this is fatal, is silent and reports recall 0.0, while codex_agent.py:1137 and antigravity_agent.py:302 both warn. - Let
SkillTriggeredCriterion.skill_nameaccept and exactly match a namespaced value, keeping bare-name as fallback, since src/coder_eval/criteria/skill_triggered.py:68 already receivesroot:<skill>and discards the namespace via.split(":")[-1]— a name collision currently emits a confident but wrong recall number backed only by advisory prose in plugins/coder-eval/skills/check-skill/SKILL.md:190. - Close CE045's three coverage holes in tests/test_custom_lint.py — strip
${{ ... }}before capture so the whitespace-truncating_ASSIGNMENTregex (:3392) stops missing theciskill's GitHub Actions form (plugins/coder-eval/skills/ci/SKILL.md:156, the surface its own docstring calls highest-cost), widen_GLOBS(:3396) to root*.md,.github/workflows/*.y*mlandtasks/**so "globbed, not enumerated" is true, and replace the blanketif "$" in value: continue(:3446) with a bare-variable-only skip so an interpolated.../.claude/skillstail is still caught (reproduced: the tasks walk currently checks nothing). - Fix the two misleading doc surfaces added in docs/agents/HARNESS_PARITY.md — the probe at :132-133 gives a false negative from a repo root because project-scoped discovery lists the skill anyway (key the reader on the
.claude:<skill>namespace prefix, not mere presence), and the exemption at :139 claims a harness scope while CE045 is name-scoped, leaving the claude-code$PLUGIN_PATHconsumer at experiments/plugin-comparison.yaml:37 and docs/agents/CODEX.md:266 unguarded.
Stats: 0 🔴 · 2 🟠 · 5 🟡 · 4 🔵 across 8 axes reviewed.
…allel surfaces, runtime guard
Every claim below was reproduced before acting on it. Three of them proved my own
text wrong.
## CE045's matcher could not see the surface it was written for
`[^"'\s]+` stops at the first space, so the GitHub Actions form
`SKILL_SOURCE_PATH=${{ github.workspace }}/.claude/skills` captured `${{` — never
the path. The `ci` skill, which the rule's own docstring calls the highest-cost
surface because it writes into users' SCHEDULED workflows, was structurally
unreachable. Worse, a companion test asserted that truncated capture as CORRECT,
so the hole was written down as covered.
The value now runs to end-of-line or a closing quote, and `${{ ... }}` expressions
are stripped before the last-segment check. Verified by reintroducing the pre-fix
`ci` line: the rule flags it with file and line.
The mutation guard is replaced by `KNOWN_BAD_LINES`, the four verbatim pre-fix
source lines, replayed through the FULL extract-then-predicate pipeline. The old
guard called the predicate on hand-written strings the matcher could never
produce, which is exactly how the matcher stayed unproven. That corpus is now the
incident record, so the docstring no longer miscounts it ("tutorial 07 twice" was
wrong — one line, verified against origin/main).
Two bugs my own new tests then caught: `findall` reports an unmatched alternation
group as `""` rather than `None`, so picking "the first non-None group" always
selected the empty quoted branch; and the YAML walk labelled fixture paths with
`relative_to(REPO_ROOT)`.
## The globs and the tasks walk both under-covered
`_GLOBS` missed root-level `*.md`, `.github/workflows/**`, `tasks/**` and
`experiments/**`. `test_globs_cover_every_file_naming_the_token` now derives the
expected set from `git grep -l`, so "globbed, not enumerated" is true by
construction rather than aspirational. A non-vacuity assert fails if the walk ever
matches nothing at all.
The YAML walk skipped every `$`-bearing value as "covered by the assignment
check", which those globs did not in fact cover — so it had zero eligible inputs
and could not regress-fail. It now skips only a BARE variable reference and judges
the literal tail otherwise, so `$REPO_ROOT/.claude/skills` is caught. It also
reads `defaults.agent` and `variants[].agent`, and has a fixture test.
## Three parallel surfaces carried the same bug
- `docs/AB_EXPERIMENTS.md` — the canonical "A/B a skill" recipe wrote
`path: "../skills"` under `type: claude-code`. Its `with-skill` arm loaded
nothing, so the experiment silently compared two identical arms.
- `experiments/default.yaml` — the baseline every experiment inherits documented
`"path": "/path/to/skills"`.
- `experiments/plugin-comparison.yaml` — `$PLUGIN_PATH` under a claude-code
default, with no statement of the contract.
That last one also falsifies a claim I shipped: HARNESS_PARITY and the rule's
docstring both said `PLUGIN_PATH` was "deliberately outside the rule, since a
skills directory is valid there". It is not Codex-only. Both now say the exemption
is a limit of the rule's scope, not a licence to use the deeper form.
`docs/agents/CLAUDE_CODE.md` — the reference a claude-code task author actually
reads — states the contract and links the parity section; `CLAUDE.md`'s index of
known divergences gains this one as its highest-cost entry.
## The parity probe taught the opposite of its lesson
Run from a repo root, `claude --plugin-dir "$(pwd)/.claude/skills"` STILL lists
the skill — Claude Code discovers `./.claude/skills/` natively, independent of
`--plugin-dir`. An engineer following the documented probe would have concluded
the deeper path works. The probe now runs from outside the skill's repo and tells
the reader the signal is the NAMESPACE prefix, not mere presence.
## A plugin root loads the whole plugin, not just its skills
Verified against the real CLI: a root holding `skills/probe-beta/`,
`agents/probe-subagent.md` and `commands/probe-cmd.md` offers all three as
`root:probe-beta`, `root:probe-subagent`, `root:probe-cmd`.
So pointing a suite at a repo's `.claude` hands the evaluated agent every project
subagent and command. With `.claude/agents/pdf-expert.md` present while measuring
`pdf-forms`, the agent may delegate instead of calling the skill; `skill_triggered`
records `no` and recall drops for a reason unrelated to the skill's description —
and the number becomes repo-dependent, so two suites stop being comparable.
`check-skill` and the bundled template now prescribe staging a MINIMAL root
containing only the skill under test, and `ci` carries the workflow step to build
one (a scheduled job inherits no shell state). The whole-tree form is still
offered, with the confound stated.
## A runtime guard, because a repo-scoped lint rule cannot reach users
`utils.process_plugins` warned only on an UNDEFINED env var. A path that resolves
and exists but holds no `skills/` — precisely `.claude/skills` — reached the SDK
with no signal at all. Codex and Antigravity already log the equivalent condition;
claude-code, the one harness where the depth is fatal, was silent. It now warns,
with four tests. That covers the user repos where `/coder-eval:check-skill`
actually writes these suites, which CE045 structurally cannot.
## Exception-safe patch scoping
`test_install_failure_is_not_latched` lifted its `signal.signal` patch with a bare
`monkeypatch.undo()` after the calls. If `ensure_crash_handlers` raised — the
regression the test exists to catch — the undo was skipped and the teardown
`ValueError` masked the real failure. Now `monkeypatch.context()`, which restores
on every exit path.
## Not addressed here
The reviewer's namespaced-`skill_name` matching, the sibling tests' process-wide
`signal.signal` patches, and the proposed CE046/CE047 meta-rules are real but are
their own changes. The migration note for already-emitted user artifacts needs a
release decision first.
Verified: `make lint` exit 0 (382 passed); `make test` shows the same 10
pre-existing litellm failures as clean main and nothing else; ruff clean;
`claude plugin validate --strict`; `coder-eval plan` on the edited template.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V5WZ1BbtygK9YVvcgPfNGh
pyright (`reportImplicitStringConcatenation`) rejected the multi-line warning string added in 754805b. It failed two gates — Quality Gate and Windows Smoke, both on the `Type check with pyright` step — and I had not caught it because pyright could not install in this session, so the one gate my change could break was the one gate I never ran. Now matches the explicit `+` style the neighbouring warning in `codex_agent._setup_skills` already uses. Verified with CI's exact extras (`--extra dev --extra uipath --extra codex --extra litellm`): pyright reports 0 errors, 1 pre-existing warning in `antigravity_agent.py:442` that is not from this branch. `make lint` exit 0, `make test` 4369 passed, ruff clean. The third red gate, Evalboard, is unrelated: it failed on `Install dependencies (lockfile-pinned)`, this branch touches no JS or lockfile, and the job passes on main's last three runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V5WZ1BbtygK9YVvcgPfNGh

The bug
Six surfaces told users to point
SKILL_SOURCE_PATHat.claude/skills. A local plugin path must be a plugin root — a directory holdingskills/— so the skill resolves at<path>/skills/<name>/SKILL.md. One level too deep loads nothing at all.Probed against the real CLI, one skill at
<root>/.claude/skills/probe-alpha/SKILL.md:So every activation suite
/coder-eval:check-skillgenerates today reports recall 0.0 — what the bundled template's own comment calls "reads exactly like a broken skill." And/coder-eval:ciwrote the same path into users' scheduled workflows, where it renders as a permanent red indistinguishable from the drift the schedule exists to detect.Corrected across five files:
check-skill,ci,activation.yaml,docs/PLUGIN.md, and tutorial 07 (its guidance line, plus its troubleshooting row, which had blamed only an unset variable). Review turned up three more surfaces with the same bug — see below.Why it survived in six places at once — a harness divergence
docs/agents/HARNESS_PARITY.mdgains a section. claude-code requires a plugin root;codex_agent._setup_skillsandantigravity_agent._resolve_skills_pathsboth scan the bare and the nested layout and take whichever holds a<skill>/SKILL.md.So
.claude/skillsworks on two backends out of three and fails silently on the third. Per the parity rule a divergence is either fixed or documented — this one was neither.CE045 keeps it fixed
Repo convention: a fixed bug becomes a lint rule when the root cause is mechanically detectable.
SKILL_SOURCE_PATHmust never name a directory whose last segment isskills.It globs the surfaces rather than listing them, so a seventh is caught by existing rather than by being remembered. Mutation-guarded on both the rejected and accepted forms, and verified to fail with file and line when the pre-fix value is reintroduced.
Authored as CE044; renumbered on rebase after #141's merge claimed that id for the manifest-parity rule. The two are complementary — CE044 checks the manifests agree with each other, CE045 checks the path names a plugin root. Both green.
CE045 keys on
SKILL_SOURCE_PATHonly. That is a limit of the rule's reach, not a licence to use the deeper form elsewhere:$PLUGIN_PATHfeedsexperiments/plugin-comparison.yaml, whose default agent is claude-code, and is unlinted. The guard that reaches every user is the new runtime warning.Salvaged from the closed PR #109
Only what stands alone — nothing depending on its
optimize/subsystem orDataset.split_field.Descriptions. Two carry evidence:
analyzeis #109's A/B-promoteda-regressionvariant (train 1.000 vs 0.667 non-overlapping, test 1.000 vs 0.909) and adds the "what regressed" trigger it deterministically missed;lint-tasks' trim was measured at ceiling. Thecheck-skill/init/tasktrims are tighter prose, not individually A/B'd — stated so nobody later reads them as validated.Listing budget 1,576 → 1,351 of 1,600. That headroom is the point: the budget is shared with every skill the user has installed.
A silent-wrong-measurement warning in
check-skill.skill_triggeredmatches on the bare name and stripsplugin:prefixes, and Claude Code ships its own unscopedinit— a collidingskill_namedoes not error, it credits whichever skill fires.Deliberately not taken. #109's
--splitguidance, itsrun_limits/setting_sourcestemplate notes, its threshold-currency warning, and its "planexpands the dataset" claim. Each was checked againstmainand is absent — porting the prose would document behavior this tree does not have.One test unpinned from ordering
test_install_failure_is_not_latchedpatchedsignal.signalmodule-wide for its whole duration, so the async teardown's ownsignal.signal(SIGINT, default_int_handler)restore hit the refusing mock and raisedValueErrorout of teardown. Latent — it does not fail on today's sharding, but any change to how tests distribute surfaces it, and one did. The patch is now lifted right after the calls under test, and the count assertion became a comparison of the two calls' signal sets.Not in this PR
The repo's
litellm/directory is importable as a namespace package, soimport litellmsucceeds and returns an empty module — which is whytests/test_judge_litellm.pyfails instead of skipping without the optional extra. Ten failures on a clean checkout, invisible to CI because CI installs it. Left for a separate change; work parked onfix/litellm-namespace-shadowing.Verification
make lint— exit 0, 382 passed (CE044 and CE045 both green)make test— the same 10 pre-existingtest_judge_litellm.pyfailures as cleanmain, and nothing elseruff check+formatcleanclaude plugin validate --strict;coder-eval planon the edited template🤖 Generated with Claude Code
https://claude.ai/code/session_01V5WZ1BbtygK9YVvcgPfNGh
Review round (commit
754805b)Every claim was reproduced before acting. Three proved my own text wrong.
Both blockers fixed. CE045's
[^"'\s]+stopped at the first space, soSKILL_SOURCE_PATH=${{ github.workspace }}/.claude/skillscaptured${{— thecisurface, which the rule's own docstring called the highest-cost one, was structurally unreachable, and a companion test asserted that truncation as correct. The mutation guard is now four verbatim pre-fix lines replayed through the full extract→predicate pipeline; verified to flag the reintroducedciline. Writing it that way caught two further bugs of mine (findallreports unmatched groups as"", notNone; the YAML walk mislabelled fixture paths).A plugin root loads the whole plugin. Verified: a root with
skills/probe-beta/,agents/probe-subagent.mdandcommands/probe-cmd.mdoffers all three. Pointing a suite at.claudehands the agent every project subagent — one that answers the request instead of the skill depresses recall for unrelated reasons and makes the number repo-dependent.check-skilland the template now prescribe a minimal staged root;cicarries the workflow step to build one.Three claims of mine were false:
PLUGIN_PATHis not Codex-only; the parity probe gave a false negative from a repo root (Claude Code discovers./.claude/skills/natively, so the deeper path still lists the skill — the namespace prefix is the real signal); and "tutorial 07 twice" was one line.Three more surfaces carried the bug, including
docs/AB_EXPERIMENTS.md's canonical A/B recipe, whosewith-skillarm loaded nothing and silently compared two identical arms. Also fixed:experiments/default.yaml,experiments/plugin-comparison.yaml, plusdocs/agents/CLAUDE_CODE.mdandCLAUDE.md's divergence index.A runtime guard.
process_pluginswarned only on an undefined env var; a path that exists but has noskills/reached the SDK silently. Codex and Antigravity already log the equivalent; claude-code — where it is fatal — did not. Now it does, with four tests. That reaches user repos, which a repo-scoped lint rule structurally cannot.Deferred: namespaced
skill_namematching, the sibling tests' process-widesignal.signalpatches, and the proposed CE046/CE047 meta-rules — real, but their own changes. The migration note for already-emitted user suites needs a release decision first.On the red check
Live Integration Tests (Settings Enforcement + Cost Budget)→test_broad_deny_limits_agent_to_sandboxfailed withAgent did not attempt to read outside the sandbox. Calls: []. The test needs the agent to attempt the read so the deny rule can be shown to block it; the model declined on its own ("the file name and directory name … make it clear this is a sandbox escape test"). A model-behavior flake —mainis green across its last sixpr-checksruns and this diff touches nothing near settings enforcement. Separately worth noting: the test is fragile by construction, since it depends on the model choosing to misbehave.