Skip to content

fix(pr-verify): report an untested file-tool fence apart from an open one - #13963

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/capture-probe-never-tried
Nov 7, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/capture-probe-never-tried

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

No related issue: lane-sweep finding 8b25204f106f4541, listed in the bot-written digest #9922 (a digest entry, not a spec)

Summary

Lane-sweep finding 8b25204f106f4541 on #9922: when a probe leg made no Read call on any fenced path, capture-probe.mjs reads reported every target "not tried" and exited 1, and the workflow said the file tools "were not fenced off /proc", a fence breach, when the fence was never tested. The check now reports that case separately. Both cases still fail the job.

Fix

  • .github/scripts/pr-verify/capture-probe.mjs: reads exits 1 when any target was read or got no answer, whatever the others did. Otherwise it exits 3 when any target was never tried (all not tried, or refused mixed with not tried). Exit 3 matches what policy-probe.mjs uses for an unproven run (fix(ci): frame the eval policy probe as a self-test and rerun a no-call session #13665).
  • .github/workflows/pr-verify-capture.yml (the capture job; the subscription job no longer runs the probe) and the three identical checks in .github/workflows/upstream-apply-changelog.yml: exit 3 prints "made no Read call ... so its file-tool fence was not tested"; any other non-zero exit keeps the breach message. The step fails either way.
  • No retry: the probe is a run-claude-cli step, which bash cannot loop, and workflow.test.mjs pins two launcher steps.

Verification

  • capture-probe.test.mjs, pr-verify/workflow.test.mjs and upstream-apply-changelog.test.mjs: 150 of 150 pass. On the old sources, one new test fails in each file (147 of 150).
  • The fence check is no weaker: a read or an unanswered target still exits 1 and fails the job.

Related

Follow-ups

  • Pre-existing (advisory): a missing or non-JSON execution file still makes reads exit 1, so it is reported as a breach too (the catch branch in reads()), the same "no evidence reported as a breach" shape.

session: automation-lanes

🤖 Generated with Claude Code

… one

capture-probe.mjs reads exits 3 when no target Read came back or went
unanswered but a target was never tried, so a probe session that made no
Read call no longer reads as "the driver's file tools are not fenced".
Any read or unanswered Read still exits 1. Check the probe in
pr-verify-capture.yml and the three scrub-probe checks in
upstream-apply-changelog.yml name the untested case and still fail.

Refs: #9922

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

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

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

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-07T16:00:31.002254Z
We couldn’t complete this review. Comment @codex review to try again.
ae90043 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.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

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

Code review: no findings

I read every hunk in all 7 changed files. The PR now reports a probe leg that never tried a Read on a fenced path as an untested fence (exit 3) instead of an open one. The job still fails either way.

What I checked

  • capture-probe.mjs reads(): the order is right. clean exits 0. Any read or unknown status exits 1 before the not tried check, so a breach is never downgraded to "untested", even when other paths went untried. A refused path mixed with an untried one exits 3. An empty target list still exits 1, and so does a missing or non-JSON execution file. main() returns the code as process.exitCode, and no other branch of reads can return 3.
  • Fail-closed behaviour in the workflows: in pr-verify-capture.yml (Check the probe) and the three copies in upstream-apply-changelog.yml, node … || reads=$? keeps a non-zero exit from tripping -e too early. 0) falls through to the existing Write and Grep checks. Both 3) and *) exit 1: fail() exits 1 at upstream-apply-changelog.yml#L1439-L1442. The fence is no weaker.
  • Tests: the new capture-probe.test.mjs case covers no messages, Bash-only, a partial refusal, a Read that came back and a Read with no result. The workflow.test.mjs case runs the real step script against hand-built transcripts for both messages. scriptOf, tempDir and mkdirSync are already imported or defined in that file. The upstream-apply-changelog.test.mjs case pins the new untested message.
  • Changelog fragment: bump: patch fits a fix that changes only messages and an exit code.

Context

  • The PR description says Refs: #9922 and closes no issue, so there were no acceptance criteria to check.
  • The pass counts in the description (150 of 150 tests) are the author's claim. I did not run them, because this lane has no shell.
  • The pre-existing case that the PR's own Follow-ups list already records (a missing or non-JSON execution file still exits 1 and is reported as a breach) is unchanged and outside this PR's scope.
  • This PR changes nothing that costs money and needs no hold. No embedded instructions were found in the PR context.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

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

Security review: PR #13963. No security issues found

I read every hunk of all 7 changed files. Two of them match trusted paths and got the extra scrutiny: .github/workflows/pr-verify-capture.yml and .github/workflows/upstream-apply-changelog.yml.

What changed. capture-probe.mjs reads used to exit 1 in every failure case. It now exits 3 when the probe session made no Read call on a fenced path and no other path was read or left unanswered. The four workflow checks that call it now use a case on the exit code, so they can print a different message for exit 3.

