Skip to content

feat(testing): make run-e2e follow a Mask hand-off and accept a none: values result - #13774

Merged
kyle-sexton merged 7 commits into
mainfrom
feat/run-e2e-mask-hand-off
Nov 7, 2026
Merged

kyle-sexton merged 7 commits into
mainfrom
feat/run-e2e-mask-hand-off

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Member

Closes #13759
Refs: #13757

session: App Driven PR Verify

Summary

/testing:run-e2e scans e2e-summary.json with the values from the hub's ## Mask values: line. It now uses the Mask in effect: the hub's own ## Mask, or its hand-off target's. These are the rules PR 13765 adds to app-control:

  • The hand-off target resolves from the hub copy that was read, inside trusted-root=, never from the checkout under test.
  • Each values: source is judged on its own; a single none: line with exit 0 means that source has no values.
  • A hand-off never falls into the "no values: line" branch. An unreadable target or a failed source keeps e2e-summary.json out of publishable.

Fix

  • plugins/testing/skills/run-e2e/SKILL.md, Steps 2 and 5. They point at drive's "Mask post-check" instead of restating it.
  • New evals:
    • affected-05: the target's source fails, so the summary stays unpublishable even though its text is clean;
    • affected-06: the target's source prints none:, so the summary is scanned with the generic pattern and published.
  • A changelog fragment (bump: minor).

Verification

  • bash scripts/check-changed-skills.sh origin/main: PASS, 0 errors. The one warning this change adds is run-e2e's line growth.
  • check-evals-quality.sh on run-e2e's evals: PASS.
  • scripts/check-affected.sh: 14 passed, 2 skipped.

Remaining

  • Merge order: PR 13765 merged first; this PR points at its "Mask post-check" text.
  • Merge hold: it changes a skill body, so it carries do-not-merge until a valid before/after eval lands (hub rule R379/R381).
  • Eval gap: the "never from the checkout" expectation in the two new evals cannot fail as written, because neither prompt holds a checkout copy that would give a different answer. The eval run should add that decoy.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits November 6, 2026 12:15
run-e2e's unattended summary scan now reads the Mask in effect: the
hub's own, or, when the hub hands off, its driving tool's, resolved from
the hub copy read and inside trusted-root=, never the checkout under
test. A values: source printing one `none: <reason>` line with exit 0
resolves with no values. An unreadable hand-off target or a failed
source keeps e2e-summary.json out of publishable.

Adds run-e2e evals affected-05 (failed source behind the hand-off) and
affected-06 (none: source, clean summary publishes).

Refs: #13757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 6, 2026 17:30
@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.

@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 6, 2026

@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 b79ed81e281fa048c1cdb16b72a33f3eb2b0bc3f; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread plugins/testing/skills/run-e2e/SKILL.md Resolved
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

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

Code review: incremental pass over 16b2ea02..55a91a31 (blocking only)

This lane has reviewed earlier heads of this PR, so this pass reports blocking findings only. I read every hunk of the one file in scope:

  • plugins/testing/evals/run-e2e-touched-feature-calls-refresh/samples/calls-refresh-for-cart-total.json

