fix: stop the public-review provider gate from blocking every ordinary agent task - #5866
Merged
Conversation
…y agent task #5830 collapsed the two per-stage public-review provider checks into one and dropped the `publicReview &&` guard along with them. The surviving gate ran for every spawn, and an ordinary task carries no execution profile — so its posture is `null`, no vendor declares a recipe for `null`, and the check answered "unsupported". Every normal agent task (the release-check scheduled task included) was blocked at spawn with "Provider 'codex-tui' has no enforced null public-content review mode". Move the decision into `publicReviewProviderBlock(provider, posture)`, which returns null when no posture was requested and otherwise owns the reason text and blocked category. Taking the posture rather than a boolean keeps the "nothing to enforce" case explicit, so it cannot be dropped by a caller-side refactor again.
…its category The gate fix added a second copy of the `id || command || 'unknown'` provider label to providerVendors.js; extract `providerLabel()` so both error strings read from one place. In the caller, destructure `category` alongside `reason` rather than reaching back into `postureBlock` for it — the helper owns both fields, and the guard test now pins the destructure instead of the field access.
atomantic
added a commit
that referenced
this pull request
Sep 2, 2026
The guards that existed when #5866 landed could not have caught it: agentLifecycle.test.js reads the orchestrator as a string, and a dropped `publicReview &&` is invisible to a grep — as is the same block reintroduced unconditionally tomorrow, which is exactly what a reviewer flagged about the first cut of this fix. Drive spawnAgentForTask with the leaves stubbed instead: an ordinary task must not be blocked, and a public-review stage on a provider with no maintained recipe must still fail closed. Verified against the broken revision — the two ordinary-task cases fail there and pass here, while the fail-closed case passes on both, so the test cannot be satisfied by simply deleting the gate.
atomantic
added a commit
that referenced
this pull request
Sep 2, 2026
…gory #5866 left mislabeled Two follow-ups to #5866, which fixed the outage itself. The gate's guards are source greps: they pin that the call site names `publicReviewProviderBlock` with exactly those arguments. That catches an edit TO that line, but not a second, unconditional posture assertion added beside it — which blocks every ordinary task again. Verified: with such a block added and the pinned call left verbatim, agentLifecycle.test.js passes 61/61 while every agent on the install is blanked. The new spawn-level test fails there, and its fail-closed case still passes, so it cannot be satisfied by deleting the gate instead. validatePublicReviewModel takes a `posture` but hardcoded the no-tool category, so a sandboxed-actions failure reported `public-review-provider-unsupported` for every provider. It now reuses the same descriptor the gate does. Latent today — the one caller passes the no-tool literal — but it fails closed with a plausible-looking wrong code, which is harder to diagnose than the outage was.
atomantic
added a commit
that referenced
this pull request
Sep 2, 2026
…gory #5866 left mislabeled Two follow-ups to #5866, which fixed the outage itself. The gate's guards are source greps: they pin that the call site names `publicReviewProviderBlock` with exactly those arguments. That catches an edit TO that line, but not a second, unconditional posture assertion added beside it — which blocks every ordinary task again. Verified: with such a block added and the pinned call left verbatim, agentLifecycle.test.js passes 61/61 while every agent on the install is blanked. The new spawn-level test fails there, and its fail-closed case still passes, so it cannot be satisfied by deleting the gate instead. validatePublicReviewModel takes a `posture` but hardcoded the no-tool category, so a sandboxed-actions failure reported `public-review-provider-unsupported` for every provider. It now reuses the same descriptor the gate does. Latent today — the one caller passes the no-tool literal — but it fails closed with a plausible-looking wrong code, which is harder to diagnose than the outage was.
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
Every ordinary agent task — the release-check scheduled task included — was blocked at spawn with
Provider 'codex-tui' has no enforced null public-content review mode.#5830 collapsed the two per-stage public-review provider checks into one and dropped the
publicReview &&guard along with them. The surviving gate ran for every spawn, and an ordinary task carries no execution profile, so its posture isnull. No vendor declares a recipe fornull, so the check answered "unsupported" and blocked the task.The decision now lives in
publicReviewProviderBlock(provider, posture), which returnsnullwhen no posture was requested and otherwise owns both the reason text and the blocked category. Taking the posture rather than a boolean keeps the "nothing to enforce" case explicit, so a caller-side refactor cannot drop it again the way #5830 did.The gate is not weakened: for any non-null posture the new path is equivalent to the old
!supportsPublicReviewPosture(...)check, and it still fails closed for an unsupported provider or a TUI transport. The category derivation moved frompublicReviewActions(a profile comparison) toposture === PUBLIC_REVIEW_ACTIONS_POSTURE, which is equivalent because only thepublic-review-actionsprofile maps to that posture.Test plan
server/lib/providerVendors.publicReview.test.js— new cases pin that a task requesting no posture is never blocked (the regression), and that a requested-but-unsupported posture still blocks with the right reason and category, including the TUI transport case.server/services/agentLifecycle.test.js— guard pins the caller to the helper so the gate cannot re-derive the decision from a boolean support check and reintroduce the collapse.