Skip to content

fix: name both Git grants when the platform cannot tell them apart - #1374

Draft
l2ysho wants to merge 1 commit into
masterfrom
claude/cli-git-grant-ambiguity-9d41c7
Draft

fix: name both Git grants when the platform cannot tell them apart#1374
l2ysho wants to merge 1 commit into
masterfrom
claude/cli-git-grant-ambiguity-9d41c7

Conversation

@l2ysho

@l2ysho l2ysho commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Important

TL;DRapify create --source github in some scenarios I got stuck in auth flow, this needs further investigation


Stacked on #1371 — it depends on the GitAccount parameter that PR adds to getGitConnectUrl. Retarget to master once #1371 merges.

Not the redirect bug in #1371. This one hits personal accounts too.

The state the CLI has to read

apify-core, src/api/src/routes/integrations/git_providers.ts, reports four situations in three shapes:

situation integration workspaces addWorkspaceUrl grant needed
not authorized absent authorize
authorized, app installed present filled present none
authorized, app installed nowhere present [] absent install
provider token revoked present [] absent authorize

addWorkspaceUrl comes from data.installations[0]?.app_slug, so it is absent when there are no installations — and absent again in the catch when the installation listing fails, which is what a revoked token produces. The last two rows are byte-identical on the wire. No heuristic separates them.

The old code always chose the installation link for both, so a revoked token was handed a link that changes nothing, and the poll loop waited out its full budget on a state that could never move.

What changed

src/lib/git-source/gitSource.ts

  • getPendingGrants() returns the grants that may still be missing, likeliest first. No integration means authorization, unambiguously. An integration carrying addWorkspaceUrl means the listing succeeded, so the token works and only the installation is left. Anything else is ambiguous and names both.
  • createGrantTracker() holds the state that spans polls: the last URL opened, and whether this run watched an authorization land. A run that offered authorization while no integration existed knows the integration it then sees is that grant landing. Offering authorization in the ambiguous state proves nothing and does not count — treating it as proof reorders the grants on the very next poll and opens a second tab on top of a user still sitting on the consent page.
  • The wait loop opens the first grant and prints the rest, so an ambiguous state always leaves the other link on screen. It never opens the same URL twice, and resets the deadline and backoff when a new URL is opened, so each browser step gets the whole budget rather than the previous step's leftovers.
  • The wait line follows the grant, so it no longer says "Waiting for authorization" while the pending step is the installation.
  • The noWorkspace recovery steps name both grants, for the same reason the wait loop does.

Reviewer notes

  • Splitting the decision out of the polling is what makes it testable. createGrantTracker is driven directly over a poll sequence, with no mocking of open, fetch or timers. Reverting the tie-break fails five of those tests.
  • --json gitConnectUrl still carries a single URL for noWorkspace. The same payload includes nextSteps, which names both, and a single-URL field cannot represent an ambiguity the API itself cannot resolve.
  • Worth fixing in apify-core, which would let this logic be deleted: the catch in getGithubIntegration turns a revoked token into a shape meaning "authorized, nothing installed". Returning null on a 401 collapses that into "not authorized". There is also no github_app_authorization webhook handler, so isAuthorized is never cleared when a user revokes on GitHub.

Verification

Reproduced live by revoking the Apify authorization on GitHub, which leaves GET /v2/integrations/git reporting workspaces: [] and no addWorkspaceUrl — the state that used to hang. The CLI now opens the authorization URL, the grant lands, and workspace selection proceeds.

Checks

lint, format, build, test:local (503 passed, 4 skipped). No docs regen — no flag, arg or description changed.

🤖 Generated with Claude Code

@l2ysho
l2ysho requested a review from DaveHanns as a code owner September 1, 2026 09:05
@l2ysho
l2ysho marked this pull request as draft September 1, 2026 09:08
Base automatically changed from claude/cli-github-auth-stuck-47be9b to master September 1, 2026 10:21
`GET /v2/integrations/git` reports an authorized user with no app
installation exactly like one whose token the provider revoked: the
integration is present, `workspaces` is empty, and `addWorkspaceUrl`
is absent because it is derived from the first installation. Only one
of the two is fixed by installing the app, so picking either one dead
ends the other. The old code always picked the installation, which
left a revoked token polling a state that could never change.

`getPendingGrants` now returns both, likeliest first, and the wait
loop opens the first and prints the rest. A run that watched the user
authorize knows the integration it then sees is that grant landing,
which settles authorization and leaves only the installation. That
only counts when no integration existed at the time — offering
authorization in the ambiguous state says nothing about whether the
user completed it.

`createGrantTracker` holds the across-poll state, so the decision is
testable without mocking the browser, the API or the clock.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@l2ysho
l2ysho force-pushed the claude/cli-git-grant-ambiguity-9d41c7 branch from 738efd9 to 0d13335 Compare September 1, 2026 10:23
@github-actions github-actions Bot added this to the 148th sprint - Tooling team milestone Sep 1, 2026
@github-actions github-actions Bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants