[kyle-sexton]

Closes #12988

Summary

run-validity.py passed eval runs whose fetch the sandbox's network block had denied, so a score that measured a blocked request looked like a valid measurement of the skill. A tool result whose <sandbox_violations> block holds a deny network-outbound <host>:<port> line now counts as a denial, and the summary names the denied hosts.

Fix

• A new denial source in check_denials, read only from tool_result blocks, so a prompt or answer that quotes the report text is not counted. Most real violations arrive with is_error: false, so the existing permission-denial reader never saw them.
• A with-arm network denial is a FAIL. A without-arm one follows the existing denial rules. In an invocation-probe or control case it is set aside as a warning, the same way own-data denials are. These are the three choices recorded on the issue.
• SKILL.md, reference/reading-results.md and the script docstring describe the new source.

Verification

• run-validity.test.sh: 156 tests OK, 5 of them new (with-arm FAIL naming the host, the every-run case still FAILs, a same-score without-arm WARN, probe and control WARN, quoted report text and a non-network violation stay PASS).
• On the real fix(x): the read skill's spool write is denied in every eval run, so fetch cases cannot be measured #12882 smoke 2 result, the check now prints FAIL (9 denials in 9 runs; 9 sandbox network denials, outbound to publish.twitter.com:443, x.com:443, xtomd.com:443). Before this change it printed PASS.
• scripts/run-ruff.sh check passes on both Python files.
• Left out: no paid eval run, because this changes only how a finished run is judged.

Related

• fix(x): the read skill's spool write is denied in every eval run, so fetch cases cannot be measured #12882: the x suite smoke that found the gap.
• test(x): fetch cases get a scaffolded response, since plugin eval blocks the agent's network #12987: the x fetch cases the sandbox blocks.

🤖 Generated with Claude Code

https://claude.ai/code/session_015te8Cs4gtBwNShf5j6Qg1o

[chatgpt-codex-connector]

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-04T00:34:44.301532Z	f38ba65	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]

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

Security review (incremental, f38ba65..b83eefa): no security issues found

This review covers only the changes since f38ba65. I read every hunk of all 6 files listed in .claude-lane/incremental-files.txt, in .claude-lane/incremental.diff.

What changed since the last review. In run-validity.py, network_denials returns nothing for a without-arm run. It now reads only tool results that belong to a Bash tool_use, so a report quoted in a Read result or anywhere else is not counted. A new no-network prompt-frontmatter tag (NO_NETWORK_TAG, is_no_network) joins the probe and control cases: in those cases a with-arm network denial is a warning, not a FAIL. The warned filter now compares len(network[r]) == len(offline[r]), which matches the with-arm-only collector. The fixture host is now xtomd.com:443, which the closing issue's checklist asks for. The tests were updated to match. The docs (SKILL.md, reading-results.md, the docstring and the changelog fragment) now describe the report in prose and point to a new Sources row instead of quoting the upstream format.

Surfaces checked.

• Trust controls and trusted paths: none of the 6 changed files matches a TRUSTED PATHS glob. Nothing touches the trusted-trigger check, a lane token request, the kill switch, the token broker or a merge-hold module.
• Plugin manifests: no plugin.json or marketplace.json changed, so there are no manifest <field> lines.
• Instruction surface: SKILL.md and reading-results.md narrow the eval-validity check in two ways: without-arm network denials are no longer read, and the new no-network tag turns a FAIL into a warning. I checked both by what breaks when the rule is gone. Each only decides whether an eval run's score is reported as a valid measurement. Neither touches a secret, a token, an install path, a merge gate or code execution, so neither falls in a protected class. Both match the scope and checklist recorded on fix(evals): run-validity passes runs whose fetch the sandbox's network block denied #12988. docs/contracts/instruction-exception-register/README.md was not opened in this run, so the protected-class list was not consulted. I judged by consequence instead.
• Untrusted input: the parser still reads local eval trace JSONL with regex findall and string joins only. A trace's tool name or tag values reach no shell, file path, template or credentialed action, so there is no injection path.
• Secrets and tokens: none are touched. The fixture holds only a public host and a made-up curl command.

Settled threads: the three owner-resolved threads (the no-network tag, with-arm-only scope, and the restated upstream format) are marked lines changed since resolution: no and raise no security point. None is re-raised.

Prior findings: this lane's earlier summary had 0 findings, so nothing carries over.

Spend: none. The change only alters how a finished eval run is judged and starts no runs.

critical-findings: 0

[github-actions]

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

claude-lane: claude-review

[github-actions]

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

Code review (incremental, f38ba65..b83eefa): no new findings

Read: every hunk of all 6 changed files in .claude-lane/incremental.diff. Also read for context: check_denials at head (run-validity.py#L2150-L2251), closing issue #12988, and the settled threads and prior findings.

Earlier findings: all three are fixed and settled

Each was resolved with a decision from the owner, and the job marked none of them lines changed since resolution: yes. I checked each fix against the code:

1. The no-network tag (closing-issue criterion). NO_NETWORK_TAG and is_no_network are read through prompt_fields, the same way is_probe is. offline now sets these cases aside. The fixture now holds deny network-outbound xtomd.com:443. The tests assert FAIL for a with-arm denial and WARN for a probe, a control case and a no-network case. All four fix(evals): run-validity passes runs whose fetch the sandbox's network block denied #12988 checklist items have code and a test that would fail if they broke.
2. Scope limited to the with-arm run. network_denials returns [] when r.arm != "with", and it reads only results of Bash tool_use ids, which the checklist names. The warned condition is now len(network[r]) == len(offline[r]). It is equivalent to the old r.arm == "with" guard because a without-arm run now has no network denials. A new test checks that a without-arm denial, in one run or in both, leaves the check at PASS.
3. Volatile upstream format (.claude/rules/skill-bodies-state-current-rules.md). SKILL.md and reading-results.md now describe the report in their own words and point to a new Sources row. That row has a pointer, the observed version (2.1.317), an as-of date of 2026-11-04 and a recheck trigger. The regex comment in the script points back to that row.

New code checked, nothing to report

• network_denials keeps a set of Bash ids. A result counts only when its id is in that set and not in skip. A Read result that quotes the report is not counted, and the new read-quoting.jsonl test covers that.
• The network_run helper's every argument now touches only the arm rows it names. No other caller depends on the old behaviour of filling both arms.
• The text in the changelog fragment, the script docstring and the summary strings matches the code's behaviour.

Not checked by me: the author says run-validity.test.sh gives 155 tests OK. That is their claim; I had no shell and ran nothing.

Manifest: no plugin.json or marketplace.json changed. The fragment's bump: patch fits a fix.

I found no embedded instructions in the PR context, the settled threads or the prior findings. The change spends nothing beyond prepaid usage.

[github-actions]

The pr-verify verifier failed head f38ba65daf5fe00d59b7211f43741f411fb99ce4. What it wrote, as data:

What it wrote (30 lines)

Summary: Issue #12988: the PR counts sandbox `deny network-outbound` violations from tool results as
  permission denials, names the hosts, fails with-arm ones, and lets invocation-probe and control
  cases warn instead. CI is green (test-python (0)
  https://github.com/melodic-software/claude-code-plugins/actions/runs/41747424372/job/126303971226,
  test-bash (0)
  https://github.com/melodic-software/claude-code-plugins/actions/runs/41747424372/job/126303971235).
  But part of the spec is missing. The issue body says a case that declares it needs no network can
  opt out. The trusted Decision-recorded comment makes that a `no-network` prompt tag, and its
  done-when items ask for a WARN test for a `no-network` case. The diff adds no such tag. Its
  fixture also uses example.org:443 rather than the required xtomd.com:443.
- plugins/evals/skills/plugin-eval/scripts/run-validity.py:2150: the `offline` opt-out only checks
  `is_probe` and `is_control`. The issue body and the Decision-recorded done-when item also require
  a case whose prompt frontmatter has the `no-network` tag to warn instead of fail. To fix, add a
  `no-network` tag check, for example a helper that reads the case's prompt.md tags the way
  `is_probe`/`is_control` do, include it in `offline`, and update the summary wording
  ('invocation-probe or control case') to match.
- plugins/evals/skills/plugin-eval/scripts/test_run_validity.py:1979: there is no test for a
  `no-network` case. The done-when item asks `test_run_validity.py` to assert WARN for a control
  case and for a `no-network` case. To fix, add a `network_run(tags="tags: [knowledge,
  no-network]")` case that asserts WARN.
- plugins/evals/skills/plugin-eval/scripts/fixtures/run-validity/traces/network-denied.jsonl:3: the
  done-when item says the fixture trace holds a `deny network-outbound xtomd.com:443` violation.
  This fixture uses example.org:443. To fix, change the fixture host and the expected strings in the
  tests (NETWORK_FAIL and the others) to xtomd.com:443.
- plugins/evals/skills/plugin-eval/SKILL.md:296: the docs here and in
  reference/reading-results.md:105 list only probe and control cases as the exceptions. Once the
  `no-network` opt-out exists, add it to both.
- The PR body and an earlier lane-4 claim comment say no new tag was chosen. The later trusted
  Decision-recorded comment overrides that and asks for `no-network`, and the issue body itself asks
  for an opt-out for no-network cases. This is a spec gap, not an attempt to steer the reviewer.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass at a new commit removes the label.