--user-approved: let the human say the thing HARD_DENY demands - #495
Merged
Conversation
Owner
Author
|
USER-APPROVED MERGE OVERRIDE Reason: maintainer decision 2026-08-05: --user-approved plus three #486 guidance edits. Reviewed by Codex and Fable (both NOT_CERTIFIED round 1, all findings fixed); both confirmed no argument shaping merges over red CI or removed tests. CI validate green. Maintainer chose to keep the merge-wrapper permission rule: they are present and reading, and their standing rule is to act when both reviewers agree. Waived (would otherwise have blocked):
Still verified, not waivable: CI Head: Posted by |
HARD_DENY printed "a human decides this one" and gave the human no way to say so. The maintainer hit that wall three times in one session; the only route left was clicking merge in the GitHub UI, which bypasses the hook harness and leaves no record in the repo. PR #479 was merged exactly that way. ./scripts/merge-pr.sh <PR#> --user-approved "<reason>" Waives process gates: both denylist tiers, the version-bump confirmation, and the clearance-artifact checks. Provably cannot waive a red CI validate or a PR that net-removes tests -- a human may waive paperwork, never facts. Both reviewers attacked that split and could not defeat it. Posts a durable override record to the PR (reason + every waived path) BEFORE merging, and refuses to merge if that post fails. An unrecorded override is the thing this exists to prevent. It does NOT prove a human typed it. Nothing available to a local script can. It makes an override explicit and durable instead of silent. Four defects found while using it, three by review: - Cleared the STRICTER tier then blocked on the weaker one. - `--user-approved --cross-model-cleared` consumed the next flag AS the reason, firing a full override justified by the string "--cross-model-cleared". Whitespace-only reasons passed too. Found by probing, not by the tests. - The success line always claimed "clearance CERTIFIED round>=2 fresh, denylist clear" -- a false audit record when the override skipped both. - The reason went to stderr and evaporated. PR #494 was merged with this flag and left zero durable trace, which is exactly what it replaced. Also #486, three low-risk edits both reviewers agreed on after cross-examination (they concluded SKILL.md should NOT split per model): - Concision guidance added at CLAUDE.md level, with Anthropic's actual position (a reminder in long prompts is endorsed) kept separate from this repo's stricter local ban. The first draft overstated the evidence. - Cross-model CI-log audit scoped to release/workflow/control-plane PRs. One recorded catch does not justify a per-push verification layer. - Preflight no longer frames fewer findings as success; the recheck's scope limiter is paired with "report every defect". The #486 regression test was itself vacuous -- it grepped for the wording the fix deletes, so it validated nothing. Now positively anchored and verified against the reviewer's own mutation. SKILL.md byte-neutral at 19,988/20,000; cowork copy byte-identical. Suite 65/65, merge gate 60/60. Status REVIEWED, not CERTIFIED: all findings are fixed but neither reviewer has seen the fixes.
BaseInfinity
force-pushed
the
fix/486-opus5-guidance
branch
from
August 7, 2026 05:22
286d21c to
129cd41
Compare
Owner
Author
|
USER-APPROVED MERGE OVERRIDE Reason: maintainer decision 2026-08-05: --user-approved plus three #486 guidance edits. Codex and Fable both reviewed (NOT_CERTIFIED round 1, all findings fixed); both confirmed no argument shaping merges over red CI or removed tests. CI validate green on the rebased head. Maintainer kept the merge-wrapper permission rule: they are present and reading, and their standing rule is to act when both reviewers agree. Waived (would otherwise have blocked):
Still verified, not waivable: CI Head: Posted by |
BaseInfinity
added a commit
that referenced
this pull request
Aug 7, 2026
… tests meant to catch them (#497) * fix(hooks,docs): three shipped defects, plus the tests that were supposed to catch them v1.94.0 was meant to remove every unbounded stdin read from the shipped hooks. It missed one. `hooks/model-effort-check.sh` still drained stdin with a bare `cat > /dev/null` — the same read-to-EOF that was observed alive at 10h19m against a 10-second timeout — while its SessionStart twin `token-spike-check.sh` was converted correctly. The reason it escaped is the interesting part. `tests/test-hook-stdin-bounded.sh` already tested this behaviourally, but its hook roster was hand-listed and `model-effort-check.sh` was never in it. A hand-maintained roster stops covering whatever nobody remembered to add, and reports success either way. The roster is now derived from `hooks/hooks.json` (6 -> 8 hooks) with a guard that fails if it ever stops matching the manifest, and a rostered file that does not resolve now fails instead of being silently skipped. Two more defects that shipped: `hooks/instructions-loaded-check.sh` told every consumer, on every session start with an update available, to run a global npm install of Claude Code. GH #476 read as "we omit native-install guidance"; in fact we shipped the opposite advice, from a hook that cannot detect which channel installed the running binary. The nudge is now channel-neutral. The docs promised a 30-minute stall watchdog governed by a variable that has never existed in this repo. `scripts/codex-review-with-progress.sh` loops on `kill -0` and enforces no timeout at all. The first fix here was to repoint the sentence at the wrapper that does exist — which ROADMAP.md had explicitly warned against, because renaming satisfies a path check while leaving the promise just as false, and harder to spot. Reverted; the claim is now replaced with what the wrapper actually does. Both reviewers then declined to certify, and the tests are the reason. A source-text regex for `cat` missed `$(cat 2>/dev/null)`, `/bin/cat`, `cat <&0`, `mapfile`, `IFS= read` without `-t` and more, while false-positiving on the same text inside a comment. A one-item variable denylist was escapable by renaming the phantom or dropping its name. Both are the unwinnable denylist shape ROADMAP #495(a) describes, so both are deleted rather than widened: the stdin guarantee is behavioural, and the watchdog guarantee is a positive assertion on the paragraph that defines it. Codex also found the defect one level out: `scripts/` is absent from package.json's `files`, so `npm pack` ships none of it, and five shipped references pointed consumers at tools they never receive — including `skills/sdlc/SKILL.md` mandating `scripts/merge-pr.sh`. The path test had blessed all five by asking whether they exist on the maintainer's disk. It now asks whether the consumer gets them. skills/sdlc/SKILL.md 19,988 -> 19,984 bytes, still byte-identical to its cowork twin. Suite 65/65. * fix(tests): round-2 review — close every paper-over both reviewers constructed Neither reviewer certified round 1, and both then broke round 2's fixes as well. Everything here is a defect one of them demonstrated, not a defect either of them merely suspected. Hook roster (Codex P1): the manifest extractor matched only flat hooks/<name>.sh, so a registered hooks/nested/hang.sh was reported as covered while never being executed — and the coverage guard repeated the same blind spot, so the two agreed on an answer that was wrong. Both now walk the manifest's actual command fields at any depth. Codex's exact mutation now fails. CI breakage (Fable P2-a, demonstrated): the new positive npm test ran the real hook, whose nudge branch is gated on `command -v claude`. ubuntu-latest has no Claude Code, so the test passed here and would have turned the required `validate` check red on push. claude and npm are now stubbed into an isolated PATH, which also pins the version instead of depending on whatever is installed locally. Shipped-doc coverage (Codex P1): the script-reference guard hand-picked its doc list, omitting README.md — which npm packs implicitly — plus skills/feedback and skills/update, while scanning the cowork copy that does not ship. A phantom path appended to README.md passed the entire suite. The list is now derived from package.json's files plus README.md, and every reference on a line is checked rather than the first. The watchdog paper-over, three ways (Fable P2-b, Codex P1): a per-paragraph assertion can only prove one paragraph is honest. Codex slipped "a built-in thirty-minute stall watchdog" past a digit-based regex; Fable pasted the verbatim original sentence into the parallel codex callout, and re-added STALL_SECONDS to the shipped skill. All three passed. Replaced with a surface-wide pin: across every packed markdown file, STALL_SECONDS is forbidden outright and any 'watchdog' mention must be a denial or a history note. This is a regression pin on two tokens that have already shipped as lies, not the #495(a) arms race of guessing unknown future spellings. All four constructed paper-overs now fail. The phantom merge mandate (Codex P1): the previous commit replaced scripts/merge-pr.sh with "your repo's merge wrapper" — still unconditional, still a tool the wizard installs for nobody. That is the same rename-instead- of-fix this branch exists to correct. Now names a concrete non-wrapper path and makes the wrapper conditional. Package-manager installs (Codex P1): the nudge omitted dnf and apk, and its test accepted a regression to a bare `claude update` — wrong for every package-managed install, which claude update reports as already current. Both halves are now asserted. Fable accepted the one disputed finding, so no dispute is outstanding. skills/sdlc/SKILL.md 19,993 bytes, twins identical. Suite 65/65. * fix(tests): round-3 review — sentence-level judging, and a guard I'd skipped Both reviewers independently landed on the same root cause: the #491 doc guards judged whole lines, and the canonical paragraph is one physical line. Any affirmative claim appended to it inherited that line's denial credit. Fable replaced "Nothing bounds a hung review except you noticing and killing it" with "The harness supplies a built-in thirty-minute stall watchdog, so every review is bounded" and the suite passed 115/0. The same looseness ran the other way. A README sentence about configuring systemd's watchdog was blocked as though it promised the Codex wrapper mechanism, and a legitimate history note — "v1.95 removed the stall watchdog claim" — was blocked for lacking an allowlist word. A guard that blocks honest future prose is its own defect, not extra safety. Judging is now per sentence and per clause, and only sentences actually about this review wrapper are in scope. All four blocking mutations fail; both must-not-fire cases pass. Per-clause qualification forced three real doc rewrites: my own "maintainer tooling, NOT installed by the wizard" qualifiers sat across a semicolon or em-dash from the path they were supposed to qualify, so by the rule I had just written they qualified nothing. Fable also caught a guard I had skipped. _packed_markdown_surfaces feeds both #491 doc tests, and when it returns nothing both pass vacuously — the exact failure mode I had guarded the hook roster against in the very commit that shipped this helper unguarded. It now asserts sentinel membership rather than mere non-emptiness, so a partial derivation is caught too. For the blocking-command-prefix hole the test does not simulate a shell — that invites quoting bugs in the test itself. It asserts the property that makes the simpler execution valid: every hooks.json command must be a bare script path. If that stops holding, it fails and says the harness needs extending first. Suite 65/65. SKILL.md 19,993 bytes, twins identical. * fix(tests): round-4 review — markdown weld, and a genuine reviewer conflict Both reviewers independently found the same bypass, from different angles. The sentence splitter required punctuation followed by whitespace, and the canonical paragraph ends `...on your behalf.**`. Markdown emphasis therefore welded the next sentence onto it, so an appended "The wrapper has a thirty-minute stall watchdog." inherited the earlier sentence's denial credit and passed 115/0. The same hole let `**scripts/audit-session-load.sh;**` shield a later consumer instruction. Emphasis markers are formatting, not meaning, so they are stripped before splitting and the boundary now tolerates trailing markup. The interesting part of this round was a real disagreement between the two reviewers, not a defect. Fable wanted bare `review` restored to the watchdog scope so "Reviews are bounded by a built-in watchdog." would be caught, and noted it would cost nothing. It would have: Codex's own must-not-fire case is "Our deployment REVIEW confirms the systemd service has a watchdog for stalled daemons." Each reviewer was right about its own case and neither could see the other's, because they run blind to each other by design. Resolved by scoping on the claim rather than the topic word — `reviews? (are|is) bounded`, `bounds (every|each|all) reviews?` — so both hold at once. Taking either recommendation verbatim would have broken the other. Two smaller ones from Fable, both mine: Extensionless scripts/ refs were tagged MISSING and then filtered away, so they received no qualifier check at all — a coverage regression I introduced in round 3 while fixing something else. Every reference is now checked regardless of extension. And the two guards now split text differently, deliberately. Watchdog judging is sentence-level, because a semicolon joins related independent clauses and "includes a stall watchdog; that claim was false" is one honest statement that clause-splitting severed. Script-qualifier judging stays clause-level, because a qualifier must be local to the reference it qualifies. Different questions, different granularity. Codex also ruled on the round-3 deviation: asserting every manifest command is a bare script path is "an acceptable equivalent to executing the full command under this harness; it is not a dodge." Mutation matrix now spans all four rounds: 5 must-fire cases all fire, 4 must-not-fire cases all stay silent. Suite 65/65.
This was referenced Aug 8, 2026
BaseInfinity
added a commit
that referenced
this pull request
Aug 8, 2026
…hipping (#506) The repo is now BaseInfinity/claude-sdlc-harness. The npm package stays agentic-sdlc-wizard and the plugin IDs, CLI bin and slash commands are unchanged — those are what an install depends on, and renaming them would force every consumer to reinstall for no functional gain. The old repo URL still redirects, and npm Trusted Publishing has been re-pointed at the new slug, which GitHub's redirect does not cover. Carries #495 (--user-approved), #497 (three shipped defects, and five review rounds fixing the tests meant to catch them), #500 (dual-slug CI tolerance, landed before the rename so the rename could not break it), #503 (the rename) and #505 (the Cowork gate fix and the Fable-decides contract). Version markers: eleven had to move together, and the suite caught three I missed — the cowork marketplace entry, the update skill's example, and the SDLC.md table. That is GH #493's defect class caught by tests rather than by review, which has not happened before. Historical v1.94.0 references in ROADMAP and in two test comments are left alone deliberately: they describe what was true then. Suite 65/65.
BaseInfinity
added a commit
that referenced
this pull request
Aug 12, 2026
…osed to catch them v1.94.0 was meant to remove every unbounded stdin read from the shipped hooks. It missed one. `hooks/model-effort-check.sh` still drained stdin with a bare `cat > /dev/null` — the same read-to-EOF that was observed alive at 10h19m against a 10-second timeout — while its SessionStart twin `token-spike-check.sh` was converted correctly. The reason it escaped is the interesting part. `tests/test-hook-stdin-bounded.sh` already tested this behaviourally, but its hook roster was hand-listed and `model-effort-check.sh` was never in it. A hand-maintained roster stops covering whatever nobody remembered to add, and reports success either way. The roster is now derived from `hooks/hooks.json` (6 -> 8 hooks) with a guard that fails if it ever stops matching the manifest, and a rostered file that does not resolve now fails instead of being silently skipped. Two more defects that shipped: `hooks/instructions-loaded-check.sh` told every consumer, on every session start with an update available, to run a global npm install of Claude Code. GH #476 read as "we omit native-install guidance"; in fact we shipped the opposite advice, from a hook that cannot detect which channel installed the running binary. The nudge is now channel-neutral. The docs promised a 30-minute stall watchdog governed by a variable that has never existed in this repo. `scripts/codex-review-with-progress.sh` loops on `kill -0` and enforces no timeout at all. The first fix here was to repoint the sentence at the wrapper that does exist — which ROADMAP.md had explicitly warned against, because renaming satisfies a path check while leaving the promise just as false, and harder to spot. Reverted; the claim is now replaced with what the wrapper actually does. Both reviewers then declined to certify, and the tests are the reason. A source-text regex for `cat` missed `$(cat 2>/dev/null)`, `/bin/cat`, `cat <&0`, `mapfile`, `IFS= read` without `-t` and more, while false-positiving on the same text inside a comment. A one-item variable denylist was escapable by renaming the phantom or dropping its name. Both are the unwinnable denylist shape ROADMAP #495(a) describes, so both are deleted rather than widened: the stdin guarantee is behavioural, and the watchdog guarantee is a positive assertion on the paragraph that defines it. Codex also found the defect one level out: `scripts/` is absent from package.json's `files`, so `npm pack` ships none of it, and five shipped references pointed consumers at tools they never receive — including `skills/sdlc/SKILL.md` mandating `scripts/merge-pr.sh`. The path test had blessed all five by asking whether they exist on the maintainer's disk. It now asks whether the consumer gets them. skills/sdlc/SKILL.md 19,988 -> 19,984 bytes, still byte-identical to its cowork twin. Suite 65/65.
BaseInfinity
added a commit
that referenced
this pull request
Aug 12, 2026
…nstructed Neither reviewer certified round 1, and both then broke round 2's fixes as well. Everything here is a defect one of them demonstrated, not a defect either of them merely suspected. Hook roster (Codex P1): the manifest extractor matched only flat hooks/<name>.sh, so a registered hooks/nested/hang.sh was reported as covered while never being executed — and the coverage guard repeated the same blind spot, so the two agreed on an answer that was wrong. Both now walk the manifest's actual command fields at any depth. Codex's exact mutation now fails. CI breakage (Fable P2-a, demonstrated): the new positive npm test ran the real hook, whose nudge branch is gated on `command -v claude`. ubuntu-latest has no Claude Code, so the test passed here and would have turned the required `validate` check red on push. claude and npm are now stubbed into an isolated PATH, which also pins the version instead of depending on whatever is installed locally. Shipped-doc coverage (Codex P1): the script-reference guard hand-picked its doc list, omitting README.md — which npm packs implicitly — plus skills/feedback and skills/update, while scanning the cowork copy that does not ship. A phantom path appended to README.md passed the entire suite. The list is now derived from package.json's files plus README.md, and every reference on a line is checked rather than the first. The watchdog paper-over, three ways (Fable P2-b, Codex P1): a per-paragraph assertion can only prove one paragraph is honest. Codex slipped "a built-in thirty-minute stall watchdog" past a digit-based regex; Fable pasted the verbatim original sentence into the parallel codex callout, and re-added STALL_SECONDS to the shipped skill. All three passed. Replaced with a surface-wide pin: across every packed markdown file, STALL_SECONDS is forbidden outright and any 'watchdog' mention must be a denial or a history note. This is a regression pin on two tokens that have already shipped as lies, not the #495(a) arms race of guessing unknown future spellings. All four constructed paper-overs now fail. The phantom merge mandate (Codex P1): the previous commit replaced scripts/merge-pr.sh with "your repo's merge wrapper" — still unconditional, still a tool the wizard installs for nobody. That is the same rename-instead- of-fix this branch exists to correct. Now names a concrete non-wrapper path and makes the wrapper conditional. Package-manager installs (Codex P1): the nudge omitted dnf and apk, and its test accepted a regression to a bare `claude update` — wrong for every package-managed install, which claude update reports as already current. Both halves are now asserted. Fable accepted the one disputed finding, so no dispute is outstanding. skills/sdlc/SKILL.md 19,993 bytes, twins identical. Suite 65/65.
BaseInfinity
added a commit
that referenced
this pull request
Aug 12, 2026
…hipping The repo is now BaseInfinity/claude-sdlc-harness. The npm package stays agentic-sdlc-wizard and the plugin IDs, CLI bin and slash commands are unchanged — those are what an install depends on, and renaming them would force every consumer to reinstall for no functional gain. The old repo URL still redirects, and npm Trusted Publishing has been re-pointed at the new slug, which GitHub's redirect does not cover. Carries #495 (--user-approved), #497 (three shipped defects, and five review rounds fixing the tests meant to catch them), #500 (dual-slug CI tolerance, landed before the rename so the rename could not break it), #503 (the rename) and #505 (the Cowork gate fix and the Fable-decides contract). Version markers: eleven had to move together, and the suite caught three I missed — the cowork marketplace entry, the update skill's example, and the SDLC.md table. That is GH #493's defect class caught by tests rather than by review, which has not happened before. Historical v1.94.0 references in ROADMAP and in two test comments are left alone deliberately: they describe what was true then. Suite 65/65.
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.
What
./scripts/merge-pr.sh <PR#> --user-approved "<reason>"HARD_DENY printed "a human decides this one" and gave the human no way to say so. Three times in one session the only route left was clicking merge in the GitHub UI — which bypasses the hook harness and leaves no record in the repo. PR #479 was merged exactly that way.
Waives process gates: both denylist tiers, the version-bump confirmation, the clearance-artifact checks.
Cannot waive facts: a red CI
validate, or a PR that net-removes tests. Both reviewers attacked that split and could not defeat it.Posts a durable override record to the PR — reason plus every waived path — before merging, and refuses to merge if that post fails.
It does not prove a human typed it. Nothing available to a local script can. It makes an override explicit and durable instead of silent.
Four defects found while building it
--user-approved --cross-model-clearedconsumed the next flag as the reason — a full override justified by the string--cross-model-cleared. Whitespace reasons passed too. Found by probing, not by my tests.clearance CERTIFIED round>=2 fresh, denylist clear— a false audit record when the override skipped both.Also: #486, three low-risk edits
Fable and Codex cross-examined each other and both concluded
SKILL.mdshould not split per model. These are the agreed items.The #486 regression test was itself vacuous — it grepped for the wording the fix deletes. Now positively anchored and verified against the reviewer's own mutation.
Status
REVIEWED, not CERTIFIED. Both reviewers returned NOT_CERTIFIED round 1; every finding is fixed, but neither has seen the fixes.
SKILL.mdbyte-neutral at 19,988/20,000, cowork byte-identical. Suite 65/65, merge gate 60/60.Closes #479. Part of #486.