fix(dashboard): repair and polish the GitHub link-existing project flow#1441
Open
BilalG1 wants to merge 19 commits into
Open
fix(dashboard): repair and polish the GitHub link-existing project flow#1441BilalG1 wants to merge 19 commits into
BilalG1 wants to merge 19 commits into
Conversation
- Generated workflow now passes the required --cloud-project-id flag (sourced from the STACK_PROJECT_ID secret), which was previously missing and never read — every workflow run failed. - workflow_dispatch is now best-effort: it 404s when the workflow is not on the default branch, but the workflow-file commit already triggers a run via the push paths filter, so the flow continues. - Config paths are normalized (leading ./ stripped) so the workflow's push paths filter actually matches ongoing config edits. - The github-repository step now shows a Connect button when no GitHub account is connected, instead of a dead-end alert. - "Connect new" uses linkConnectedAccount so it can actually add an account, rather than getOrLinkConnectedAccount which just returns the existing one. - Repositories load via an effect when the step has a selected account, fixing the empty repo list after a connect redirect or page reload. - Local CLI command shown to users uses --cloud-project-id, matching the actual CLI flag.
The generated GitHub Actions workflow ran the CLI via `pnpx`, but the ubuntu-latest runner has Node/npx but no pnpm, so the step failed with `pnpx: command not found` (exit 127). - Run the CLI with `npx --yes` and add an actions/setup-node step to pin Node on the runner. - Update the local CLI command shown to users to `npx` as well, since `pnpx` is not universally available.
- Add an npx/pnpx/bunx package-runner toggle (npx default) so users can pick the runner that matches their setup. - Split the single command block into separate "Sign in" and "Push config" snippets so users who already ran login can copy just the push command. - Move --config-file to the end of the push command so the whole command up to the placeholder is easy to copy. - Reuse the shared CodeBlock component (built-in copy button) instead of a bare <pre> for consistency.
Remove the "skip this if already signed in" and "this pushes the config for project ..." helper lines for a cleaner page.
`config push` and `config pull` no longer require --cloud-project-id; when omitted, the project id is read from the STACK_PROJECT_ID environment variable via a new resolveProjectId helper. Empty option strings are treated as absent. The generated GitHub Actions workflow already exports STACK_PROJECT_ID as a step env var, so the explicit --cloud-project-id flag is dropped from the run command.
…opdown The connected-account selector on the 'Choose repository and branch' step rendered with the numeric providerAccountId until the GitHub /user fetch populated githubAccountLogins. Replace the dropdown with a small Spinner + 'Loading GitHub account...' row while the selected account's login is unknown, then show the dropdown once available.
- New RemoteSearchCombobox (Popover + cmdk pattern already used in dashboard data-tables) drives both selectors. - Repository selector: type-ahead with debounced /search/repositories fetch so users with more than 100 repos can find any of them, not just the first /user/repos page. - Branch selector: type-ahead with debounced /git/matching-refs/heads prefix search (the branches endpoint itself has no query support). - Drop the Branch "Refresh" button — branches already auto-load on repository select, and the combobox can refresh by reopening.
…us update The startStatusTransition wrap around a single Map insert into projectStatuses wasn't deferring anything meaningful, and the [, startStatusTransition] destructure with an unused first slot was noise. Inline the setState call and drop the useTransition import.
- RemoteSearchCombobox derives the trigger label internally from items + value (falling back to the value string) instead of taking a selectedLabel prop, so call sites don't have to thread it. - loadRepositories now uses a runId guard (matching the existing pollingRunIdRef / localMonitoringRunIdRef pattern) so a stale call can't clobber state set by a newer one. The repo auto-load effect's catch only resets the loaded-account ref when it still matches the failed account, for the same reason. - Drop a defensive try/catch around parseRepositoryFullName in the branch-search effect; selectedRepository is already null-guarded.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors link-existing onboarding with a RemoteSearchCombobox, guarded async repo/branch remote search, workflow YAML normalization and Node v20 setup, non-fatal workflow dispatch, runner-aware CLI command rendering, and adds resolveProjectId to make the CLI project-id flag optional. ChangesDashboard Onboarding Refactor
CLI Project ID Resolution
Sequence DiagramsequenceDiagram
participant GitHubRepositoryStep
participant loadRepositories
participant GitHubAPI
participant workflowDispatch
GitHubRepositoryStep->>loadRepositories: enter step -> start run-id
loadRepositories->>GitHubAPI: fetch /user and repos
GitHubAPI-->>loadRepositories: return user/repos
loadRepositories->>GitHubRepositoryStep: set repositories (only if run-id current)
GitHubRepositoryStep->>workflowDispatch: dispatch workflow (try/catch)
workflowDispatch-->>GitHubRepositoryStep: log dispatch error, continue monitoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR repairs the end-to-end Link Existing Config onboarding flow in the dashboard and the
Confidence Score: 4/5Safe to merge with minor follow-up. The critical workflow and CLI bugs are properly fixed, and the new combobox and useEffect-driven repo loading are logically sound. The changes are well-reasoned and cover the stated bugs. Two small gaps exist: project.id is interpolated unescaped into the displayed bash command (against the team's escaping convention), and the config-path button guard does not account for paths like "./" that pass the trim-length check but normalize to an empty string inside buildWorkflowYaml — producing a broken workflow file if the user somehow arrives there. link-existing-onboarding.tsx (configPushCommand escaping and config-path empty-after-normalization guard), link-existing-onboarding-workflow.ts (normalizeConfigPath edge case downstream) Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (Dashboard)
participant GH as GitHub OAuth
participant API as GitHub API
participant WF as GitHub Actions
participant CLI as stack-cli
U->>GH: linkConnectedAccount("github")
GH-->>U: OAuth redirect back
Note over U: useEffect detects selectedGithubAccount
U->>API: GET /user (githubFetch)
U->>API: GET /user/repos
U->>U: Select repo + branch (RemoteSearchCombobox)
Note over U: Debounced /search/repositories on query change
Note over U: Debounced /git/matching-refs/heads/{prefix} on branch query
U->>U: Enter config path (normalizeConfigPath strips ./)
U->>API: Commit workflow YAML via Contents API
U->>API: POST workflow_dispatch (best-effort, 404 non-fatal)
WF->>CLI: "npx --yes @stackframe/stack-cli@latest config push"
Note over CLI: resolveProjectId reads STACK_PROJECT_ID env var
CLI-->>U: Config pushed → step advances to github-logs
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.ts (1)
10-15: ⚡ Quick winConsider adding defensive validation for empty results.
The function correctly normalizes paths, but edge cases like
"./"or whitespace-only input would produce an empty string. An empty config path in the generated workflow would be invalid.🛡️ Proposed defensive check
export function normalizeConfigPath(configPath: string): string { - return configPath.trim().replace(/^(?:\.\/)+/, ""); + const normalized = configPath.trim().replace(/^(?:\.\/)+/, ""); + if (!normalized) { + throw new Error("Config path cannot be empty after normalization"); + } + return normalized; }As per coding guidelines: "Fail early, fail loud; fail fast with an error instead of silently continuing" and "Code defensively; prefer
?? throwErr(...)over non-null assertions, with good error messages explicitly stating the assumption that must've been violated".🤖 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/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.ts around lines 10 - 15, normalizeConfigPath currently returns an empty string for inputs like "./" or whitespace-only, which leads to invalid workflow config; after computing the normalized value in normalizeConfigPath (and/or before returning), defensively validate that the result is non-empty and if it is throw a clear error (e.g., throw new Error or use the project's throwErr helper) with a message stating the assumption violated ("configPath must resolve to a non-empty path after normalization"); reference the normalizeConfigPath function name and ensure callers either catch this or rely on the thrown error to fail fast.
🤖 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/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.ts:
- Around line 41-44: The GitHub Actions step named "Set up Node.js" currently
uses actions/setup-node@v4; update that usage to the latest major
(actions/setup-node@v6 or at least `@v6.4.0`) by replacing the version token in
the step (the "uses" value) so the workflow uses actions/setup-node@v6 while
preserving the existing node-version input ("20") and other step metadata.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.ts:
- Around line 10-15: normalizeConfigPath currently returns an empty string for
inputs like "./" or whitespace-only, which leads to invalid workflow config;
after computing the normalized value in normalizeConfigPath (and/or before
returning), defensively validate that the result is non-empty and if it is throw
a clear error (e.g., throw new Error or use the project's throwErr helper) with
a message stating the assumption violated ("configPath must resolve to a
non-empty path after normalization"); reference the normalizeConfigPath function
name and ensure callers either catch this or rely on the thrown error to fail
fast.
🪄 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
Run ID: d1ab8631-98db-4385-b31b-c1585677e322
📒 Files selected for processing (7)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/content.tsxapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-combobox.tsxapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.tsapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsxpackages/stack-cli/src/commands/config-file.tspackages/stack-cli/src/lib/auth.test.tspackages/stack-cli/src/lib/auth.ts
- Repo search now adds `user:<login>` to the /search/repositories query so results stay within the connected user's repos instead of returning global GitHub results - Inline rate-limit message in the repo and branch combobox when GitHub returns a 403/429, instead of firing a generic alert - Refresh icon button next to the branch combobox so users who create a branch on GitHub mid-flow can refetch without switching repos - Clearer log when workflow_dispatch fails because the workflow file is not yet on the default branch
…node}@v6 Matches the version used by every other workflow in this repo.
…I command Matches the team convention for interpolating values into CLI commands displayed for user copy-paste. Visually identical for current project ID formats, defensive against future changes.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsx (1)
363-381: 💤 Low valueInconsistent error handling compared to other parse functions.
parseGithubMatchingRefssilently returns[]for non-array input, while similar functions (parseGithubBranches,parseGithubRepositories) throw errors. Per coding guidelines, prefer failing loudly over silent fallbacks unless there's a documented reason. If this is intentional for resilience, consider adding a comment explaining why this function differs.🛡️ Suggested fix for consistency
function parseGithubMatchingRefs(value: unknown): string[] { if (!Array.isArray(value)) { - return []; + // matching-refs can return 404 for empty matches on some repos; callers + // handle errors upstream, so return empty here for resilience. + return []; }As per coding guidelines: "Fail early, fail loud. Fail fast with an error instead of silently continuing."
🤖 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/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsx around lines 363 - 381, The function parseGithubMatchingRefs currently returns [] when given non-array input; update it to fail loudly like parseGithubBranches and parseGithubRepositories by throwing a descriptive Error when value is not an array (e.g., "Expected array of refs"), preserving the rest of the logic that strips the "refs/heads/" prefix and returns branch names; alternatively, if the silent behavior is intentional, add a clear comment above parseGithubMatchingRefs explaining why it differs from the other parse functions.
🤖 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.
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsx:
- Around line 363-381: The function parseGithubMatchingRefs currently returns []
when given non-array input; update it to fail loudly like parseGithubBranches
and parseGithubRepositories by throwing a descriptive Error when value is not an
array (e.g., "Expected array of refs"), preserving the rest of the logic that
strips the "refs/heads/" prefix and returns branch names; alternatively, if the
silent behavior is intentional, add a clear comment above
parseGithubMatchingRefs explaining why it differs from the other parse
functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52d9ee02-e293-455c-8662-bd8b7a411310
📒 Files selected for processing (1)
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsx
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
parseGithubMatchingRefs was silently returning [] on non-array input, unlike every other parseGithub* helper in the file which throws. Match the established pattern so a malformed response surfaces instead of quietly producing an empty branch list.
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.
Rework of the new-project → Link Existing Config flow on the dashboard, plus the published
stack-cliit depends on.The starting point on
devhad the link-existing flow effectively broken end-to-end (the generated GitHub workflow could never authenticate, and the GitHub-account selection UI dead-ended in several states). This PR fixes the blockers, polishes the local-CLI path, and adds a searchable repo/branch picker.What was broken
--cloud-project-idflag → every run failed at Commander before the action ran.d0e6ad15f,55ff7e319STACK_PROJECT_IDenv var the CLI never read.55ff7e319(CLI now reads it; workflow drops the explicit flag)pnpxisn't onubuntu-latest→ step failed withcommand not found.65789a1acd0e6ad15fgetOrLinkConnectedAccount(get-or-link) → silently returned the existing account instead of starting a fresh OAuth flow.d0e6ad15fworkflow_dispatch404s on non-default branches; threw before advancing to the logs step even though the push-triggered run worked.d0e6ad15f./, which breaks GitHub'son.push.pathsfilter — ongoing config edits never re-triggered the workflow.d0e6ad15fproviderAccountIdbefore the GitHub/userfetch populated the username.de9ec19237550eaacbWhat changed
Dashboard — Link Existing Config flow
ed25eabf9,ebb090e5b): split into separate "Sign in" and "Push config" code blocks using the sharedCodeBlockcomponent (copy button built-in), added anpx / pnpx / bunxrunner pill toggle (defaultnpx), moved--config-file <path>to the end of the push command so users can copy everything up to the placeholder, trimmed redundant helper text.d0e6ad15f,de9ec1923): empty-state "Connect GitHub account" button; "Connect new" now useslinkConnectedAccountso it actually starts OAuth; loading row instead ofproviderAccountIdflash.7550eaacb,5ce1b6bd9): newRemoteSearchCombobox(Popover + cmdk, same pattern asdata-table/faceted-filter), debounced GitHub/search/repositoriesand/git/matching-refs/heads/{prefix}calls so users with > 100 repos/branches can find any of them. Branch "Refresh" button removed — branches auto-load on repo select.d0e6ad15f,65789a1ac): config paths normalised (strip leading./); workflow usesactions/setup-node@v4+npx --yes;workflow_dispatchfailure is now best-effort (the workflow-file commit's push event triggers the run on any branch).Stack CLI
STACK_PROJECT_IDenv-var fallback for--cloud-project-id(55ff7e319). Bothconfig pushandconfig pullare affected; explicit flag still wins. NewresolveProjectIdhelper inlib/auth.tswith 5 unit tests (auth.test.ts).Misc
2faffb662drops an unuseduseTransitionwrapper around asetProjectStatusesMap insert in the new-project flow.Release ordering note
The generated workflow's
run:line no longer passes--cloud-project-id— the CLI readsSTACK_PROJECT_IDfrom env instead. This means a workflow generated by this branch only works against a@stackframe/stack-clipublished with the env-var fallback from55ff7e319. The CLI and dashboard ship from the same monorepo so this should be a non-issue in the normal release cadence, but worth confirming the CLI publishes alongside the dashboard deploy.Existing workflows already committed in user repos still have the explicit flag and continue to work unchanged.
Validation
pnpm --filter @stackframe/dashboard run typecheck✅pnpm --filter @stackframe/dashboard run lint✅pnpm --filter @stackframe/stack-cli run typecheck✅pnpm --filter @stackframe/stack-cli run lint✅pnpm --filter @stackframe/stack-cli test✅ (14 tests; 5 new forresolveProjectId)Summary by CodeRabbit
New Features
Improvements
Tests