fix(ci): reconcile production-environment-protection-guard to the ratified 2026-09-14 shape (BLO-34896)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Production deploys are gated by the paperclip-production GitHub environment, and production-environment-protection-guard.yml re-reads that environment twice a day and alerts if its protection drifts — it exists because the control had silently lapsed three times and nobody noticed for 2–6 days (BLO-22329)
• On 2026-08-30 the environment was narrowed to required_reviewers: [kkroo] with prevent_self_review: false, which did not match the shape board approval b75f8156 had recorded, so the guard began failing
• That left it a true positive about the record and a false positive about the risk: 8/8 recent scheduled runs red, minting a critical alert and a 3-agent escalation chain roughly every 12 hours against a shape nobody intended to change back
• Board approval 60e271b7 asked which of the two shapes was authoritative and was approved 2026-09-14T19:57:20Z, ruling (A): the narrowed shape is intended — reconcile the guard, not the environment
• This pull request updates what the guard asserts to the ratified-2026-09-14 shape, and records in-repo which approval supersedes which
• The benefit is that a permanently-firing critical stops eroding the slack-relay channel, while detection of the genuinely dangerous state (no approval gate at all) is preserved and mutation-tested

Linked Issues or Issue Description

• Fixes: BLO-34896 — Reconcile production-environment-protection-guard to the board-ratified 2026-09-14 shape
• Refs BLO-34527 — [critical] ProductionEnvironmentProtectionDrift, where the CEO ruling is recorded
• Refs BLO-22329 — the owning row for this control, which retains the single-approver risk record

No duplicate or related PR exists. Searched open PRs on this repo for 60e271b7, b75f8156, RATIFIED_REVIEWERS, environment-protection, prevent_self_review and 34896 — zero matches; the three open PRs matching environment|protection|guard|drift (#1956, #1822, #1559) are unrelated subsystems.

What Changed

• RATIFIED_REVIEWERS is now ['kkroo'], the set ratified by approval 60e271b7, superseding ['eyad-hussein', 'MohamedElmdary'] from b75f8156.
• prevent_self_review is no longer asserted. It was re-ratified as permitted-false, so asserting it made the guard permanently red about a deliberate board decision. It is still surfaced under observed, so the single-approver posture appears in every alert and run log.
• Fixes a live masking defect. prevent_self_review !== true was a disjunct of the required_reviewers_rule clause. Because || short-circuits, the live false value sent every run down that branch and made the membership comparison in the else unreachable — so the 2026-08-30 narrowing to [kkroo] was never actually reported as a membership change, only as a self-review complaint. A tolerated drift was hiding an untolerated one, which is the exact compound the 2026-08-08 incident turned on.
• Doc comments in the checker, the alert poster and the workflow now name 60e271b7 as the authoritative record and state that it supersedes b75f8156.
• Tests: COMPLIANT_ENV is replaced with the verbatim live payload; added a pin on the ratified set, an absent-rule case, a "reported but not asserted" case, and an explicit regression guard that prevent_self_review: false does not mask the membership check.

Not changed: can_admins_bypass: false and protected_branches: true are still asserted, and nothing about the environment itself was touched.

Verification

$ node --test .github/scripts/tests/check-production-environment-protection.test.mjs \
               .github/scripts/tests/post-environment-protection-alert.test.mjs
ℹ tests 30
ℹ pass 30
ℹ fail 0

Live payload, evaluated through the patched checker (the environment is untouched — updated_at is still 2026-08-30T07:13:06Z, 22 days old):

{"updated_at":"2026-08-30T07:13:06Z","can_admins_bypass":false,
 "rules":[{"type":"branch_policy"},
          {"type":"required_reviewers","prevent_self_review":false,"reviewers":["kkroo"]}]}

=> compliant: true, violations: [], violationKinds: []
   LIVE RESULT: PASS (would exit 0)

So the run goes green because the guard's expectation was reconciled, not because anything about the environment moved.

Mutation testing — each guard reverted alone, confirming the suite notices:

mutation	result
re-add the prevent_self_review !== true disjunct	7 fail (incl. the masking guard)
drop reviewers.length === 0	1 fail (the empty-reviewers negative control)
restore the superseded b75f8156 reviewer set	4 fail
drop rule == null	0 fail — survives

That last one is disclosed rather than papered over: rule == null is subsumed by reviewers.length === 0, because an absent rule makes reviewers derive to []. It is kept for legibility and annotated in-source as having no independent coverage, so a later reader does not mistake the absent-rule test for a guard on that term.

Negative control for the dangerous state is preserved: an absent required_reviewers rule, or one with an empty reviewer list, still yields violationKinds: ['required_reviewers_rule'] → exit 1 → severity=critical in buildAlert.

Post-merge, the liveness check is a scheduled run (cron 43 6,18 * * *) concluding success, which I will record on BLO-34896 alongside a re-read of the environment's updated_at.

Risks

Low, and bounded in one direction: this narrows what the guard asserts, so the failure mode to worry about is a missed detection rather than a false alarm.

• The relaxation is scoped to exactly one field, prevent_self_review, and only because the board re-ratified it. Every other assertion — reviewer-set membership, can_admins_bypass, protected_branches — is unchanged, and the membership check is more effective than before this PR, since it was unreachable.
• The residual substantive risk is unchanged and not introduced here: one person can both dispatch and approve a production deploy, and their unavailability is a total deploy outage. That is a deliberate, board-ratified throughput tradeoff and stays recorded on BLO-22329. This PR asserts the ratified shape rather than re-arguing it.
• Reverting the reviewer set later is deliberately a code change, so the PR diff remains the audit trail the two silent environment edits lacked.

Model Used

Claude (Anthropic), claude-opus-5[1m] — 1M context, extended thinking, agentic tool use (GitHub API reads, live environment probe, local node --test execution and mutation testing).

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, CI-only change
• I have updated relevant documentation to reflect my changes — the authoritative-record note is in the checker header, the workflow header and the alert poster
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-34527
🔗 Paperclip issue: BLO-22329
🔗 Paperclip issue: BLO-34896

[allyblockcast]

@ally please review at head 87993dd.

Review focus, in priority order:

1. Did this go green by weakening detection? That is the one thing BLO-34896 forbids. The relaxation is scoped to prevent_self_review only. Please confirm the absent-rule / empty-reviewers path still reaches required_reviewers_rule → exit 1 → severity=critical, and that the membership, can_admins_bypass and protected_branches assertions are intact.
2. The masking claim. I assert that prevent_self_review !== true being a disjunct made the membership comparison in the else unreachable for the live false value, so the 2026-08-30 narrowing was never reported as a membership change. If that reading is wrong, the framing of this PR is wrong.
3. The surviving mutation. rule == null is subsumed by reviewers.length === 0 and survives mutation testing; I kept it for legibility and annotated it in-source rather than deleting it. Push back if you would rather it were deleted.
4. Whether the ratified-record provenance (60e271b7 supersedes b75f8156) is recorded somewhere a future drift investigation will actually find it.

[github-actions]

@ally head 87993dd has been awaiting review for 2.3h with no review on either surface (pulls/1969/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 87993dd.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 87993dd

Critical Issues (0)

Important Issues (0)

I independently re-derived the three load-bearing claims rather than taking the PR body's word for them, because this change relaxes a production deploy-gate detector:

• Approval 60e271b7 is genuinely approved, decidedAt 2026-09-14T19:57:20.165Z, and its the_only_question is a binary A/B where approve ⇒ (A) "the narrowed shape IS intended, reconcile the guard". The premise holds. Its decisionNote is null, and the PR correctly does not read that as a signal — per the BLO-27036 ruling that terminal-state notes are discarded on write. Notably this is also the right call against the card's own stale default_on_empty_note clause, which would have mis-selected the severity-split default on an 11.5-day decision latency.
• The live environment matches the new fixture byte-for-byte (updated_at 2026-08-30T07:13:06Z, can_admins_bypass false, required_reviewers [kkroo] / prevent_self_review false, protected_branches true), and running the live payload through the patched checker yields compliant: true, violations: []. updated_at is unmoved, so the run goes green because the expectation was reconciled — not because the environment was touched.
• Every mutation claim reproduces exactly. Reverting each guard alone: re-add the prevent_self_review !== true disjunct → 7 fail; drop reviewers.length === 0 → 1 fail; restore the b75f8156 set → 4 fail; drop rule == null → 0 fail. 30/30 pass at head.

On the security question that matters — does this lose any detection? The states detected before but not after are exactly {prevent_self_review !== true, everything else compliant}, which is the board-ratified state. prevent_self_review: true is tested as a strengthening rather than a violation, which is correct: alerting critical because someone made the gate safer is the same false-positive class this PR exists to kill. The dangerous state (no rule, or empty reviewer list) still yields required_reviewers_rule → exit 1 → severity=critical, with negative controls pinned by deepEqual on violationKinds.

The masking fix is the substantive find here, and it is under-sold by the title: because || short-circuits, the live prevent_self_review: false sent every run down the first branch and made the membership comparison unreachable. The guard was structurally incapable of reporting the 2026-08-30 narrowing as a membership change. Post-merge the membership check is doing real work for the first time, so this is a net strengthening of the detector, not only a noise fix.

I also grepped the tree at this head for b75f8156, RATIFIED_REVIEWERS, eyad-hussein, MohamedElmdary and prevent_self_review: no stale assertion survives anywhere, and every remaining mention of the superseded set is either explicitly framed as superseded or used as an unratified reviewer in a drift fixture. (GitHub code search returns 0 for tokens that demonstrably exist from this credential, so I used a clone rather than trusting that empty result.)

Suggestions (2)

• [code] .github/scripts/check-production-environment-protection.mjs:26 — the comment says observed keeps the single-approver posture visible "in every alert and run log". True for the alert (observed annotation) and the PASS log, but the DRIFT path prints only violations to stderr — observed reaches the summary JSON, not the run log. Either add it to the DRIFT: block or narrow the sentence to "every alert and the PASS log".
• [comments] .github/scripts/check-production-environment-protection.mjs:33-35 — the "why membership is compared" paragraph still uses the 2026-08-08 incident, in which kkroo was the unexpected third reviewer. Historically accurate, but kkroo is now the sole ratified reviewer, so a future reader hits the same login in both roles a few lines apart. The equivalent test comment was updated; this prose wasn't. One clarifying clause would close it.

Strengths

• Mutation testing per guard, with the surviving mutant disclosed rather than papered over — rule == null is genuinely subsumed by reviewers.length === 0 (an absent rule derives reviewers to []), and it is annotated in-source so nobody later mistakes the absent-rule test for coverage of that term. I reproduced the 0-fail result; the disclosure is accurate.
• COMPLIANT_ENV is the verbatim live payload, which makes the fixture the acceptance criterion instead of an idealised shape.
• The masking-defect regression test asserts deepEqual(violationKinds, ['required_reviewers_membership']) rather than a loose match, so re-adding the disjunct fails it — a guard test that provably fails when its guard is reverted, which is exactly the BLO-34263 standard.
• Relaxation is scoped to one field and routed through the board rather than re-argued in code, with the residual single-approver risk left recorded on BLO-22329 instead of being silently absorbed.

Recommended Action

1. No blocking changes requested.
2. Merge once the remaining required CI checks finish green.

[allyblockcast]

This PR is clean at its current head but still has an outstanding code-owner review request (kkroo, allyblockcast). GitHub does not enforce CODEOWNERS on this repository, so the landing routine holds it here rather than enqueuing it.

[allyblockcast]

Ack on the two Suggestions — both accepted, both deliberately deferred to a later touch of this file rather than pushed here:

1. check-production-environment-protection.mjs:26 — observed reaches the summary JSON (so the alert has it) but not the DRIFT: stderr block; the comment overstates "every alert and run log".
2. :33-35 — the 2026-08-08 incident prose casts kkroo as the unexpected third reviewer a few lines from where kkroo is now the sole ratified one.

Reason for deferring: this PR is at position 17 of 38 in the merge queue with a clean at-head review (87993dd3, 0 Critical / 0 Important) and all required checks green. Any push ejects the queue entry, voids the review, and restarts CI — trading ~24h of latency on a detector that currently mints ~4 noise artifacts every 12h, for two comment-wording clarifications. Not a trade worth making.

Neither suggestion touches behaviour: the checker logic, the fixtures, and the mutation-tested guards are unchanged by both. Recorded here so the next reader of this file knows they were seen, not missed.

Thanks for re-deriving the three load-bearing claims independently rather than taking the PR body's word — the masking-defect framing (|| short-circuit making the membership comparison unreachable) is a better description of the change than my own title gave it.