fix(deploy): automatically connect GitHub for one-click deployment - #344
Conversation
|
@cursoragent review this |
|
Reviewed PR #344. The host readiness gate and popup-blocker fallback are sound, but automatic GitHub connect does not run on the one-click path. The new The new tests render Ask: move auto-authorize to   |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
| // Closing or blocking the popup must not start another authorization loop. | ||
| autoAuthorizeAttemptedRef.current = true; | ||
| onAutoAuthorize(); |
There was a problem hiding this comment.
Review
The host-side readiness gate and the popup-blocker fallback are the right shape for this problem. Automatic auth is withheld until workspace credentials and the connection lookup are ready, and a blocked window.open keeps the manual Connect GitHub button instead of navigating the embedded Brain frame to GitHub.
The feature still does not run on the one-click path.
Blocking: auto-authorize is mounted only after GitHub is already connected
onAutoAuthorize is invoked from an effect inside GithubDeployerUrlInput. That is the inverse of the unauthorized state this PR is trying to handle.
Production always renders GithubDeployer.Shell, not UrlInput as a Root child:
<GithubDeployer.Root
actions={githubDeployer?.actions}
autoDeploy={githubAutoDeploy}
initialRepoUrl={initialGithubRepoUrl}
key={`${resetKey}:${initialGithubRepoUrl ?? ""}:${githubAutoDeploy ? "auto" : "manual"}`}
states={githubDeployer?.states ?? EMPTY_GITHUB_DEPLOYER_STATES}
>
<GithubDeployer.Shell />
</GithubDeployer.Root>Shell only mounts UrlInput once authorized:
function GithubDeployerShell({ className, ...props }: ComponentProps<"div">) {
const {
states: { deployedRepo, isAuthorized },
} = useGithubDeployer();
const showGithubAccount = !deployedRepo;
const showRepositoryInput = isAuthorized && !deployedRepo;
// ...
{showRepositoryInput ? (
<DeploymentSettings.Section
description="Deploy from a repository URL."
icon={<Link2 aria-hidden className="size-4" />}
title="Repository URL"
>
<DeploymentSettings.Control>
<GithubDeployerUrlInput />
</DeploymentSettings.Control>
</DeploymentSettings.Section>
) : null}A signed-in user with no GitHub Connection therefore never mounts the effect. The existing auto-deploy effect can stay on UrlInput because it only runs after authorization. Auto-authorize has to live on Root or Shell, which stay mounted on the Import pane.
The new tests render <GithubDeployer.UrlInput /> under Root and miss this. The same file already proves the production tree does not include URL input while unauthorized (GithubDeployer asks for authorization before showing URL input). After moving the effect, cover it through Shell (or the githubDirect creation pane): unauthorized + valid auto-deploy URL + ready onAutoAuthorize should fire once; closing the popup must not loop; autoDeploy: false / invalid URL must not fire.
Non-blocking
initiateGithubAuth({ automatic: true })skippingwindow.location.assignwhen the popup is blocked is the correct embedded-frame behavior.- Ordinary manual Connect GitHub is unchanged, and ADR-0036 / ADR-0056 / ADR-0057 still hold: this is still the per-actor OAuth connect with
prompt=select_account. - Residual product risk after the mount fix: browsers will still block
window.openfrom an effect. The documented fallback to the Connect GitHub button is acceptable; live OAuth on the 189 cluster is still needed to confirm Desktop does not swallow the popup (related: fix(desktop): preserve GitHub deployment through sign-in sealos#7318).
| } | ||
| // Closing or blocking the popup must not start another authorization loop. | ||
| autoAuthorizeAttemptedRef.current = true; | ||
| onAutoAuthorize(); |
There was a problem hiding this comment.
This effect never runs on the one-click GitHub Import pane.
GithubDeployer.Shell only mounts UrlInput when isAuthorized && !deployedRepo. Production always uses Shell (project-creation-pane.tsx, project-creator.stage.tsx, github-deployment-pane.tsx). While the user still needs GitHub connected, UrlInput is not in the tree, so onAutoAuthorize is never called.
The existing auto-deploy effect can stay here because it requires isAuthorized. Auto-authorize is the opposite condition and belongs on Root or Shell.
Please add a Shell-level (or githubDirect pane) test: unauthorized + valid auto-deploy URL + ready onAutoAuthorize must fire once. The new tests render UrlInput directly and cannot catch this — the same file already asserts URL input is absent from Shell while unauthorized.
ReviewThe host-side readiness gate and the popup-blocker fallback are the right shape for this problem. Automatic auth is withheld until workspace credentials and the connection lookup are ready, and a blocked The feature still does not run on the one-click path. Blocking: auto-authorize is mounted only after GitHub is already connected
Production always renders Shell only mounts The existing auto-deploy effect can stay on The new tests render Non-blocking
|
| repos: [], | ||
| }} | ||
| > | ||
| <GithubDeployer.UrlInput /> |
There was a problem hiding this comment.
These cases mount UrlInput as a Root child, which is not the production tree. Shell does not render UrlInput when isAuthorized is false, so a passing unauthorized auto-connect case here does not exercise one-click import.
Please drive auto-authorize through GithubDeployer.Shell (or ProjectCreationPane with entryMode="githubDirect"). The existing test “asks for authorization before showing URL input” already documents that contract.
|
@cursoragent review this again. The mount issue is fixed in 1528712: automatic authorization now runs in Root, and regression tests exercise the production Shell tree. Please verify that authorization only starts for valid one-click deployment links (autoDeploy=1) when GitHub is not connected, and never for ordinary Brain or manual GitHub Import entry. |
|
❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues. Please try again later or Open in Web to view the status. Learn more about Cursor Agents |
|
@cursoragent review this |
|
Re-reviewed PR #344 at Auto-auth only starts for a valid one-click link ( No remaining blocking issues on that rule. Optional nit: |
Re-review (
|
| Gate | Result |
|---|---|
Valid one-click link (autoDeploy=1 + canonical https://github.com/owner/repo) |
Required. /deploy only sets autoDeploy: true when the query flag is exactly "1" and the repo survives normalizeGithubRepoUrl. |
| GitHub not connected | Required. isAuthorized short-circuits; after connect, the existing auto-deploy effect in UrlInput takes over. |
| Ordinary Brain / New Project | Does not start. onNewProject opens entryMode: "general". GithubPanel does not pass autoDeploy (defaults false) or initialRepoUrl (empty → normalizeGithubRepoUrl is null). |
| Manual GitHub Import | Does not start. /deploy?githubRepo=… without autoDeploy=1, and the assistant github intent (githubDirect with no repo and no autoDeploy), both leave autoDeploy false. |
| In-project GitHub pane / chat AGUI | Do not start. No autoDeploy, and github-deployment-pane does not wire onAutoAuthorize. |
Root still withholds until onAutoAuthorize is provided (host: credentials + connection lookup ready, no error), !isLoading, !deployedRepo, and a one-shot ref so a closed/blocked popup does not loop. initiateGithubAuth({ automatic: true }) still skips window.location.assign when window.open is blocked.
GithubPanel still receives onAutoAuthorize because it spreads the shared githubDeployer.actions. That is not a leak today: Root also requires autoDeploy and a valid restored URL, which that panel never supplies. Optional defense in depth would be to attach onAutoAuthorize only on the githubDirect auto-deploy host.
Residual (unchanged): browsers may still block the effect-driven popup; the Connect GitHub button remains the fallback. Live OAuth on 189 is still the missing confirmation.
No further blocking issues on the mount or the autoDeploy=1-only rule.
2 participants
A signed-in user following a GitHub one-click deployment link stops at the import pane when their GitHub connection is missing. Automatically initiate the existing Connect GitHub flow once credentials and the connection lookup are ready, then continue deploying the original repository after authorization.
Automatic authorization runs in the always-mounted Root and only for valid auto-deploy links. Each mounted flow attempts it once. If the browser blocks the popup, retain the manual Connect GitHub button instead of navigating the embedded frame to GitHub. Ordinary manual entry remains unchanged.
Preserve the encoded return path across OAuth session creation, completion, and opener notification. Previously each validation decoded the nested pane URL again; a later pass rejected the decoded repository URL and fell back to
/, losing the repository and auto-deploy intent. Validate the relative destination without rewriting query encoding, while continuing to reject external redirects and unsafe path forms.Validation:
bun typecheckandbun checkpassed.Related Desktop login-intent fix: labring/sealos#7318