Skip to content

fix(evals): run invocation probes without subagents and name the run line in run-validity - #11901

Merged
kyle-sexton merged 3 commits into
mainfrom
fix/probe-suite-validity
Nov 1, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
fix/probe-suite-validity

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: probe suites are judged INVALID by construction, blocking #11895

Summary

Every side of #11895's round-4 probe comparison came out INVALID under run-validity.py. On current main two causes remain, and both come from the run setup, not the skill:

  • Timeouts. After the skill loaded, the agent handed the task to a subagent ("I don't have a file-writing tool, so I'm handing the build to a subagent"), and the run outlived its 300-second timeout. All 33 timed-out probe runs across the four sides were with-arm runs; 32 of them called Agent. The remaining one read the skill's spokes with Read and Glob, then ran out the clock.
  • Denials. All 27 denials that failed the gate across the four sides were Glob or Grep calls made from the empty workspace. The round ran without the emitted README's --disallow-tools Read Glob Grep (fix(skill-quality): run emitted invocation probes without the read-only search tools #11755), so every probe session's init line still listed Task, Glob, Grep, Read.

The third reason the round reported, skill fired failing should-fire probes that missed, is already fixed on main by #11639: a case tagged invocation-probe, which emit-plugin-eval writes, reports misses as the measured rate (WARN). The round read the gate from a checkout older than #11639. On current main that check is WARN on all four sides.

Fix

  • measure-invocation.sh emit-plugin-eval README: the run line now reads --disallow-tools Read Glob Grep Agent, and it says why. A case's allowed_tools only grants tools; the plugin-evals page has no per-case field that removes one, so the run line is the only place this can be set.
  • run-validity.py gains a probe tools check. It warns when an invocation-probe run's session could call Read, Glob, Grep or Agent (Task on the init line), and names the run line. It only warns: the error and denial checks still judge those runs at full force. The mode marker stays the existing invocation-probe tag the emitter writes in the suite, not a caller flag.
  • Docs: reading-results.md states how the gate reads invocation probes; the plugin-eval skill points there; invocation-probes.md gives the new run line, with an updated pointer record; rewrite-a-plugin.md points at the README's run line instead of a copy of it that had gone stale.

Verification

  • run-validity.test.sh: 96 tests pass. The three new probe tools tests fail against origin/main's run-validity.py (93 pass, 3 fail), and every other test passes there.
  • measure-invocation.test.sh: all pass. Its README assertion now requires Agent, and origin/main's emitter has no such line.
  • scripts/check-affected.sh: 15 passed.
  • Round-4 traces: all 33 timeouts and all 27 failing denials share the cause above. The current gate on the base side prints check probe tools: WARN (297 runs ... could call Read/Glob/Grep/Agent/Task ...).
  • claude -p --disallowed-tools Agent Read Glob Grep (Claude Code 2.1.316): the init line lists Bash, Skill, TaskStop, ToolSearch, with neither Agent nor Task, so Agent is the name that removes the subagent tool.
  • Not yet run: a live claude plugin eval probe run with the new line. The fix(user-interface): trigger design on single visual-value questions #11895 re-measure will use it, and its probe tools line confirms whether the flag took effect.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits November 1, 2026 02:38
…line in run-validity

Emitted probe runs now remove Agent beside Read, Glob and Grep: after the
skill loaded, the agent handed the task to a subagent that outlived the
300-second timeout, and every timed-out probe run in the #11895 round
had dispatched one. run-validity.py gains a probe tools check that warns
when a probe run could still call those tools and names the run line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 1, 2026 07:41
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T07:44:32.315776Z b4ee1c0 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed b97668626f10db0b124dbc2a943826474cec32ea: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Security review: incremental (b4ee1c0 → b976686)

No security issues found.

What changed in this increment. The probe tools check in run-validity.py used to pass silently when a probe trace had no init tools list. It now reports unread probe runs:

  • Some unread: the check is a WARN.
  • None readable: the check is UNCHECKED, which makes the run INVALID.

The rest of the increment is:

  • a fixture update: clean.jsonl and the test traces now carry "tools":["Skill"];
  • three new tests;
  • matching text in the changeset entry and in reading-results.md.

