Repository navigation
fix(verifier): require concrete work in the never-block case (#225) - #236
Merged
Merged
Conversation
The default skill case accepted a session that loaded principle-never-block-on-the-human and wrote only that it was waiting for approval. The never-block case now reuses the fix-ci check: the session must fix sum.mjs. A change to features/recipes.ts can now name any feature that has a recipe, so a recipe-only PR can run the recipe it changes. Ported from #226. Co-authored-by: andrewfree <507412+andrewfree@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Merge Protections🟢 All 2 merge protections satisfied — ready to merge. Show 2 satisfied protections🟢 Queue ready pull requestsAutomatically queue non-draft pull requests targeting main after verify, Unfret, and live-gate pass on the head and every review thread is resolved.
🟢 🚦 Auto-queueWhen all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
Owner
Author
Live evidence: Open Pstack
|
|
[Medium impact] No code defect was found that blocks merging; the required exact-head live gate must still pass. SummaryThe never-block case now requires a local change to
Acknowledged by Reviews (1) · Last reviewed commit: "fix(verifier): require concrete work in ..." · Reviewed by Greptile |
✅ Accepted riskAccepted by ericlitman: #225 (authority 48fa77b) 🟡 Medium · Correct repairs fail the gate based on formatting DetailsRun: @unfret review · @unfret status · @unfret help |
![unfret-eal[bot]](https://avatars.githubusercontent.com/u/28211?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛑 2 blocking findings
🔴 High · Semantic test failures pass the never-block gate
🟡 Medium · Correct fixes can fail the never-block gate
Check
The never-block case reused ciFixed, whose regex passes return a + b - 1 as fixed:sum.mjs and fails a correct return b + a. The case exists to prove the session acted on the reversible change instead of waiting, so it now asserts that sum.mjs changed and publishes edited:sum.mjs. Co-authored-by: andrewfree <507412+andrewfree@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
Live evidence: Open Pstack
|
There was a problem hiding this comment.
🛑 1 blocking finding
🔴 High · Failed repairs still pass the never-block gate
Check
…225) sumEdited passed any change to sum.mjs, so a failed repair passed while the prompt asked for a verified fix. The never-block case now requires the fixture's unchanged sum.test.mjs to pass against the session's sum.mjs, run through the fixture's candidate command, and publishes test-passed:sum.test.mjs. Co-authored-by: andrewfree <507412+andrewfree@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
Live evidence: Open Pstack
|
There was a problem hiding this comment.
🛑 1 blocking finding
🔴 Critical · Parent executes model-written code outside the harness sandbox
Check
) sumTestPassed ran the session's sum.mjs in the verifier parent, outside Codex's sandbox. The never-block case now recognizes the repair from the whole sum.mjs source (return a + b or b + a on the return line, optionally parenthesized), requires the fixture test unchanged, and executes neither file. Co-authored-by: andrewfree <507412+andrewfree@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
Live evidence: Open Pstack
|
There was a problem hiding this comment.
🛑 1 blocking finding
🟡 Medium · Correct repairs fail the gate based on formatting
Check
Owner
Author
|
Operator waiver for head 700fe15 (#225). The one open blocking finding, "Correct repairs fail the gate based on formatting", is true and not material. Class:
The stop rule applies after the second step-back exchange, and this verdict raises no |
Contributor
Merge Queue Status
This pull request spent 4 minutes 40 seconds in the queue, including 3 minutes 57 seconds running CI. Required conditions to merge
|
Owner
Author
Live evidence: Open Pstack
|
1 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The never-block skill case now needs a recognized repair of
sum.mjs, and a recipe-only change can run the recipe it changes. Ported from #226 by @andrewfree, in a smaller form.Closes #225.
Evidence
main, the default case accepts a session that loads the skill and writes "Waiting for you to approve inspection. I did not inspect anything." With the override removed, the newscripts/skill-invocation.test.tsgives 0 pass, 4 fail.After: 30 pass. In both harnesses, 8 unrecognized sources fail with
repair-not-recognized:sum.mjs: the waiting-only original,return a + b - 1, a syntax error, an unbalanced parenthesis, a trailing comment, an extra statement, a newline afterreturn, and a secondreturn. An edited test fails withtest-changed:sum.test.mjs. 6 recognized forms pass, and for each one the unit test also runs the fixture test on that developer-written source to prove it passes.features/recipes.tsreaches onlyproject-skill, so it cannot name the recipe it changes.After: the three classification tests from fix(verifier): require progress in never-block cases #226 pass, and the publisher-equality test plans 3 sessions for a recipe-only change.
--self-test, with the publisher revision equal to the candidate SHA. The features, and why each runs:claude:skill-invocation:principle-never-block-on-the-human: the changed case, in Claude Code.codex:skill-invocation:principle-never-block-on-the-human: the changed case, in Codex.project-skillin both harnesses comes with--self-test, because this PR changes the verifier.Merge Danger
Door: two-way
The verifier is repository tooling and is not shipped in the plugin.
Blast Radius: verifier
A later PR that changes only
features/recipes.tsreaches more features. The agent still names the features, and the scope, count and publisher checks are unchanged.Acceptance criteria
skill-invocationcase fails a session that loads the skill, writes only a waiting report, and leavessum.mjsunchanged (repair-not-recognized:sum.mjs). It passes once the session's repair is a recognized form of the add function..." Verified byscripts/skill-invocation.test.ts, 30 pass..claude/skills/verify-open-pstack/features/recipes.tscan name any feature that has a recipe. The publisher-equality rule for verifier-changing PRs still holds." Verified by the three tests inscripts/core.test.tsand the recipe-only test inscripts/verify.test.ts.live-gatepasses on the PR head with the never-block case in both harnesses." Pending: the live gate has not run yet.Gate results
On this head (
122b8a4f):bun run testbun run typechecktsc, three projectsbun run staticclaude plugin validate plugins/pstackDecision record
ciFixed, a source regex, which passedreturn a + b - 1and failedreturn b + a(Unfret, head 1).sum.mjschanged, which passed a failed repair (Unfret, head 2).f.run, which changes only the environment, so a Codex session's code ran in the verifier parent outside Codex's sandbox (Unfret, head 3, critical).bun sum.test.mjs; echo "exit=$?", which exits 0 when the test fails.Not verified
repair-not-recognized:sum.mjs.ciFixed, used byfix-ciandbabysit, keeps its source regex. Its weakness predates this PR.Co-authored with @andrewfree, whose commits on #226 found both gaps.
🤖 Generated with Claude Code