Is the fence check still fail-closed? Yes.

  • The script. reads still returns 0 only when fileReach reports clean, meaning every target was refused (capture-probe.mjs#L152, #L170). If any path was read or unknown, it still returns 1, even when other paths were never tried (#L171). Every other case returns a non-zero code. A missing or non-JSON execution file still returns 1 (#L164-L167).
  • The pr-verify-capture.yml check. case $reads in 0) ;; is the only branch that passes. The 3) and *) branches both exit 1 (pr-verify-capture.yml#L1004-L1016). || reads=$? captures the exit code without tripping errexit. reads=0 is set right before the call, so a stale value can't carry over.
  • The three upstream-apply-changelog.yml checks. Their 3) and *) branches call fail. fail echoes an error and then exit 1 at each of its three definitions (L1439-L1442, L1667-L1670, L1890-L1893).
  • Which copy of the script runs. Every check still runs the script from the base checkout (node .base/.github/scripts/pr-verify/capture-probe.mjs), so code on the PR's head branch can't change the check that judges it.
  • Messages. The new error text is fixed. It contains no untrusted input and prints no file content.

Trust controls. Nothing widens. The diff doesn't touch the trusted-actor list, token scope, the kill switch, merge holds, permissions: or triggers. The workflow changes are limited to how the probe check's exit code is handled.

Instruction surfaces and plugin manifests. The diff changes no standing instruction and no plugin manifest.

PR context. The description contains no embedded instructions aimed at this lane. Its claim that "both cases still fail the job" matches the code. The follow-up it lists, a missing execution file being reported as a breach, is in code this PR doesn't change and fails closed, so it is not a finding here.

Prior findings. None. This is the lane's first review of this PR, so it was not limited to blocking findings.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

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

Summary: Fail: there is no trusted spec to judge this against. The pull request names issue #9922
  only through a `Refs:` line, and that issue's body is null in spec-text.json because a bot
  (github-actions) wrote and last edited it. The issue's only trusted comment is a work-item-lease
  marker, which states no outcome, scope or done-when items. The instructions say to return fail
  when no issue has trusted spec text. The diff itself looks sound. `capture-probe.mjs reads` now
  exits 3 when a path was never tried and nothing was read or left unanswered, and 1 on any read or
  unanswered Read. pr-verify-capture.yml and the three checks in upstream-apply-changelog.yml still
  fail the job on both exit codes, only with different messages, so the fence check is no weaker.
  New tests cover both cases and could fail. test-node and test-bash passed on the judged head
  (https://github.com/melodic-software/claude-code-plugins/actions/runs/42389097118/job/128452801512).
  lint-shell was skipped. Nothing in the diff touches the takeover set, secrets or holds, and the
  diff contains no binary files.
- Issue #9922 has no trusted spec text: a bot wrote and last edited its body, and its only trusted
  comment is a lease marker. To fix, link an issue whose body a trusted person wrote with a
  done-when item for reporting an untested file-tool fence separately from an open one, or have a
  trusted person add that requirement to #9922. Then re-run verification.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass removes the label, at a new commit or at this same head: once a trusted person edits the body, and only trusted people have edited it since the fail, this head is judged again, so a fail on the body or the issues it links needs only the body fixed, no new commit.

@github-actions github-actions Bot added pr-verify-failed pr-verify failed this head; a pass at a new commit, or after a trusted body edit, removes it. and removed pr-verify-failed pr-verify failed this head; a pass at a new commit, or after a trusted body edit, removes it. labels Nov 7, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 83ef5b2 Nov 7, 2026
97 checks passed
@kyle-sexton
kyle-sexton deleted the fix/capture-probe-never-tried branch November 7, 2026 16:37
@github-actions github-actions Bot added the merge-path:owner-agent Post-merge accounting record (README.md, Merge paths) label Nov 7, 2026
@kyle-sexton

Copy link
Copy Markdown
Member Author

Record of a linkage change after a pr-verify fail. pr-verify failed head ae90043 at 16:09:41Z only because the linked issue #9922 is a bot-written lane-sweep digest with no trusted spec text. The same verdict judged the diff: "The diff itself looks sound ... pr-verify-capture.yml and the three checks in upstream-apply-changelog.yml still fail the job on both exit codes, only with different messages, so the fence check is no weaker. New tests cover both cases and could fail." I then changed the first body line from Refs: #9922 to No related issue: lane-sweep finding 8b25204f106f4541 ..., since a digest entry is not a spec, and the re-judge passed against the PR body, which this session wrote. The code was not changed between the two verdicts.

Process gap: lane-sweep digest findings, like recurring-schedule issues before #13962, have no trusted-spec path, so every PR fixing one hits this.

session: automation-lanes

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

Labels

merge-path:owner-agent Post-merge accounting record (README.md, Merge paths)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant