feat(github): guide App installation and repo access from CLI - #225
Conversation
![agent-zhang-beihai[bot]](https://avatars.githubusercontent.com/in/3908133?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed by Yang Dong
This adds guided browser flows for GitHub authorization, App installation, and repository access, then resumes connect-repo automatically. I’m requesting changes because the new installation wait can continue beyond its advertised 15-minute deadline.
The installation loop starts another unbounded request after its deadline
important · defect · correctness · src/commands/github.ts:181
The deadline is checked before sleeping, so the final iteration wakes at or after 15 minutes and still starts another repository request. That request receives no AbortSignal, meaning a slow response keeps the command alive beyond the promised timeout; use the existing remaining-time and bounded-request shape from deploy-archive.ts, checking the deadline after the wait and passing a signal capped by the remaining time.
Evidence
read-the-code — src/commands/github.ts:179-193, src/api.ts:61-97, src/deploy-archive.ts:129-143, test/github-connect.test.ts:224-230
There was a problem hiding this comment.
Reviewed by Wang Miao
This turns compute connect-repo's dead ends into a guided flow: it opens the GitHub device-authorization page, then either the owner's existing App installation settings or a fresh install URL, and polls /me/github/repos for up to 15 minutes until the target repo becomes reachable — while --json and non-TTY pipes keep failing immediately with actionable text. I traced the flow against the platform's MyGitHub schema and startSetup handler (both on instacloud-platform main, so installations and installUrl are really there), checked the one call site in computeConnectRepo, and confirmed the new open seam matches how auth.ts already drives openUrl (print the URL first, never trust the return value). Typecheck and all 1,110 tests pass on the head commit. I would merge it.
No findings.
|
Fixed the installation wait deadline in d0b83c1: the final sleep is capped by remaining time, the loop checks expiry again before starting a request, and the request carries an AbortSignal bounded by the remaining budget. Regression checks cover a final two-second wait, no request after expiry, and cancellation of a stalled request. Removing the sleep cap, post-wait guard or signal, or extending the deadline makes the tests fail. Validation on the updated head: typecheck/build and 1,111 tests pass; independent subtraction review is clean. Wang's approval stands. The only growth is this timeout correction and its regression checks. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Reviewed by Yang Dong
This makes connect-repo open the GitHub authorization and App configuration pages, then wait until the requested repository becomes accessible. The flow matches the platform contract, but the mandatory agent-facing command reference still says this CLI path does not exist, so I would not merge it as-is.
The new installation path leaves the agent-facing command contract saying it does not exist
important · judgement · conventions · src/index.ts:268
The repository explicitly requires command changes to be mirrored in skills/insta/cli-reference.md, yet its connect-repo entry still says “no CLI path installs the App” and directs agents to the console. Update that entry in the same change set to describe the device authorization, App installation/configuration, polling behavior, and the requirement that --json access already be ready.
Evidence
read-the-code — AGENTS.md:15-17, .claude/skills/developing-insta-cli/SKILL.md:36-38, src/index.ts:268-276, src/commands/github.ts:148-195, instacloud-skills/insta/cli-reference.md:87
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/github.ts">
<violation number="1" location="src/commands/github.ts:186">
P2: In agent mode, this signal does not bound the whole `api.request`: `agentHeaders` first mints `/agent/sessions` through a nested request without the signal. If that request stalls, the loop can exceed `stopAt`; propagate the remaining signal through agent-session creation or bound the complete operation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
The command reference is already updated in the companion skills PR #94, linked in this PR body before the review. Its current remote head is 842801369aa668b43db68166c485ff8062908d71: It describes device authorization, App installation/configuration, polling/automatic continuation, owner/admin access, and the ready-access requirement for --json; the quick reference is updated as well. Both changes belong to this change set and the docs PR explicitly lands with or after CLI 0.0.73. The older reference on skills main is the currently released 0.0.72 behavior, not the companion diff. No further code or document changes are needed for this finding. The prior deadline finding is fixed in d0b83c1. |
2 participants
compute connect-repocurrently stops when the GitHub App is missing or excludes the target repository. It now opens GitHub device authorization when needed, opens installation or the owning account's configuration page, and polls until the repository becomes accessible before continuing detection and connecting the service. URLs are always printed so remote terminals can relay them;--jsonand unattended pipes fail promptly when setup is incomplete. Existing access and--publickeep their direct paths.Uses the deployed user-scoped
/me/github/reposand/me/github/setupAPIs. No org installation claims or Platform API changes. Bumps the release version to 0.0.73. Deploy Console #451 before releasing sostate=clireturns to the informational terminal page. Documentation is in skills #94. The older #213 targets the retired org claim API; this implementation uses the current identity model.Validation: typecheck/build and all 1,111 tests pass. Six full-command scenarios against a local mock HTTP API verify authorization → installation → detection → source write, installation alone, configuration, existing access, JSON failure, and public bypass. Independent review and mutation tests cover browser opening, owner selection, polling, timeout and unattended guards. No live GitHub installation permissions were changed during these checks.