Why there's nothing to report:

  • Local tooling only. This is eval-validity tooling that runs on a developer's machine. It parses local JSONL traces and prints a verdict, and it gates no credentialed lane, token, merge hold or trust control.
  • No dangerous input handling. It adds no new handling of untrusted input: no subprocess calls, no path building from trace content beyond what the code already prints, and no network access.
  • Stricter, not looser. The check now fails closed (UNCHECKED, so the run is INVALID) where it used to pass.
  • No trusted paths or manifests. None of the changed files matches a trusted-path glob, and the increment changes no plugin manifest.
  • No instruction removed. The docs change adds text and removes no rule.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

claude-review: 1 inline finding(s) on b4ee1c029e1f14a98f89499fe228fb77d2824246; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed b97668626f10db0b124dbc2a943826474cec32ea: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental, b4ee1c0 → b976686)

No findings. I read every hunk of the incremental diff in all 5 in-scope files and found nothing to block or flag.

What changed

  • check_probe_tools (run-validity.py#L1104-L1180) now counts invocation-probe runs and keeps a list of runs with no readable init tools list. A run is unread when it has no trace, no init line, or a tools value that is not a list.
    • If every probe run is unread, the check is UNCHECKED. main treats UNCHECKED like FAIL, so the verdict is INVALID.
    • If only some are unread, the check is a WARN, and the WARN lists those runs next to any runs that could call a removed tool.
    • With no probe case at all, the check passes as before.
  • I checked the edge cases. next(...) takes the first init line, which matches the old break. An empty tools: [] counts as read and passes. When some runs have removable tools and others are unread, the summaries are joined with ; and both kinds of runs are listed in the details.
  • Matches the other checks. check_denials already goes UNCHECKED when even one trace is missing. The new check is looser: it only goes UNCHECKED when no probe run can be read. So a probe suite run without --keep-temp was already INVALID through the denials check. Adding "probe tools" to the traces: line (#L1372-L1377) is correct.
  • Tests. clean.jsonl now carries "tools":["Skill"]. This matters because probe_with_tools and probe_unread only replace the with-arm trace, so the without-arm run reads that fixture. Without the change, the existing PASS test would turn into a WARN. The three new tests cover a missing trace (WARN, and the trace is named), all runs unread (UNCHECKED and INVALID, with the reason on the verdict line), and one of two runs unread (WARN and VALID). Each assertion matches the format strings in the code. I could not run the tests (no shell in this lane); the PR body's pass counts were written before this commit and are the author's claim, unverified.
  • Docs and changelog. reading-results.md, the module docstring and the changelog fragment all describe the new WARN and UNCHECKED behavior. No other .md under plugins/evals still describes probe tools as warn-only.

Notes

  • The PR description's Fix section still says the probe tools check "only warns". Since this commit, a run where no probe trace can be read makes it UNCHECKED and the run INVALID. The changelog fragment states this correctly; consider updating the PR body to match. I am not counting this as a finding.
  • No closing issue: the PR context says "No related issue", so there are no acceptance criteria to check.
  • No plugin manifest changed in this increment.
  • No trusted-path file is in the incremental scope.
  • No spend or billing impact.

… tools

check_probe_tools passed when it had read nothing: a probe run with no
trace, or an init line with no tools list, added nothing. Such a run is
now a warning naming it, and none readable is UNCHECKED, so the run is
INVALID. The traces line names probe tools among the checks an
unresolved trace leaves unchecked. The clean fixture's init line and the
tests' inline init lines now carry a tools list, as real init lines do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit ca654eb Nov 1, 2026
42 checks passed
@kyle-sexton
kyle-sexton deleted the fix/probe-suite-validity branch November 1, 2026 09:06
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

Killed 1, survived 2, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 33%; mutation score 33%.

File Killed Survived Timeout No coverage Invalid Not run
plugins/skill-quality/scripts/measure-invocation.sh 1 2 0 0 0 0

Surviving mutants

Location Operator Disposition Cause
plugins/skill-quality/scripts/measure-invocation.sh:604 SBR productive no-assertion
plugins/skill-quality/scripts/measure-invocation.sh:605 SBR productive no-assertion

The run's summary has the full report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant