fix(cloud): PAT-first PR tracking, and block agent launch on checkout - #5379
Open
Hendrixx-RE wants to merge 2 commits into
Open
Hendrixx-RE wants to merge 2 commits into
Hendrixx-RE wants to merge 2 commits into
Conversation
Two independent bugs: 1. workerRaisePullRequest and workerClaimPullRequest gated the whole handler on s.checkoutBroker == nil before ever trying the configured PAT, unlike their sibling workerCheckoutGrant/workerPushGrant (which correctly try the PAT first). On any deployment without a GitHub App configured, PR raise/claim always failed with SCM_BROKER_UNAVAILABLE even with a fully valid PAT wired up. Reorder both handlers to match the PAT-first pattern already used for checkout/push grants. 2. ao-worker's prepareWorkspace (repository checkout) and startInteractiveAgent (the coding-agent process) ran in independent, unsynchronized goroutines. The transport supervisor's HoldAgentInputUntilWorkspaceReady only holds terminal input for turns after the first — the session's initial prompt is baked directly into the agent's launch argv (BuildInteractive), so it was never subject to that hold. A failed or slow checkout let the agent start its first turn against an empty workspace with no signal anywhere that checkout had failed (only a worker-side log line). startInteractiveAgent now blocks on the checkout goroutine's result before building/launching the agent command.
… string The pull-request bridge's push-grant and raise-pull-request failure branches logged the real error (which already carries the control plane's actual HTTP status and response body, via client.doMethod's "%s returned %d: %s" wrap) but returned an identical generic string to the agent regardless of cause. Every failure mode -- broker unavailable, GitHub API rejection, auth failure, a genuine outage -- looked exactly like every other one from inside the sandbox, making it impossible to tell from a worker transcript alone whether a given 502 was the SCM_BROKER_UNAVAILABLE bug, a GitHub-side rejection, or something else. Include err.Error() in the response, matching the push-failure branch which already did this.
Hendrixx-RE
marked this pull request as draft
September 14, 2026 16:19
Hendrixx-RE
marked this pull request as ready for review
September 14, 2026 17:47
5 tasks
i-trytoohard
added
bug
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
2 participants
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
Two confirmed, separate bugs found while diagnosing cloud worker failures reported against this deployment:
1. PR tracking failed with
SCM_BROKER_UNAVAILABLEeven with a valid PAT configuredworkerRaisePullRequestandworkerClaimPullRequest(cloud/internal/httpapi/worker_handlers.go) gated the entire handler ons.checkoutBroker == nilbefore ever attempting the configured user PAT — unlike their siblingworkerCheckoutGrant/workerPushGrant, which correctly try the PAT first and only fall back to (or fail without) the broker. Their own inline comments ("PAT-first, mirroring...") described behavior the code didn't actually implement. On a deployment with no GitHub App configured (checkoutBrokerstaysnil), PR raise/claim — and by extensionao claim-pr, auto-invoked by the worker'sgh pr createwrapper — always failed with a 503, regardless of PAT. Reordered both handlers to match the working PAT-first pattern.2. Fresh worker sandboxes could start their first turn against an empty, uncloned workspace
ao-worker/main.goranprepareWorkspace(git checkout) andstartInteractiveAgent(the coding-agent process) in independent, unsynchronized goroutines.HoldAgentInputUntilWorkspaceReadywas meant to prevent this, but it only holds terminal input for turns after the first — the session's initial prompt is baked directly into the agent's launch command (BuildInteractive→agentruntime.BuildLaunchCommand), so it never passed through that hold. A failed or slow checkout let the agent boot and start working in an empty workspace, with the checkout failure logged only worker-side and invisible to the session/user. Reproduced twice against this deployment.startInteractiveAgentnow blocks on the checkout goroutine's result before building or launching the agent command.Test plan
go build ./...(cloud module)go vet ./...(cloud module)go test ./...(cloud module) — all packages passworker_pat_write_test.goreproduce the exact reported bug: PAT works withcheckoutBroker == nil(previously would 503); no-PAT/no-broker fails closed withSCM_BROKER_UNAVAILABLEinstead of panicking, for both raise and claim