feat: link Stack Auth projects to GitHub and push config from the dashboard#1450
Open
BilalG1 wants to merge 32 commits into
Open
feat: link Stack Auth projects to GitHub and push config from the dashboard#1450BilalG1 wants to merge 32 commits into
BilalG1 wants to merge 32 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.
- 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.
…-flow # Conflicts: # apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsx
…yaml Inputs like "./" pass the upstream non-empty trim check but normalize to "" inside buildWorkflowYaml, which would emit `paths: [""]` and an empty STACK_AUTH_CONFIG_PATH env var. Fail fast at the boundary instead of committing a silently broken workflow to the user's 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.
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.
- Render private-repo indicator as a trailing lock icon on a single-line row instead of stacking a "private" subtitle. - Harden checkConfigPathExists against directory/symlink responses and reject `.`/`..` paths before hitting the API. - Treat 404 from `git/trees/<sha>` as "no paths yet" so freshly-initialized repos whose commit points at the empty-tree SHA (4b825dc…) no longer surface a fatal alert on the Select config file step.
Adds `--source github` with `--source-repo`, `--source-path`, and
`--source-workflow-path` (all required together) so the CLI can declare
its provenance explicitly instead of relying solely on `GITHUB_*` env
vars. `commit_hash` and `branch` are still read from `GITHUB_SHA` /
`GITHUB_REF_NAME`.
Adds `workflow_path` to the `pushed-from-github` source schema
(optional for backward compat with existing rows). The dashboard's
generated workflow YAML now emits the new flags and uses
`${{ github.repository }}` at runtime so the stored source reflects
renames/transfers. The project-settings UI surfaces the workflow file
as a clickable GitHub link.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR adds workflow path support to Stack Auth's GitHub-based config sources and implements end-to-end GitHub config push functionality. It extends the schema with an optional ChangesGitHub Config Sync and Push
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 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 |
…-path Previously these flags were only checked for `=== undefined`, so passing `--source-path ""` would store `config_file_path: ""` verbatim. Now we require a non-empty value, matching the existing `parseOwnerRepo` check for `--source-repo`. Adds tests covering both cases.
…xclave/stack-auth into feat/config-push-source-flags
…alog - Dialog now commits the user's change to the linked repo via the GitHub Contents API, replacing the prior TODO buttons. - Pre-flights `repo`+`workflow` scopes on connected GitHub accounts so the push button only enables when the token can actually commit; otherwise shows "Reconnect with GitHub". - Wraps the suspending bits in a local Suspense boundary so opening the dialog doesn't blank the dashboard. - Issues the Contents GET with `cache: no-store` so back-to-back pushes within ~60s don't 409 on a stale browser-cached blob SHA. - Moves `renderConfigFileContent` from `config-rendering.ts` (Node-only due to fs/path) into `stack-config-file.ts` so the dashboard can call it in the browser. - instrumentation.ts: read `process.env.NEXT_RUNTIME` directly instead of `getNextRuntime()` so the early-startup branch can't throw.
- instrumentation.ts: revert one-line bypass back to getNextRuntime() for consistency across all four call sites. - config-update.tsx: move `resolve?.(result)` out of the setState updater in settleDialog; sync scope-status via useLayoutEffect so users without a GitHub connection no longer see a "Checking…" flash; rename the ALL_CAPS const helper to a verb-form function. - github-api.ts: drop dead 204-branch error check. - checkConfigPathExists: pass `cache: "no-store"` to dodge the GitHub Contents 60s cache; tighten the path-normalize regex to collapse `.//path`, `././path`, and leading `/` in one pass; unify with the workflow YAML helper. Added unit tests for the new cases. - link-existing-onboarding: persist `packageRunner` (npx/pnpx/bunx) so the choice survives reloads. - stack-cli config-file: trim --source-path / --source-workflow-path before the empty check and store the trimmed form; added test cases for whitespace-only rejection and surrounding-whitespace trim.
BilalG1
changed the title
config push…e-flags # Conflicts: # apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.test.ts # apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.ts # apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding.tsx
![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: 3
🧹 Nitpick comments (3)
packages/stack-cli/src/commands/config-file.test.ts (1)
49-275: ⚡ Quick winUse inline snapshots for the new Vitest assertions in this suite.
Most of the new test cases use
toEqual/toThrowpatterns; this file should prefer inline snapshots for expected outputs/errors.Suggested pattern (example)
- expect(buildConfigPushSource("stack.config.ts", {})).toEqual({ type: "pushed-from-unknown" }); + expect(buildConfigPushSource("stack.config.ts", {})).toMatchInlineSnapshot(` + { + "type": "pushed-from-unknown", + } + `);As per coding guidelines
**/*.test.{ts,tsx}: “When writing tests, prefer.toMatchInlineSnapshotover other selectors in Vitest tests.”🤖 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 `@packages/stack-cli/src/commands/config-file.test.ts` around lines 49 - 275, The tests in this suite use toEqual/toThrow assertions but should use Vitest inline snapshots; update each expectation in the buildConfigPushSource tests to use .toMatchInlineSnapshot (for value assertions like the returned object) or .toThrowErrorMatchingInlineSnapshot (for thrown errors) referencing the same expected text/structure, e.g., replace expect(buildConfigPushSource(...)).toEqual({...}) with .toMatchInlineSnapshot(`...`) and expect(() => buildConfigPushSource(...)).toThrow(/.../) with .toThrowErrorMatchingInlineSnapshot(`...`), keeping the same expected strings/objects and trimming whitespace where current assertions already assert trimmed results; ensure all occurrences in this file are converted consistently for the buildConfigPushSource test cases.apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.test.ts (1)
17-24: ⚡ Quick winUse inline snapshots for these Vitest expectations.
The generated YAML and normalization outputs are snapshot-shaped, and the current selectors make the tests noisier than they need to be.
As per coding guidelines, "When writing tests, prefer
.toMatchInlineSnapshotover other selectors in Vitest tests."Also applies to: 34-37, 42-64
🤖 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.test.ts around lines 17 - 24, Replace the multiple toContain/toNotContain assertions against workflowYaml with Vitest inline snapshots: collapse the expected YAML fragments (including values of branch, configPath, WORKFLOW_FILE_PATH and the full run command) into one or a few expect(workflowYaml).toMatchInlineSnapshot(...) calls that embed the normalized YAML output, and remove the granular toContain/toNotContain checks; reference the test variables workflowYaml, branch, configPath, and WORKFLOW_FILE_PATH to build the snapshot content so the test captures the full, normalized output as an inline snapshot.packages/stack-cli/src/commands/config-file.ts (1)
81-90: ⚡ Quick winNormalize repo-relative source paths before storing them.
--source-pathand--source-workflow-pathcurrently keep leading./and/. The dashboard workflow generator strips those prefixes, so the same file can round-trip to different metadata depending on whether it came from the generated workflow or a manual CLI call. Reusing the same normalization here would keep the stored source consistent and avoid odd blob URLs downstream.Suggested change
+ const normalizeRepoRelativePath = (value: string, flagName: string) => { + const normalized = value.trim().replace(/^(?:\.?\/+)+/, ""); + if (normalized.length === 0) { + throw new CliError(`${flagName} must be a non-empty repo-relative path string.`); + } + return normalized; + }; + - const sourcePath = flags.sourcePath!.trim(); - if (sourcePath.length === 0) { - throw new CliError("--source-path must be a non-empty path string."); - } - const sourceWorkflowPath = flags.sourceWorkflowPath!.trim(); - if (sourceWorkflowPath.length === 0) { - throw new CliError("--source-workflow-path must be a non-empty path string."); - } + const sourcePath = normalizeRepoRelativePath(flags.sourcePath!, "--source-path"); + const sourceWorkflowPath = normalizeRepoRelativePath(flags.sourceWorkflowPath!, "--source-workflow-path");Also applies to: 108-109
🤖 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 `@packages/stack-cli/src/commands/config-file.ts` around lines 81 - 90, Normalize repo-relative paths by stripping leading "./" and leading "/" after trimming for both flags.sourcePath and flags.sourceWorkflowPath (and the other occurrences around the later block at the lines noted). Concretely: after getting trimmed values from flags.sourcePath and flags.sourceWorkflowPath (the variables sourcePath and sourceWorkflowPath), remove any leading "./" and leading "/" so stored paths match the dashboard generator's normalization; apply the same normalization to the other similar variables referenced later (the block at the other occurrence around lines 108-109) so all repo-relative source paths are stored consistently.
🤖 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/lib/github-api.test.ts`:
- Around line 11-189: Replace direct assertions with Vitest inline-snapshot
assertions across the new tests: for value checks in parseRepositoryFullName,
encodeGitHubPath, githubRepositoryContentsUrl, isObject, and getFileContent
replace expect(...).toBe / toEqual with expect(...).toMatchInlineSnapshot (or
expect(...).toMatchInlineSnapshot({}) for objects) and capture the current
expected literal as the inline snapshot; for thrown errors in getFileContent
replace expect(...).toThrow(/.../) with
expect(...).toThrowErrorMatchingInlineSnapshot and inline the current message;
for array/length/property checks in commitFile tests prefer asserting the
serialized value with toMatchInlineSnapshot (e.g., JSON.stringify(calls) or the
specific parsedBody) rather than toHaveLength/toHaveProperty. Locate these
assertions in the tests referencing parseRepositoryFullName, encodeGitHubPath,
githubRepositoryContentsUrl, isObject, getFileContent, and commitFile and
convert each to the corresponding
.toMatchInlineSnapshot/.toThrowErrorMatchingInlineSnapshot form, inserting the
current expected values as the inline snapshots.
In `@apps/dashboard/src/lib/github-config-push.test.ts`:
- Around line 4-182: Replace selector-based assertions in the
buildUpdatedConfigFileContent and pushConfigUpdateToGitHub tests with inline
snapshots: for each expect(...).toContain / .not.toContain / .toBe that checks
rendered file content or request body, call .toMatchInlineSnapshot() and paste
the exact string/serialized value as the snapshot (e.g., for tests using
buildUpdatedConfigFileContent results and the PUT body content in
pushConfigUpdateToGitHub). Target the assertions inside the
"buildUpdatedConfigFileContent" it blocks and the "fetches the existing file..."
/ "falls back..." / "skips the commit..." cases in the
"pushConfigUpdateToGitHub" suite, replacing their string containment or equality
checks with matching inline snapshots for the full rendered content or JSON
body.
In `@apps/dashboard/src/lib/github-config-push.ts`:
- Around line 74-76: The JSDoc for pushConfigUpdateToGitHub incorrectly claims
it returns a commit SHA while the function signature is Promise<void>; either
update the doc comment above export async function
pushConfigUpdateToGitHub(options: PushConfigUpdateOptions) to remove the
“Returns the commit SHA” line and state that it returns void (no value), or
change the function signature to Promise<string> and make
pushConfigUpdateToGitHub actually return the commit SHA string where the commit
is created; locate pushConfigUpdateToGitHub and adjust the JSDoc or the return
type/return statement accordingly to make the contract consistent.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.test.ts:
- Around line 17-24: Replace the multiple toContain/toNotContain assertions
against workflowYaml with Vitest inline snapshots: collapse the expected YAML
fragments (including values of branch, configPath, WORKFLOW_FILE_PATH and the
full run command) into one or a few
expect(workflowYaml).toMatchInlineSnapshot(...) calls that embed the normalized
YAML output, and remove the granular toContain/toNotContain checks; reference
the test variables workflowYaml, branch, configPath, and WORKFLOW_FILE_PATH to
build the snapshot content so the test captures the full, normalized output as
an inline snapshot.
In `@packages/stack-cli/src/commands/config-file.test.ts`:
- Around line 49-275: The tests in this suite use toEqual/toThrow assertions but
should use Vitest inline snapshots; update each expectation in the
buildConfigPushSource tests to use .toMatchInlineSnapshot (for value assertions
like the returned object) or .toThrowErrorMatchingInlineSnapshot (for thrown
errors) referencing the same expected text/structure, e.g., replace
expect(buildConfigPushSource(...)).toEqual({...}) with
.toMatchInlineSnapshot(`...`) and expect(() =>
buildConfigPushSource(...)).toThrow(/.../) with
.toThrowErrorMatchingInlineSnapshot(`...`), keeping the same expected
strings/objects and trimming whitespace where current assertions already assert
trimmed results; ensure all occurrences in this file are converted consistently
for the buildConfigPushSource test cases.
In `@packages/stack-cli/src/commands/config-file.ts`:
- Around line 81-90: Normalize repo-relative paths by stripping leading "./" and
leading "/" after trimming for both flags.sourcePath and
flags.sourceWorkflowPath (and the other occurrences around the later block at
the lines noted). Concretely: after getting trimmed values from flags.sourcePath
and flags.sourceWorkflowPath (the variables sourcePath and sourceWorkflowPath),
remove any leading "./" and leading "/" so stored paths match the dashboard
generator's normalization; apply the same normalization to the other similar
variables referenced later (the block at the other occurrence around lines
108-109) so all repo-relative source paths are stored consistently.
🪄 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: 0f55d802-1c6f-4b51-bc48-cb974e8fbc4a
📒 Files selected for processing (18)
apps/backend/src/lib/seed-dummy-data.tsapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client-parts/link-existing-onboarding-workflow.test.tsapps/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.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-settings/page-client.tsxapps/dashboard/src/lib/config-update.tsxapps/dashboard/src/lib/github-api.test.tsapps/dashboard/src/lib/github-api.tsapps/dashboard/src/lib/github-config-push.test.tsapps/dashboard/src/lib/github-config-push.tsapps/e2e/tests/backend/backend-helpers.tspackages/stack-cli/src/commands/config-file.test.tspackages/stack-cli/src/commands/config-file.tspackages/stack-shared/src/config-rendering.tspackages/stack-shared/src/schema-fields.tspackages/stack-shared/src/stack-config-file.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/projects/index.ts
Comment on lines
+11
to
+189
| describe("parseRepositoryFullName", () => { | ||
| it("splits a well-formed full name into owner and repo", () => { | ||
| expect(parseRepositoryFullName("myorg/my-repo")).toEqual({ owner: "myorg", repo: "my-repo" }); | ||
| expect(parseRepositoryFullName("acme.io/some_repo.2")).toEqual({ owner: "acme.io", repo: "some_repo.2" }); | ||
| }); | ||
|
|
||
| it("rejects names without exactly one slash", () => { | ||
| expect(() => parseRepositoryFullName("no-slash")).toThrow(/owner\/repo/); | ||
| expect(() => parseRepositoryFullName("a/b/c")).toThrow(/owner\/repo/); | ||
| }); | ||
|
|
||
| it("rejects empty owner or empty repo", () => { | ||
| expect(() => parseRepositoryFullName("/repo")).toThrow(/owner\/repo/); | ||
| expect(() => parseRepositoryFullName("owner/")).toThrow(/owner\/repo/); | ||
| }); | ||
| }); | ||
|
|
||
| describe("encodeGitHubPath", () => { | ||
| it("percent-encodes each segment but leaves slashes intact", () => { | ||
| expect(encodeGitHubPath("a/b/c")).toBe("a/b/c"); | ||
| expect(encodeGitHubPath("dir with space/file.ts")).toBe("dir%20with%20space/file.ts"); | ||
| expect(encodeGitHubPath(".github/workflows/x.yml")).toBe(".github/workflows/x.yml"); | ||
| }); | ||
|
|
||
| it("encodes special characters in segments", () => { | ||
| expect(encodeGitHubPath("hash#dir/q?file.ts")).toBe("hash%23dir/q%3Ffile.ts"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("githubRepositoryContentsUrl", () => { | ||
| it("composes a contents URL with encoded owner, repo, and path", () => { | ||
| expect(githubRepositoryContentsUrl("myorg", "my-repo", "stack.config.ts")) | ||
| .toBe("/repos/myorg/my-repo/contents/stack.config.ts"); | ||
| expect(githubRepositoryContentsUrl("my org", "my repo", "dir with space/file.ts")) | ||
| .toBe("/repos/my%20org/my%20repo/contents/dir%20with%20space/file.ts"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("isObject", () => { | ||
| it("matches plain objects only", () => { | ||
| expect(isObject({})).toBe(true); | ||
| expect(isObject({ a: 1 })).toBe(true); | ||
| expect(isObject(null)).toBe(false); | ||
| expect(isObject([])).toBe(false); | ||
| expect(isObject("string")).toBe(false); | ||
| expect(isObject(42)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("getFileContent", () => { | ||
| function fakeGithubFetch(handler: (path: string, init?: RequestInit) => unknown) { | ||
| const calls: { path: string, init?: RequestInit }[] = []; | ||
| const fn = async (path: string, init?: RequestInit) => { | ||
| calls.push({ path, init }); | ||
| return handler(path, init); | ||
| }; | ||
| return { fn, calls }; | ||
| } | ||
|
|
||
| it("decodes base64 content and returns the SHA on success", async () => { | ||
| const text = "export const config = {};\n"; | ||
| const base64 = Buffer.from(text, "utf-8").toString("base64"); | ||
| const { fn, calls } = fakeGithubFetch(() => ({ | ||
| type: "file", | ||
| encoding: "base64", | ||
| content: base64, | ||
| sha: "abc123", | ||
| })); | ||
|
|
||
| const result = await getFileContent(fn, { | ||
| owner: "myorg", | ||
| repo: "my-repo", | ||
| branch: "main", | ||
| path: "stack.config.ts", | ||
| }); | ||
| expect(result).toEqual({ text, sha: "abc123" }); | ||
| expect(calls[0].path).toBe("/repos/myorg/my-repo/contents/stack.config.ts?ref=main"); | ||
| }); | ||
|
|
||
| it("handles base64 content with embedded whitespace (GitHub line-wraps long blobs)", async () => { | ||
| const text = "x".repeat(200); | ||
| const base64 = Buffer.from(text, "utf-8").toString("base64"); | ||
| const wrapped = base64.match(/.{1,60}/g)!.join("\n"); | ||
| const { fn } = fakeGithubFetch(() => ({ | ||
| type: "file", | ||
| encoding: "base64", | ||
| content: wrapped, | ||
| sha: "abc", | ||
| })); | ||
| const result = await getFileContent(fn, { | ||
| owner: "o", | ||
| repo: "r", | ||
| branch: "main", | ||
| path: "stack.config.ts", | ||
| }); | ||
| expect(result?.text).toBe(text); | ||
| }); | ||
|
|
||
| it("returns null when the file is missing (Not Found error)", async () => { | ||
| const { fn } = fakeGithubFetch(() => { | ||
| throw new Error("Not Found"); | ||
| }); | ||
| const result = await getFileContent(fn, { | ||
| owner: "o", repo: "r", branch: "main", path: "missing.ts", | ||
| }); | ||
| expect(result).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when the response is a directory (array)", async () => { | ||
| const { fn } = fakeGithubFetch(() => [{ type: "file", path: "x" }]); | ||
| const result = await getFileContent(fn, { owner: "o", repo: "r", branch: "main", path: "x" }); | ||
| expect(result).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when the response type is not 'file'", async () => { | ||
| const { fn } = fakeGithubFetch(() => ({ type: "dir", sha: "x", content: "" })); | ||
| const result = await getFileContent(fn, { owner: "o", repo: "r", branch: "main", path: "x" }); | ||
| expect(result).toBeNull(); | ||
| }); | ||
|
|
||
| it("re-throws non-404 errors", async () => { | ||
| const { fn } = fakeGithubFetch(() => { | ||
| throw new Error("Server error"); | ||
| }); | ||
| await expect(getFileContent(fn, { owner: "o", repo: "r", branch: "main", path: "x.ts" })) | ||
| .rejects.toThrow(/Server error/); | ||
| }); | ||
|
|
||
| it("throws on unexpected encoding", async () => { | ||
| const { fn } = fakeGithubFetch(() => ({ | ||
| type: "file", | ||
| encoding: "utf-8", | ||
| content: "raw", | ||
| sha: "abc", | ||
| })); | ||
| await expect(getFileContent(fn, { owner: "o", repo: "r", branch: "main", path: "x.ts" })) | ||
| .rejects.toThrow(/encoding/); | ||
| }); | ||
| }); | ||
|
|
||
| describe("commitFile", () => { | ||
| it("PUTs the encoded content with the given message and sha", async () => { | ||
| const calls: { path: string, init?: RequestInit }[] = []; | ||
| const fn = async (path: string, init?: RequestInit) => { | ||
| calls.push({ path, init }); | ||
| return null; | ||
| }; | ||
| await commitFile(fn, { | ||
| owner: "myorg", | ||
| repo: "my-repo", | ||
| branch: "main", | ||
| path: "stack.config.ts", | ||
| content: "hello", | ||
| message: "chore: update", | ||
| sha: "deadbeef", | ||
| }); | ||
| expect(calls).toHaveLength(1); | ||
| expect(calls[0].path).toBe("/repos/myorg/my-repo/contents/stack.config.ts"); | ||
| expect(calls[0].init?.method).toBe("PUT"); | ||
| const parsedBody = JSON.parse(String(calls[0].init?.body)); | ||
| expect(parsedBody.message).toBe("chore: update"); | ||
| expect(parsedBody.branch).toBe("main"); | ||
| expect(parsedBody.sha).toBe("deadbeef"); | ||
| expect(Buffer.from(parsedBody.content, "base64").toString("utf-8")).toBe("hello"); | ||
| }); | ||
|
|
||
| it("omits sha when creating a new file", async () => { | ||
| const calls: { path: string, init?: RequestInit }[] = []; | ||
| const fn = async (path: string, init?: RequestInit) => { | ||
| calls.push({ path, init }); | ||
| return null; | ||
| }; | ||
| await commitFile(fn, { | ||
| owner: "o", repo: "r", branch: "main", path: "new.ts", content: "x", message: "create", | ||
| }); | ||
| const parsedBody = JSON.parse(String(calls[0].init?.body)); | ||
| expect(parsedBody).not.toHaveProperty("sha"); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Align Vitest assertions with inline snapshot style.
Most assertions in this new test suite use direct selectors (toBe, toEqual, toThrow) instead of inline snapshots. Please migrate these newly added tests to .toMatchInlineSnapshot where applicable to match the repository’s test convention.
As per coding guidelines, "**/*.test.{ts,tsx}: When writing tests, prefer .toMatchInlineSnapshot over other selectors in Vitest tests."
🤖 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/lib/github-api.test.ts` around lines 11 - 189, Replace
direct assertions with Vitest inline-snapshot assertions across the new tests:
for value checks in parseRepositoryFullName, encodeGitHubPath,
githubRepositoryContentsUrl, isObject, and getFileContent replace
expect(...).toBe / toEqual with expect(...).toMatchInlineSnapshot (or
expect(...).toMatchInlineSnapshot({}) for objects) and capture the current
expected literal as the inline snapshot; for thrown errors in getFileContent
replace expect(...).toThrow(/.../) with
expect(...).toThrowErrorMatchingInlineSnapshot and inline the current message;
for array/length/property checks in commitFile tests prefer asserting the
serialized value with toMatchInlineSnapshot (e.g., JSON.stringify(calls) or the
specific parsedBody) rather than toHaveLength/toHaveProperty. Locate these
assertions in the tests referencing parseRepositoryFullName, encodeGitHubPath,
githubRepositoryContentsUrl, isObject, getFileContent, and commitFile and
convert each to the corresponding
.toMatchInlineSnapshot/.toThrowErrorMatchingInlineSnapshot form, inserting the
current expected values as the inline snapshots.
Comment on lines
+4
to
+182
| describe("buildUpdatedConfigFileContent", () => { | ||
| it("merges a flat dot-notation update into the existing config", () => { | ||
| const current = `import type { StackConfig } from "@stackframe/stack"; | ||
|
|
||
| export const config: StackConfig = { | ||
| teams: { allowClientTeamCreation: false }, | ||
| }; | ||
| `; | ||
| const result = buildUpdatedConfigFileContent(current, { "teams.allowClientTeamCreation": true }); | ||
| expect(result).toContain('"teams": {'); | ||
| expect(result).toContain('"allowClientTeamCreation": true'); | ||
| expect(result).toContain('import type { StackConfig } from "@stackframe/stack"'); | ||
| }); | ||
|
|
||
| it("preserves the existing @stackframe/* import package when re-rendering", () => { | ||
| const current = `import type { StackConfig } from "@stackframe/react"; | ||
|
|
||
| export const config: StackConfig = {}; | ||
| `; | ||
| const result = buildUpdatedConfigFileContent(current, { "auth.allowSignUp": true }); | ||
| expect(result).toContain('import type { StackConfig } from "@stackframe/react"'); | ||
| }); | ||
|
|
||
| it("defaults to @stackframe/js when no recognizable import is present", () => { | ||
| const current = `export const config = {};\n`; | ||
| const result = buildUpdatedConfigFileContent(current, { "auth.allowSignUp": true }); | ||
| expect(result).toContain('import type { StackConfig } from "@stackframe/js"'); | ||
| }); | ||
|
|
||
| it("adds new top-level keys to an empty config", () => { | ||
| const current = `import type { StackConfig } from "@stackframe/js"; | ||
| export const config: StackConfig = {}; | ||
| `; | ||
| const result = buildUpdatedConfigFileContent(current, { | ||
| "payments.items.todos.displayName": "Todos", | ||
| "payments.items.todos.customerType": "user", | ||
| }); | ||
| expect(result).toContain(`"payments": { | ||
| "items": { | ||
| "todos": { | ||
| "displayName": "Todos", | ||
| "customerType": "user" | ||
| } | ||
| } | ||
| }`); | ||
| }); | ||
|
|
||
| it("replaces an existing nested value via dot notation", () => { | ||
| const current = `import type { StackConfig } from "@stackframe/js"; | ||
| export const config: StackConfig = { | ||
| payments: { items: { todos: { displayName: "Old" } } }, | ||
| }; | ||
| `; | ||
| const result = buildUpdatedConfigFileContent(current, { | ||
| "payments.items.todos.displayName": "New", | ||
| }); | ||
| expect(result).toContain('"displayName": "New"'); | ||
| expect(result).not.toContain('"Old"'); | ||
| }); | ||
|
|
||
| it("refuses to mutate a show-onboarding placeholder file", () => { | ||
| const current = `export const config = "show-onboarding";`; | ||
| expect(() => buildUpdatedConfigFileContent(current, { "auth.allowSignUp": true })) | ||
| .toThrow(/onboarding placeholder/); | ||
| }); | ||
|
|
||
| it("throws when the file does not export a `config` binding", () => { | ||
| expect(() => buildUpdatedConfigFileContent(`export const other = {};`, { "a": 1 })) | ||
| .toThrow(/must export a plain `config` object/); | ||
| }); | ||
| }); | ||
|
|
||
| describe("pushConfigUpdateToGitHub", () => { | ||
| function buildFakeFetch(initialContent: string) { | ||
| const base64 = Buffer.from(initialContent, "utf-8").toString("base64"); | ||
| const calls: { path: string, init?: RequestInit }[] = []; | ||
| const fn = async (path: string, init?: RequestInit) => { | ||
| calls.push({ path, init }); | ||
| if (init?.method === "PUT") { | ||
| return { commit: { sha: "newsha" } }; | ||
| } | ||
| return { | ||
| type: "file", | ||
| encoding: "base64", | ||
| content: base64, | ||
| sha: "oldsha", | ||
| }; | ||
| }; | ||
| return { fn, calls }; | ||
| } | ||
|
|
||
| const baseSource = { | ||
| type: "pushed-from-github" as const, | ||
| owner: "myorg", | ||
| repo: "my-repo", | ||
| branch: "main", | ||
| commitHash: "abc", | ||
| configFilePath: "stack.config.ts", | ||
| }; | ||
|
|
||
| it("fetches the existing file, merges the update, and PUTs the new content", async () => { | ||
| const { fn, calls } = buildFakeFetch(`import type { StackConfig } from "@stackframe/js"; | ||
| export const config: StackConfig = { teams: { allowClientTeamCreation: false } }; | ||
| `); | ||
| await pushConfigUpdateToGitHub({ | ||
| source: baseSource, | ||
| configUpdate: { "teams.allowClientTeamCreation": true }, | ||
| commitMessage: "feat: enable team creation", | ||
| githubFetch: fn, | ||
| }); | ||
| expect(calls).toHaveLength(2); | ||
| expect(calls[0].path).toBe("/repos/myorg/my-repo/contents/stack.config.ts?ref=main"); | ||
| expect(calls[1].init?.method).toBe("PUT"); | ||
| const body = JSON.parse(String(calls[1].init?.body)); | ||
| expect(body.message).toBe("feat: enable team creation"); | ||
| expect(body.sha).toBe("oldsha"); | ||
| expect(body.branch).toBe("main"); | ||
| expect(Buffer.from(body.content, "base64").toString("utf-8")).toContain('"allowClientTeamCreation": true'); | ||
| }); | ||
|
|
||
| it("falls back to a default commit message when none is provided", async () => { | ||
| const { fn, calls } = buildFakeFetch(`export const config = {};\n`); | ||
| await pushConfigUpdateToGitHub({ | ||
| source: baseSource, | ||
| configUpdate: { "auth.allowSignUp": true }, | ||
| commitMessage: " ", | ||
| githubFetch: fn, | ||
| }); | ||
| const putBody = JSON.parse(String(calls[1].init?.body)); | ||
| expect(putBody.message).toBe("chore(stack-auth): update config from dashboard"); | ||
| }); | ||
|
|
||
| it("skips the commit when the new rendered file is identical to the old one", async () => { | ||
| const same = `import type { StackConfig } from "@stackframe/js"; | ||
|
|
||
| export const config: StackConfig = { | ||
| "teams": { | ||
| "allowClientTeamCreation": true | ||
| } | ||
| }; | ||
| `; | ||
| const { fn, calls } = buildFakeFetch(same); | ||
| await pushConfigUpdateToGitHub({ | ||
| source: baseSource, | ||
| configUpdate: { "teams.allowClientTeamCreation": true }, | ||
| commitMessage: "no-op", | ||
| githubFetch: fn, | ||
| }); | ||
| expect(calls.find((c) => c.init?.method === "PUT")).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("surfaces a clear error when the config file is missing on the branch", async () => { | ||
| const fn = async () => { | ||
| throw new Error("Not Found"); | ||
| }; | ||
| await expect( | ||
| pushConfigUpdateToGitHub({ | ||
| source: baseSource, | ||
| configUpdate: { "auth.allowSignUp": true }, | ||
| commitMessage: "x", | ||
| githubFetch: fn, | ||
| }) | ||
| ).rejects.toThrow(/Could not find stack\.config\.ts/); | ||
| }); | ||
|
|
||
| it("propagates non-404 GitHub errors", async () => { | ||
| const fn = async () => { | ||
| throw new Error("Bad credentials"); | ||
| }; | ||
| await expect( | ||
| pushConfigUpdateToGitHub({ | ||
| source: baseSource, | ||
| configUpdate: { "auth.allowSignUp": true }, | ||
| commitMessage: "x", | ||
| githubFetch: fn, | ||
| }) | ||
| ).rejects.toThrow(/Bad credentials/); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Use inline snapshots for these Vitest assertions.
This new suite mostly uses selector-based expectations; please convert the added assertions to .toMatchInlineSnapshot where possible to stay consistent with the repository’s test style rule.
As per coding guidelines, "**/*.test.{ts,tsx}: When writing tests, prefer .toMatchInlineSnapshot over other selectors in Vitest tests."
🤖 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/lib/github-config-push.test.ts` around lines 4 - 182,
Replace selector-based assertions in the buildUpdatedConfigFileContent and
pushConfigUpdateToGitHub tests with inline snapshots: for each
expect(...).toContain / .not.toContain / .toBe that checks rendered file content
or request body, call .toMatchInlineSnapshot() and paste the exact
string/serialized value as the snapshot (e.g., for tests using
buildUpdatedConfigFileContent results and the PUT body content in
pushConfigUpdateToGitHub). Target the assertions inside the
"buildUpdatedConfigFileContent" it blocks and the "fetches the existing file..."
/ "falls back..." / "skips the commit..." cases in the
"pushConfigUpdateToGitHub" suite, replacing their string containment or equality
checks with matching inline snapshots for the full rendered content or JSON
body.
Comment on lines
+74
to
+76
| * Returns the commit SHA of the new commit, useful for surfacing in logs / UI. | ||
| */ | ||
| export async function pushConfigUpdateToGitHub(options: PushConfigUpdateOptions): Promise<void> { |
Contributor
There was a problem hiding this comment.
Fix stale return contract in the function doc comment.
Line 74 says this returns a commit SHA, but Line 76 is Promise<void>. Please update the comment (or the function contract) so callers aren’t misled.
Suggested doc fix
- * Returns the commit SHA of the new commit, useful for surfacing in logs / UI.
+ * Commits the updated config file when needed; returns once GitHub accepts the write.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * Returns the commit SHA of the new commit, useful for surfacing in logs / UI. | |
| */ | |
| export async function pushConfigUpdateToGitHub(options: PushConfigUpdateOptions): Promise<void> { | |
| * Commits the updated config file when needed; returns once GitHub accepts the write. | |
| */ | |
| export async function pushConfigUpdateToGitHub(options: PushConfigUpdateOptions): Promise<void> { |
🤖 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/lib/github-config-push.ts` around lines 74 - 76, The JSDoc
for pushConfigUpdateToGitHub incorrectly claims it returns a commit SHA while
the function signature is Promise<void>; either update the doc comment above
export async function pushConfigUpdateToGitHub(options: PushConfigUpdateOptions)
to remove the “Returns the commit SHA” line and state that it returns void (no
value), or change the function signature to Promise<string> and make
pushConfigUpdateToGitHub actually return the commit SHA string where the commit
is created; locate pushConfigUpdateToGitHub and adjust the JSDoc or the return
type/return statement accordingly to make the contract consistent.
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.
End-to-end flow for managing Stack Auth config via GitHub: link a repo during onboarding, edit settings in the dashboard, and have the change committed to your repo + synced back via a GitHub Actions workflow.
What this adds
stack config push --source github --source-repo --source-path --source-workflow-path. Records the source on the config row so the dashboard knows where the file lives. ReadsGITHUB_SHA/GITHUB_REF_NAMEfor commit + branch.stack.config.{ts,js}paths, writesSTACK_AUTH_PROJECT_ID+STACK_AUTH_SECRET_SERVER_KEYsecrets, and commits a generated workflow YAML that re-runsstack config pushon every change to the config file.repo+workflowscopes on the user's GitHub connection; if missing, the button flips to "Reconnect with GitHub". On push, commits the dashboard's edit straight to the linked repo/branch via the Contents API (withcache: "no-store"to dodge GitHub's 60s GET cache so consecutive pushes don't 409). Suspense boundary scoped to the dialog body so opening it doesn't blank the dashboard.workflow_path.Test plan
pnpm lint(29/29) ✓pnpm typecheck(29/29) ✓pnpm --filter @stackframe/stack-cli test(111/111) ✓link-existing-onboarding-workflow,github-api,github-config-push) — 37/37 ✓BilalG1/lex-lookuplinked to a local dev project; passkey toggled, push committed0bb958bd(commit).Summary by CodeRabbit
Release Notes
New Features
Improvements