Incremental change: no blocking findings. The push adds three fail samples for the calls-refresh-for-cart-total regex grader, app-control:map[\s\S]{0,80}?\brefresh[ "]+cart/total\b`. I checked each one against the regex by hand. I could not run the sample checker in this lane.

  • Split-line form, wrong feature: misses. The only refresh is followed by checkout. cart/total comes later, and no refresh sits right before it.
  • "was not called": misses. The only refresh is the sentence-final refresh., and . is not in [ "]+`.
  • Skill-call form, no feature: misses. refresh is followed by --unattended.

Each sample names both the skill and cart/total, so together they pin the regex's requirement that cart/total come right after refresh, not just anywhere nearby. The four existing pass samples are unchanged and still match (re-checked).

Finding (restated: summary findings carry over each round)

  1. The post-check rules that run-e2e Step 5 relies on are still not on main. At this head, evidence.md L79-L82 still says "A source that fails or yields no value leaves every capture not-checked and both public reports unpublished". It has no hand-off rule and no none: exception.

    Meanwhile, run-e2e SKILL.md L277-L281 finds the hand-off target "as that post-check finds it" and publishes a none: result "as that post-check accepts it".

    How it fails: in the affected-06 setup, the target's source prints one none: line and exits 0. run-e2e publishes the summary, but the post-check on main rejects that source as one that "yields no value", and it has no hand-off to follow.

    Fix: the PR body sets the merge order (after PR 13765), but nothing enforces it. Either hold the merge until PR 13765 is on main, or enforce the order: stack this branch on PR 13765's branch, or add a dependency label.

Notes

  • Closing issue feat(app-control): publish masked CI captures through a hub Mask hand-off and a none: values result - part 2 of 3 #13759: this push does not touch affected-05, the eval its criterion names, so I re-checked nothing for it. An earlier review found that criterion met.
  • Settled thread (the "never from the checkout" condition at run-e2e SKILL.md L279): marked lines changed since resolution: no. It stays settled, and I did not raise it again.
  • Not verified: the author's Verification section (check-changed-skills.sh, check-evals-quality.sh, check-affected.sh) is their own claim. I could not run any of it in this lane.

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

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

Security review: PR #13774, incremental from 16b2ea02 to 55a91a31

No security issues found.

critical-findings: 0

This review only reports blocking findings, because the lane already reviewed an earlier head. Only CRITICAL and IMPORTANT findings count here. I read every hunk of the one file in the incremental scope (1 of 1).

  • plugins/testing/evals/run-e2e-touched-feature-calls-refresh/samples/calls-refresh-for-cart-total.json: the push adds three known-bad samples to the eval data for the loosened calls-refresh-for-cart-total regex grader:

    • the skill name and its arguments on separate lines, refreshing the wrong feature (checkout);
    • a run that names the skill but never calls refresh;
    • a Skill(...) call with no feature.

    These are static JSON strings that only feed eval grading. Nothing executes them, and they gate nothing in the merge pipeline or in any lane that holds credentials.

The file matches no TRUSTED PATHS glob. The push touches no plugin manifest, workflow, token request, trust control, merge hold or standing instruction.

The only prior finding is the summary comment at 16b2ea02, which has no line anchor and reported 0 findings, so nothing carries forward. The PR context file has no text that tries to steer the review.

@kyle-sexton

Copy link
Copy Markdown
Member Author

Held on do-not-merge only until a VALID no-loss before/after plugin-eval pair lands for this head (skill-body change, hub rule R379). It runs after PR 13765's pair, in the App Driven PR Verify session.

kyle-sexton and others added 5 commits November 6, 2026 15:31
…ed-root runs

Without trusted-root=, the hub copy read is the checkout's, so the hand-off
target resolves there; the clause now applies only when trusted-root= is given.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…esh grader

The regex grader failed a run that wrote the refresh as
Skill("app-control:map", "refresh cart/total --unattended") while the judge
grader passed it. The pattern now accepts that form too; a sample covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kill and arguments

A correct run wrote the refresh as `/app-control:map` with arguments on the
next line, `refresh cart/total --unattended`, and the regex failed it. The
pattern now allows up to 80 characters of formatting between the skill name
and `refresh`, still requiring `refresh` then `cart/total`; a sample covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wider pattern must still fail a split-line refresh of the wrong feature,
a reply naming cart/total with no refresh, and a Skill-call refresh with no
feature; all three samples fail it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton

Copy link
Copy Markdown
Member Author

Eval pair: VALID, no loss. plugins/testing: the run-e2e skill before (67e80f97, main merged into this branch) and after (16b2ea02). Both sides used the head's evals/, so only the skill differs. Model and judge claude-opus-5-5, Claude Code 2.1.319, --scaffold --allow-tools Bash Write Edit --runs 6, the 11 cases that drive run-e2e.

  • Result: head minus base, paired over 11 cases: +0.02, 95% interval -0.01 to +0.04, non-inferior at margin 0.05 (noise-report.py --baseline --margin 0.05). Mean with-arm score 0.99.

  • Validity: run-validity.py --runs 6 --scaffolded returned VALID on both sides, with warnings for skill fired and judge votes. It ran on the laptop that holds the traces, per the hub's relay of the lane verdicts.

  • Per case:

    • run-e2e-app-control-absent-reports-drift: 0.83 to 1.00.
    • run-e2e-route-decline-uses-testing-keys: 0.92 to 0.92.
    • The other 9 cases: 1.00 to 1.00.
    • No case lost.
  • Earlier rounds:

    • Rounds 1 and 2 (3 runs each) were VALID but not shown non-inferior. Their intervals were -0.07 to +0.01 and -0.09 to +0.03.
    • A regex grader had failed correct runs in both rounds. It rejected Skill("app-control:map", "refresh cart/total …") and a two-line form, while the judge grader passed both. The grader was fixed before round 3 (e30b79df, 16b2ea02).
  • Grader check: the loosened pattern is app-control:map[\s\S]{0,80}?\brefresh[ "]+cart/total\b`.

    Sample Expected Matches
    /app-control:map refresh cart/total --unattended pass yes
    backticked app-control:map refresh cart/total pass yes
    Skill("app-control:map", "refresh cart/total --unattended") pass yes
    /app-control:map with arguments on the next line refresh cart/total --unattended pass yes
    refresh with no feature fail no
    refresh of the legacy map path fail no
    refresh of checkout fail no
    split-line refresh of checkout with cart/total named nearby fail no
    names the skill and cart/total, with no refresh call fail no
    Skill refresh with no feature fail no
  • Measured head vs current head: measured at 16b2ea02. The only later commit, 55a91a31, adds only grader samples (samples/calls-refresh-for-cart-total.json). The case loader does not read samples: it reads only prompt.md, case.yaml and graders/.

  • Merge order: this PR still merges only after feat(app-control): let a hub hand off its Mask and a values source resolve to none #13765 (the body's Hold line).

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

The do-not-merge hold waited only on eval evidence (reason=eval), and a listed human posted a VALID no-loss eval-pair record for 55a91a31b1950e1b36ba65d16e10b1c8c12ef783: #13774 (comment). pr-merge removed do-not-merge. To hold again, add the label back with a new pr-hold reason line.

@melodic-automation-lanes melodic-automation-lanes Bot removed the do-not-merge Hard merge gate: do not merge while applied. label 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 c814889 Nov 7, 2026
74 of 76 checks passed
@kyle-sexton
kyle-sexton deleted the feat/run-e2e-mask-hand-off branch November 7, 2026 01:08
@github-actions github-actions Bot added the merge-path:owner-agent Post-merge accounting record (README.md, Merge paths) label Nov 7, 2026
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.

feat(app-control): publish masked CI captures through a hub Mask hand-off and a none: values result - part 2 of 3

1 participant