Skip to content

fix(desktop): inline actionable clone errors in onboarding project step - #6116

Merged
Kitenite merged 2 commits into
mainfrom
hoakiet98/super-1729-clone-inline-errors
Aug 2, 2026
Merged

Kitenite merged 2 commits into
mainfrom
hoakiet98/super-1729-clone-inline-errors

Conversation

@Kitenite

@Kitenite Kitenite commented Aug 1, 2026 •

Copy link
Copy Markdown
Member

Part of SUPER-1729 — https://linear.app/superset-sh/issue/SUPER-1729

Onboarding step 2's "Clone a repo" card used to surface clone failures as a sonner toast of raw git stderr that fades in ~4s, with no hint that private repos 404 when GitHub CLI isn't signed in, and no retry affordance.

Changes

  • Clone failures now render as a persistent inline error under the clone input, cleared on the next submit or input edit. Error text is select-text cursor-text per the renderer's selectable-error rule.
  • stderr matching "Repository not found", "Authentication failed", "could not read Username", or "Permission denied (publickey)" maps to "Couldn't access this repository — if it's private, sign in to GitHub CLI first." plus a Sign in to GitHub CLI button that opens the existing GhAuthDialog on this step; the entered URL stays intact for an immediate retry. Unmatched errors show their raw message inline instead of a toast.
  • The "host service isn't ready" early-return also moved from toast to the same inline slot.
  • Clone input placeholder changed from the SSH example to https://github.com/org/repo.git (SSH keys are an unchecked prerequisite).

Other cards (create, open folder, template), step 1, GhAuthDialog internals, and Continue/navigation logic are untouched.

Verification

  • bun run lint:fix + bun run lint exit 0
  • apps/desktop bun run typecheck passes

Summary by CodeRabbit

  • New Features

    • Added GitHub authentication support during project cloning.
    • Added an authentication dialog for eligible clone failures.
    • Updated the clone URL placeholder to use HTTPS.
  • Bug Fixes

    • Improved handling and messaging for authentication-related clone errors.
    • Displayed local-host readiness and clone failures inline.
    • Cleared previous errors when the clone input changes.

Screenshot evidence (CDP-verified)

Before (SUPER-1729 walkthrough): raw git stderr in a toast that auto-dismissed in ~4s —

Before: vanishing toast with raw git stderr

After:

Behavior Screenshot
Persistent inline error with cause + sign-in action, URL preserved
Still present 12s later (no auto-dismiss, zero toasts)
Sign in to GitHub CLI opens the dialog right on step 2
URL + error intact after dialog close, ready to retry
Error clears on input edit; HTTPS placeholder

Images live on the evidence/super-1729 branch (never merge; deletable after the PRs land).

Replace the auto-dismissing clone-failure toast with a persistent inline
error under the clone input. Auth-shaped git failures (repo not found,
auth failed, no username, publickey denied) map to a human cause with a
Sign in to GitHub CLI button that opens GhAuthDialog in place; other
errors render their raw message inline and selectable. Also switch the
placeholder to an HTTPS example since SSH keys are unchecked.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: af11f7de-52c4-4c0a-bdcf-d0814db47dfc

📥 Commits

Reviewing files that changed from the base of the PR and between 662ef50 and 5f32e8d.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx

📝 Walkthrough

Walkthrough

The project onboarding page now handles GitHub CLI authentication during cloning. It stores structured clone errors, displays failures inline, clears errors during retries or URL edits, and opens GhAuthDialog for authentication-related failures.

Changes

Project clone authentication

Layer / File(s) Summary
Clone error detection and normalization
apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx
Clone exceptions are converted into structured errors. Recognized GitHub authentication failures set an authentication-required flag.
Clone error state and input handling
apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx
Clone attempts clear previous errors. Local-host readiness failures and URL edits update inline clone-error state. The URL placeholder uses HTTPS.
Inline errors and GitHub CLI dialog
apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx
The page renders clone errors inline and shows a GitHub CLI sign-in button for eligible failures. GhAuthDialog is mounted and wired to page state.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • superset-sh/superset#6117: Integrates the GitHub authentication dialog and CLI authentication flow used by project cloning.

Suggested reviewers: saddlepaddle

Sequence Diagram(s)

sequenceDiagram
  participant ProjectPage
  participant CloneOperation
  participant GhAuthDialog
  ProjectPage->>CloneOperation: Start project clone
  CloneOperation-->>ProjectPage: Return clone failure
  ProjectPage->>ProjectPage: Normalize and store clone error
  ProjectPage->>GhAuthDialog: Open dialog for authentication-required failure
  GhAuthDialog-->>ProjectPage: Return dialog state or exit
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description explains the problem, changes, testing, and UI evidence; the checklist headings are omitted but the relevant items are covered.
Title check ✅ Passed The title clearly summarizes the main change: inline actionable clone errors in the desktop onboarding project step.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hoakiet98/super-1729-clone-inline-errors

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Kitenite

Kitenite commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

End-to-end CDP verification (real dev app, worktree renderer on vite port 8105, CDP 9613, matched page target before driving):

  • Placeholder: clone input shows https://github.com/org/repo.git — /tmp/super-1729-t3-verify/01-clone-card-placeholder.png, full step 2 in 01b-full-step2.png.
  • Failure path (real clone of a private/nonexistent repo via native value setter + form.requestSubmit()): inline error rendered in the card — "Couldn't access this repository — if it's private, sign in to GitHub CLI first." with the Sign in to GitHub CLI button, zero sonner toasts, URL preserved in the input. Computed style on the error node: user-select: text, cursor: text while body is user-select: none. 02-inline-error.png; still present 12s later (DOM re-checked + 03-inline-error-after-12s.png) — no auto-dismiss.
  • Sign-in action: button opens GhAuthDialog right on step 2 with the live gh terminal prompt (04-gh-auth-dialog.png). Did not drive the auth flow (machine gh already authenticated). After closing the dialog, the entered URL and the inline error are both intact for an immediate retry (05-after-dialog-close-url-intact.png).
  • Error lifecycle: editing the input clears the error immediately (06-error-cleared-on-edit.png); resubmit also clears it first (code path exercised by the submit above).

All checks were end-to-end through the real renderer (onboarding gate armed by nulling onboarded_at, then restored); screenshots captured via Page.captureScreenshot.

@Kitenite
Kitenite marked this pull request as ready for review August 2, 2026 00:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx (1)

30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the configured renderer path alias for GhAuthDialog.

The new import uses a relative path while this file uses renderer/... aliases for local modules. Use the configured alias to keep the import stable if the route directory moves.

Proposed import
-import { GhAuthDialog } from "../components/GhAuthDialog";
+import { GhAuthDialog } from "renderer/routes/_authenticated/onboarding/components/GhAuthDialog";

As per coding guidelines, use the path aliases defined in tsconfig.json when possible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx`
at line 30, Update the GhAuthDialog import in the onboarding project page to use
the configured renderer path alias instead of the relative path, matching the
existing local-module import convention.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx`:
- Around line 41-57: Scope the GitHub CLI authentication classification in
toCloneError to failures from the GitHub clone operation only, rather than
applying message-pattern matching to the outer onboarding catch that also
handles readiness, project creation, setup, and navigation. Track the clone
operation and GitHub host context, or use typed provider metadata, so unrelated
errors—including host-service messages containing authentication or repository
text—cannot set needsGhAuth; add regression coverage for non-GitHub and
non-clone failures.
- Around line 250-254: Update the cloneError message container in the onboarding
page to expose the asynchronous failure through an assertive live region, such
as role="alert". Keep the existing cloneError.message rendering and styling
unchanged.

---

Nitpick comments:
In `@apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx`:
- Line 30: Update the GhAuthDialog import in the onboarding project page to use
the configured renderer path alias instead of the relative path, matching the
existing local-module import convention.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f9448b63-9a0b-4ffc-ae09-909c9140ffbe

📥 Commits

Reviewing files that changed from the base of the PR and between f0be47c and 662ef50.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

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="apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx:41">
P2: When an SSH URL clone fails with "Permission denied (publickey)", the new inline error tells the user to sign in to GitHub CLI and shows the corresponding button. But that failure means the SSH key isn't authorized for the host — the PR even states SSH keys are an unchecked prerequisite and switched the placeholder to HTTPS to steer away from SSH. Signing in to `gh` does not fix an unauthorized SSH key, so the user would click "Sign in to GitHub CLI", complete the flow, retry, and hit the same error. Consider dropping "Permission denied (publickey)" from the GH-auth pattern set (or special-casing it with an SSH-specific message, e.g. add your SSH key to the host), rather than routing it through the GH CLI sign-in affordance.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant UI as Clone Form UI
    participant State as React State (url, cloneError, ghAuthOpen)
    participant CloneHandler as Handle Clone Submit
    participant HostService as Local Host Service
    participant ProjectAPI as V2 Cloud Project API
    participant GhAuth as GhAuthDialog

    Note over UI,GhAuth: Clone Repository Flow (Onboarding Step 2)

    UI->>UI: User enters HTTPS URL
    UI->>State: setUrl(url)
    UI->>UI: User clicks "Clone" button
    UI->>CloneHandler: handleClone()

    CloneHandler->>State: setCloneError(null), setBusy(true)

    alt Host service not ready
        CloneHandler->>State: setCloneError({message, needsGhAuth:false})
        CloneHandler->>State: setBusy(false)
        State-->>UI: Inline error displayed
    else Host service ready
        CloneHandler->>HostService: waitForHostReady()
        HostService-->>CloneHandler: activeHostUrl
        CloneHandler->>ProjectAPI: createProject(url)
        alt Clone succeeds
            ProjectAPI-->>CloneHandler: projectId
            CloneHandler->>UI: finish() - navigate away
        else Clone fails
            ProjectAPI-->>CloneHandler: Error with stderr message
            CloneHandler-->>CloneHandler: toCloneError(message)
            alt Auth-related failure (GH_AUTH_FAILURE_PATTERNS)
                CloneHandler->>State: setCloneError({message:"Couldn't access...", needsGhAuth:true})
                State-->>UI: Inline error + "Sign in to GitHub CLI" button
                UI->>UI: User clicks "Sign in"
                UI->>State: setGhAuthOpen(true)
                UI->>GhAuth: Open dialog
                GhAuth-->>UI: Auth flow completed
                UI->>State: setGhAuthOpen(false)
                UI->>UI: User retries clone (same URL)
            else Other failure
                CloneHandler->>State: setCloneError({message:raw stderr, needsGhAuth:false})
                State-->>UI: Inline error (raw message)
            end
        end
    end

    Note over UI: User edits URL - clear error
    UI->>State: setUrl(newValue)
    alt cloneError exists
        State->>State: setCloneError(null)
        State-->>UI: Error removed
    end
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment on lines +41 to +46
const GH_AUTH_FAILURE_PATTERNS = [
"Repository not found",
"Authentication failed",
"could not read Username",
"Permission denied (publickey)",
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When an SSH URL clone fails with "Permission denied (publickey)", the new inline error tells the user to sign in to GitHub CLI and shows the corresponding button. But that failure means the SSH key isn't authorized for the host — the PR even states SSH keys are an unchecked prerequisite and switched the placeholder to HTTPS to steer away from SSH. Signing in to gh does not fix an unauthorized SSH key, so the user would click "Sign in to GitHub CLI", complete the flow, retry, and hit the same error. Consider dropping "Permission denied (publickey)" from the GH-auth pattern set (or special-casing it with an SSH-specific message, e.g. add your SSH key to the host), rather than routing it through the GH CLI sign-in affordance.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/onboarding/project/page.tsx, line 41:

<comment>When an SSH URL clone fails with "Permission denied (publickey)", the new inline error tells the user to sign in to GitHub CLI and shows the corresponding button. But that failure means the SSH key isn't authorized for the host — the PR even states SSH keys are an unchecked prerequisite and switched the placeholder to HTTPS to steer away from SSH. Signing in to `gh` does not fix an unauthorized SSH key, so the user would click "Sign in to GitHub CLI", complete the flow, retry, and hit the same error. Consider dropping "Permission denied (publickey)" from the GH-auth pattern set (or special-casing it with an SSH-specific message, e.g. add your SSH key to the host), rather than routing it through the GH CLI sign-in affordance.</comment>

<file context>
@@ -27,11 +27,35 @@ import { EmptyProjectModal } from "renderer/routes/_authenticated/components/Emp
+	needsGhAuth: boolean;
+}
+
+const GH_AUTH_FAILURE_PATTERNS = [
+	"Repository not found",
+	"Authentication failed",
</file context>
Suggested change
const GH_AUTH_FAILURE_PATTERNS = [
"Repository not found",
"Authentication failed",
"could not read Username",
"Permission denied (publickey)",
];
const GH_AUTH_FAILURE_PATTERNS = [
"Repository not found",
"Authentication failed",
"could not read Username",
];

…le=alert

Address review: map GitHub CLI advice only for errors thrown by the
clone mutation itself (non-clone failures show their raw message with
no gh action), give Permission denied (publickey) SSH-specific copy
pointing at the HTTPS URL, and announce the inline error via role=alert.
@Kitenite
Kitenite merged commit ba021c9 into main Aug 2, 2026
10 of 11 checks passed
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch

Thank you for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant