chore(helm): activate comment-shaped Ally review gate in observe-only mode (BLO-26602) - #1346
Merged
allyblockcast[bot] merged 1 commit intoAug 15, 2026
Conversation
… mode (BLO-26602) Set githubApp.prCommentReviewGateStatusContext to "review/ally-comment" in the Blockcast production values so the server begins publishing the comment-shaped review gate status. This does not block any merge: the status only gates once the same context is marked required on master branch protection, which is a human-only step (agent tokens get 403 there). Sequencing is deliberate. GitHub's required-checks picker only offers status contexts it has observed recently, and this repo has never carried a commit status of any kind, so the context must be posted at least once before a human can select it. Deliberately does NOT set prReviewGateStatusContext. Its only writer posts state=failure on reviewer-chain exhaustion; nothing in the server, and no workflow or script in this repo, ever posts success for it. Requiring it here would strand every healthy PR at "Expected - waiting for status". Refs BLO-26602, BLO-21907, BLO-17456
Author
Author
|
@ally please review at head c9ee47f — helm: activates the comment-shaped Ally review gate in observe-only mode. Focus on confirming observe-only cannot block a merge. Context: the original review request on this PR was lost during the codex provider outage (BLO-27123) — codex |
Author
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: c9ee47f
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The deployment-only change opts in the comment-shaped status context without configuring the separate formal-review status context.
- The chart wiring passes the value as an environment variable; the gate implementation remains a status publisher and does not modify branch protection or merge settings.
- The comments clearly document that making the context required is a separate follow-up operation.
Recommended Action
- Safe to merge from the reviewed change surface.
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
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.
Thinking Path
Linked Issues or Issue Description
Refs #1262
Refs BLO-26602 (activation), BLO-21907 (comment-shaped gate), BLO-17456 (formal-review gate)
What Changed
githubApp.prCommentReviewGateStatusContext: "review/ally-comment"indeploy/helm/paperclip/values.blockcast.yaml, naming it consistently with the existingreview/ally-*convention used in the runbooks and shipped skills catalog.prReviewGateStatusContextis deliberately left unset here (see Risks).This PR does not make anything blocking. The status only gates merges once the same context is marked required on
masterbranch protection, which agent tokens cannot do (403 Resource not accessible by integration).Why this must precede the branch-protection change
GitHub's required-status-checks picker only offers contexts it has observed on a recent commit.
Blockcast/papercliphas never carried a legacy commit status — I checked the eight most recent open PR heads and all report<no statuses>. So the context has to be posted at least once before a human can select it in repo settings. Setting the value first is a prerequisite, not a shortcut.Verification
Chart renders and lints locally against the production values:
Confirmed the key parses under
githubApp(not reparented into a sibling block) and thatPAPERCLIP_PR_REVIEW_GATE_STATUS_CONTEXTis correctly absent from the render.Post-deploy, the observable signal is a
review/ally-commentstatus appearing on PR heads —successwhen no unresolved Critical/Important comment finding attests to the current head,failurewhen one does.Risks
Low. The status is advisory until a human marks it required; a non-required status does not block a merge or merge-queue entry.
One thing this PR deliberately does not do, and why it matters. BLO-26602 asked to bundle
prReviewGateStatusContext(the formal-review gate) and mark both contexts required. Setting the value is harmless, but marking that one required on this repo would wedge every PR, so I did not set it up for that:queueFailedPrReviewGateStatus, which postsstate: "failure"on reviewer-chain exhaustion or ambiguous lifecycle end.successfor it.github-status-delivery-outbox.tsskips delivery when a success already exists — i.e. it assumes an external writer owns the green.onprem-k8sthat external writer exists (.github/scripts/require-ally-review.py). This repo has no equivalent, and no workflow here posts commit statuses at all.So a healthy PR would simply never receive that context, and GitHub renders a required-but-never-posted context as "Expected — waiting for status" indefinitely. That is the exact wedge the
values.yamlcomment warns about. The comment-shaped gate does not have this problem: it posts bothsuccessandfailureon every evaluated head.Remaining risk to confirm at rollout: the App installation needs
statuses: writeon this repo. It has never posted a status here, so that permission is unproven empirically — if it is missing, the gate logs a post failure and stays inert rather than breaking anything.Model Used
Claude Opus 4.5 (
claude-opus-4-5), extended thinking, tool-assisted repository analysis and local Helm render/lint verification.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template