feat: allow triggering initial deploy from dashboard#5166
Merged
ogzhanolguncu merged 4 commits intomainfrom Mar 3, 2026
Merged
feat: allow triggering initial deploy from dashboard#5166ogzhanolguncu merged 4 commits intomainfrom
ogzhanolguncu merged 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
Contributor
📝 WalkthroughWalkthroughThis change adds support for unauthenticated deployments and implements a complete deployment creation flow. Backend support for public GitHub API access enables unauthenticated branch lookups. A new TRPC mutation endpoint with rate limiting, external control service integration, and audit logging enables dashboard-based deployment creation. Project creation is simplified to use transaction metadata for project ID retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Dashboard as Dashboard UI
participant TRPC as TRPC Server
participant CtrlSvc as Control Service
participant DB as Database
participant AuditLog as Audit Log
User->>Dashboard: Click Deploy
Dashboard->>TRPC: deploy.deployment.create(projectId, environmentSlug)
TRPC->>TRPC: Validate input & check rate limit
TRPC->>DB: Resolve project by projectId
TRPC->>CtrlSvc: Create deployment request
CtrlSvc-->>TRPC: deploymentId
TRPC->>AuditLog: Log "deployment.create" event
TRPC-->>Dashboard: Return deploymentId
Dashboard->>Dashboard: Invalidate deployments query
Dashboard->>Dashboard: Show success toast
Dashboard->>Dashboard: Navigate to deployment page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Command failed Comment |
This was referenced Mar 2, 2026
19 tasks
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
mcstepp and
perkinsjr
as code owners
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
d508da6 to
33266fb
Compare
ogzhanolguncu
force-pushed
the
update-all-env-settings
branch
from
d46f025 to
09b4777
Compare
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
33266fb to
e7ff4b4
Compare
ogzhanolguncu
force-pushed
the
update-all-env-settings
branch
from
09b4777 to
aa3d88f
Compare
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
e7ff4b4 to
d71bdc2
Compare
ogzhanolguncu
force-pushed
the
update-all-env-settings
branch
from
aa3d88f to
c654a92
Compare
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
d71bdc2 to
74e31cf
Compare
chronark
approved these changes
Mar 3, 2026
ogzhanolguncu
force-pushed
the
update-all-env-settings
branch
from
c790e8d to
61a2959
Compare
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
6e2a149 to
d8b61c2
Compare
ogzhanolguncu
force-pushed
the
update-all-env-settings
branch
from
fe565ce to
00949fc
Compare
ogzhanolguncu
force-pushed
the
update-all-env-settings
branch
from
00949fc to
fe5ceef
Compare
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
d8b61c2 to
8db047e
Compare
graphite-app
bot
changed the base branch from
update-all-env-settings
to
graphite-base/5166
Flo4604
approved these changes
Mar 3, 2026
ogzhanolguncu
force-pushed
the
graphite-base/5166
branch
from
fe5ceef to
74916d8
Compare
ogzhanolguncu
force-pushed
the
trigger-deploy
branch
from
8db047e to
3867447
Compare
Merge activity
|
19 tasks
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@svc/ctrl/worker/github/noop.go`:
- Around line 35-78: The Noop GetBranchHeadCommitPublic implementation violates
the Noop contract by performing real HTTP requests; change
Noop.GetBranchHeadCommitPublic to behave like other Noop methods and immediately
return an error instead of making network calls. Replace the body in
Noop.GetBranchHeadCommitPublic to return a suitable fault (e.g., fault.New("noop
client: operation not supported", fault.Internal("no-op github client")), or a
consistent sentinel error) and zero-value CommitInfo, or alternatively move the
current HTTP logic into the real GitHub client implementation (e.g., the
non-Noop type) and keep Noop as a pure error-returning stub; ensure refs to
CommitInfo, GetBranchHeadCommitPublic and Noop are updated accordingly.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/create-project.tsx:
- Line 54: Replace the unsafe type assertion on tx.metadata with a runtime type
guard: create a small predicate (e.g., isProjectMetadata(obj): obj is {
projectId: string }) that checks obj is non-null and typeof obj.projectId ===
"string", use it to validate tx.metadata then call onProjectCreated(projectId)
when valid, and handle the error/early-return when the check fails; reference
the existing onProjectCreated call and tx.metadata to locate where to apply the
guard.
In `@web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.ts`:
- Line 14: The createDeploy router is wired to the wrong rate-limit bucket:
replace the call using ratelimit.update with the create bucket so create traffic
is tracked correctly; locate the chain where createDeploy applies middleware
(.use(withRatelimit(ratelimit.update))) and change it to use ratelimit.create
(i.e., .use(withRatelimit(ratelimit.create))) so the createDeploy mutation
consumes from the create quota/telemetry.
- Around line 57-67: The inline .catch on the promise returned by
ctrl.createDeployment should be removed so raw Error objects aren't passed into
TRPCError; instead return the awaited result directly from createDeployment and
allow the surrounding try/catch (the outer catch that constructs a sanitized
TRPCError) to handle failures. Locate the call to ctrl.createDeployment in the
create-deploy handler and delete the chained .catch(...) block (including
console.error and the TRPCError construction) so that any thrown error bubbles
to the existing outer catch which already converts unknown errors to a safe
string message.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
svc/ctrl/worker/deploy/deploy_handler.gosvc/ctrl/worker/github/client.gosvc/ctrl/worker/github/interface.gosvc/ctrl/worker/github/noop.goweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/configure-deployment.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/onboarding/steps/create-project.tsxweb/apps/dashboard/lib/collections/deploy/projects.tsweb/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.tsweb/apps/dashboard/lib/trpc/routers/index.tsweb/internal/schema/src/auditlog.ts
Comment on lines
+35
to
+78
|
|
||
| // GetBranchHeadCommitPublic retrieves the HEAD commit using the public GitHub | ||
| // API without authentication. Works for public repositories even when GitHub | ||
| // App credentials are not configured. | ||
| func (n *Noop) GetBranchHeadCommitPublic(repo string, branch string) (CommitInfo, error) { | ||
| httpClient := &http.Client{Timeout: 30 * time.Second} | ||
|
|
||
| requestURL := fmt.Sprintf("https://api.github.com/repos/%s/commits/%s", repo, url.PathEscape(branch)) | ||
|
|
||
| req, err := http.NewRequest(http.MethodGet, requestURL, nil) | ||
| if err != nil { | ||
| return CommitInfo{}, fault.Wrap(err, fault.Internal("failed to create request")) | ||
| } | ||
|
|
||
| req.Header.Set("Accept", "application/vnd.github+json") | ||
| req.Header.Set("X-GitHub-Api-Version", "2022-11-28") | ||
|
|
||
| resp, err := httpClient.Do(req) | ||
| if err != nil { | ||
| return CommitInfo{}, fault.Wrap(err, fault.Internal("failed to fetch branch head commit")) | ||
| } | ||
| defer func() { _ = resp.Body.Close() }() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| body, _ := io.ReadAll(resp.Body) | ||
| return CommitInfo{}, fault.New( | ||
| "failed to fetch branch head commit (public)", | ||
| fault.Internal(fmt.Sprintf("status %d: %s", resp.StatusCode, string(body))), | ||
| ) | ||
| } | ||
|
|
||
| var commit ghCommitResponse | ||
| if err := json.NewDecoder(resp.Body).Decode(&commit); err != nil { | ||
| return CommitInfo{}, fault.Wrap(err, fault.Internal("failed to decode commit response")) | ||
| } | ||
|
|
||
| return CommitInfoFromRaw( | ||
| commit.SHA, | ||
| commit.Commit.Message, | ||
| commit.Author.Login, | ||
| commit.Author.AvatarURL, | ||
| commit.Commit.Author.Date, | ||
| ), nil | ||
| } |
Contributor
There was a problem hiding this comment.
Noop implementation makes real HTTP calls, contradicting its documented purpose.
The Noop struct is documented as "returns errors for all operations" (line 14), but GetBranchHeadCommitPublic performs actual HTTP requests. This breaks the expected contract where Noop should be safe to use when GitHub credentials aren't configured without making external calls.
Consider either:
- Return an error like other Noop methods if this behavior is unintended
- Rename/restructure to clarify that this client can make unauthenticated calls even without credentials
Option 1: Return error like other Noop methods
func (n *Noop) GetBranchHeadCommitPublic(repo string, branch string) (CommitInfo, error) {
- httpClient := &http.Client{Timeout: 30 * time.Second}
- // ... full implementation ...
+ return CommitInfo{}, fault.New("GitHub client not configured: use Client for public repository access")
}📝 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
| // GetBranchHeadCommitPublic retrieves the HEAD commit using the public GitHub | |
| // API without authentication. Works for public repositories even when GitHub | |
| // App credentials are not configured. | |
| func (n *Noop) GetBranchHeadCommitPublic(repo string, branch string) (CommitInfo, error) { | |
| httpClient := &http.Client{Timeout: 30 * time.Second} | |
| requestURL := fmt.Sprintf("https://api.github.com/repos/%s/commits/%s", repo, url.PathEscape(branch)) | |
| req, err := http.NewRequest(http.MethodGet, requestURL, nil) | |
| if err != nil { | |
| return CommitInfo{}, fault.Wrap(err, fault.Internal("failed to create request")) | |
| } | |
| req.Header.Set("Accept", "application/vnd.github+json") | |
| req.Header.Set("X-GitHub-Api-Version", "2022-11-28") | |
| resp, err := httpClient.Do(req) | |
| if err != nil { | |
| return CommitInfo{}, fault.Wrap(err, fault.Internal("failed to fetch branch head commit")) | |
| } | |
| defer func() { _ = resp.Body.Close() }() | |
| if resp.StatusCode != http.StatusOK { | |
| body, _ := io.ReadAll(resp.Body) | |
| return CommitInfo{}, fault.New( | |
| "failed to fetch branch head commit (public)", | |
| fault.Internal(fmt.Sprintf("status %d: %s", resp.StatusCode, string(body))), | |
| ) | |
| } | |
| var commit ghCommitResponse | |
| if err := json.NewDecoder(resp.Body).Decode(&commit); err != nil { | |
| return CommitInfo{}, fault.Wrap(err, fault.Internal("failed to decode commit response")) | |
| } | |
| return CommitInfoFromRaw( | |
| commit.SHA, | |
| commit.Commit.Message, | |
| commit.Author.Login, | |
| commit.Author.AvatarURL, | |
| commit.Commit.Author.Date, | |
| ), nil | |
| } | |
| // GetBranchHeadCommitPublic retrieves the HEAD commit using the public GitHub | |
| // API without authentication. Works for public repositories even when GitHub | |
| // App credentials are not configured. | |
| func (n *Noop) GetBranchHeadCommitPublic(repo string, branch string) (CommitInfo, error) { | |
| return CommitInfo{}, fault.New("GitHub client not configured: use Client for public repository access") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@svc/ctrl/worker/github/noop.go` around lines 35 - 78, The Noop
GetBranchHeadCommitPublic implementation violates the Noop contract by
performing real HTTP requests; change Noop.GetBranchHeadCommitPublic to behave
like other Noop methods and immediately return an error instead of making
network calls. Replace the body in Noop.GetBranchHeadCommitPublic to return a
suitable fault (e.g., fault.New("noop client: operation not supported",
fault.Internal("no-op github client")), or a consistent sentinel error) and
zero-value CommitInfo, or alternatively move the current HTTP logic into the
real GitHub client implementation (e.g., the non-Noop type) and keep Noop as a
pure error-returning stub; ensure refs to CommitInfo, GetBranchHeadCommitPublic
and Noop are updated accordingly.
| if (created) { | ||
| onProjectCreated(created.id); | ||
| } | ||
| onProjectCreated((tx.metadata as { projectId: string }).projectId); |
Contributor
There was a problem hiding this comment.
Avoid type assertion as Type per coding guidelines.
The as { projectId: string } cast bypasses type safety. Consider using runtime validation or defining a proper type for the transaction metadata.
Proposed fix using runtime check
- onProjectCreated((tx.metadata as { projectId: string }).projectId);
+ const metadata = tx.metadata;
+ if (metadata && typeof metadata === "object" && "projectId" in metadata && typeof metadata.projectId === "string") {
+ onProjectCreated(metadata.projectId);
+ } else {
+ throw new Error("Project creation failed: missing projectId in response");
+ }As per coding guidelines: "Never compromise type safety: No any, no ! (non-null assertion), no as Type"
📝 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
| onProjectCreated((tx.metadata as { projectId: string }).projectId); | |
| const metadata = tx.metadata; | |
| if (metadata && typeof metadata === "object" && "projectId" in metadata && typeof metadata.projectId === "string") { | |
| onProjectCreated(metadata.projectId); | |
| } else { | |
| throw new Error("Project creation failed: missing projectId in response"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/onboarding/steps/create-project.tsx
at line 54, Replace the unsafe type assertion on tx.metadata with a runtime type
guard: create a small predicate (e.g., isProjectMetadata(obj): obj is {
projectId: string }) that checks obj is non-null and typeof obj.projectId ===
"string", use it to validate tx.metadata then call onProjectCreated(projectId)
when valid, and handle the error/early-return when the check fails; reference
the existing onProjectCreated call and tx.metadata to locate where to apply the
guard.
| import { z } from "zod"; | ||
|
|
||
| export const createDeploy = workspaceProcedure | ||
| .use(withRatelimit(ratelimit.update)) |
Contributor
There was a problem hiding this comment.
Use the create rate-limit bucket for this create mutation.
createDeploy currently uses ratelimit.update, which can skew quota behavior/telemetry for create traffic. Prefer ratelimit.create here.
Suggested surgical change
export const createDeploy = workspaceProcedure
- .use(withRatelimit(ratelimit.update))
+ .use(withRatelimit(ratelimit.create))📝 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
| .use(withRatelimit(ratelimit.update)) | |
| .use(withRatelimit(ratelimit.create)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.ts` at
line 14, The createDeploy router is wired to the wrong rate-limit bucket:
replace the call using ratelimit.update with the create bucket so create traffic
is tracked correctly; locate the chain where createDeploy applies middleware
(.use(withRatelimit(ratelimit.update))) and change it to use ratelimit.create
(i.e., .use(withRatelimit(ratelimit.create))) so the createDeploy mutation
consumes from the create quota/telemetry.
Comment on lines
+57
to
+67
| const result = await ctrl | ||
| .createDeployment({ | ||
| projectId: input.projectId, | ||
| environmentSlug: input.environmentSlug, | ||
| }) | ||
| .catch((err) => { | ||
| console.error(err); | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: err, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the create-deploy.ts file to see the full context
cat -n web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.tsRepository: unkeyed/unkey
Length of output: 3546
🏁 Script executed:
# Check for fault library usage in TRPC routers
rg -l 'fault' web/apps/dashboard/lib/trpc/routers/ --type ts | head -20Repository: unkeyed/unkey
Length of output: 1296
🏁 Script executed:
# Inspect error handling patterns in this specific file
rg -nP 'TRPCError|catch|fault' web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.tsRepository: unkeyed/unkey
Length of output: 328
🏁 Script executed:
# Verify the error flow - check if removing inner catch would work properly
# Look at other TRPC routers to see if they use similar patterns
rg -A8 'createClient|DeployService' web/apps/dashboard/lib/trpc/routers/ -t ts | head -50Repository: unkeyed/unkey
Length of output: 4796
🏁 Script executed:
# Check if fault library is being used in error handling across TRPC routers
# and what the pattern looks like
rg -B2 -A5 'from.*fault|import.*fault' web/apps/dashboard/lib/trpc/routers/ --type ts | head -60Repository: unkeyed/unkey
Length of output: 2537
🏁 Script executed:
# Check if fault library is actually used in the codebase at all
rg -l 'fault' web/apps/dashboard/lib --type ts | head -30Repository: unkeyed/unkey
Length of output: 1905
🏁 Script executed:
# Check the lib directory structure to see where fault might be
fd -t f 'fault' web/apps/dashboard/lib 2>/dev/null | head -20Repository: unkeyed/unkey
Length of output: 175
🏁 Script executed:
# Search for fault library package or configuration
rg 'fault' web/apps/dashboard/package.json web/package.jsonRepository: unkeyed/unkey
Length of output: 39
🏁 Script executed:
# Check if fault is imported anywhere in the codebase with different patterns
rg 'import.*from.*fault|from.*fault|require.*fault' web/apps/dashboard --type tsRepository: unkeyed/unkey
Length of output: 1590
🏁 Script executed:
# Check the actual fault files to see what they contain
cat -n web/apps/dashboard/lib/zod-helper.ts | head -50Repository: unkeyed/unkey
Length of output: 1777
🏁 Script executed:
# Look for error handling in similar TRPC routers without the inner catch pattern
rg -B5 -A10 'catch \(error\)' web/apps/dashboard/lib/trpc/routers/deploy/ --type ts -A5 | head -80Repository: unkeyed/unkey
Length of output: 7126
🏁 Script executed:
# Double-check lines 86-95 are actually correct and not problematic
sed -n '86,96p' web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.tsRepository: unkeyed/unkey
Length of output: 359
🏁 Script executed:
# Check if there are any other inner .catch() patterns with message: err in this file
rg -n '\.catch\(' web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.ts -A3Repository: unkeyed/unkey
Length of output: 199
Remove the inner .catch() to prevent raw error propagation through TRPC.
At line 66, message: err passes a raw error object that can leak control-plane internals and violates type safety (TRPCError expects a string message). The outer catch block (lines 86–95) already handles this correctly by sanitizing unknown errors with a generic message. Let the outer catch handle all errors from the createDeployment call.
Suggested change
- const result = await ctrl
- .createDeployment({
- projectId: input.projectId,
- environmentSlug: input.environmentSlug,
- })
- .catch((err) => {
- console.error(err);
- throw new TRPCError({
- code: "INTERNAL_SERVER_ERROR",
- message: err,
- });
- });
+ const result = await ctrl.createDeployment({
+ projectId: input.projectId,
+ environmentSlug: input.environmentSlug,
+ });📝 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
| const result = await ctrl | |
| .createDeployment({ | |
| projectId: input.projectId, | |
| environmentSlug: input.environmentSlug, | |
| }) | |
| .catch((err) => { | |
| console.error(err); | |
| throw new TRPCError({ | |
| code: "INTERNAL_SERVER_ERROR", | |
| message: err, | |
| }); | |
| const result = await ctrl.createDeployment({ | |
| projectId: input.projectId, | |
| environmentSlug: input.environmentSlug, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/apps/dashboard/lib/trpc/routers/deploy/deployment/create-deploy.ts`
around lines 57 - 67, The inline .catch on the promise returned by
ctrl.createDeployment should be removed so raw Error objects aren't passed into
TRPCError; instead return the awaited result directly from createDeployment and
allow the surrounding try/catch (the outer catch that constructs a sanitized
TRPCError) to handle failures. Locate the call to ctrl.createDeployment in the
create-deploy handler and delete the chained .catch(...) block (including
console.error and the TRPCError construction) so that any thrown error bubbles
to the existing outer catch which already converts unknown errors to a safe
string message.
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
3 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.
What does this PR do?
Adds support for unauthenticated deployments from public GitHub repositories. This enables deployments from public repos without requiring GitHub App installation or authentication.
The changes include:
GetBranchHeadCommitPublicmethod to GitHub client interface and implementations that uses the public GitHub API without authenticationallowUnauthenticatedDeploymentsis enableddeployment.createfor triggering deployments from the frontenddeployment.createaudit log event typeFixes # (issue)
Type of change
How should this be tested?
allowUnauthenticatedDeploymentsis enabledChecklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated