Skip to content

fix(sandbox): return 409 instead of 500 for tokenless GitHub publish - #5211

Merged
pedrofrxncx merged 1 commit into
mainfrom
fix/publish-github-auth-required-409-w3
Jul 24, 2026
Merged

fix(sandbox): return 409 instead of 500 for tokenless GitHub publish#5211
pedrofrxncx merged 1 commit into
mainfrom
fix/publish-github-auth-required-409-w3

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Bug fix in the sandbox daemon's git/publish route, in the same family as #5189/#5150/#5144/#5175 (the recurring "surface a client/data condition as 4xx instead of a raw 500" pattern in packages/sandbox/daemon/routes/git.ts).

publish() already refuses a tokenless GitHub origin (cloneUrlHasCredentials check) with a clear message, but threw a plain Error, which makeGitPublishHandler's catch-all falls through to a generic 500 — the same opaque-server-error UX the prior fixes in this file eliminated for protected-branch/detached-HEAD refusals. A maintainer wants this because the daemon's own convention (see PublishBlockedError right above this check, used for detached HEAD / protected branch) is that a refusal caused by current repo/config state, not a server fault, should map to 409 so the UI can show an actionable message instead of a scary crash.

Failure scenario: a sandbox whose GitHub origin has no embedded credentials (project not yet connected to GitHub) calls POST /git/publish → daemon returns 500 with no structured error the UI can act on, even though the underlying condition ("connect GitHub and restart the sandbox") is entirely a client-actionable state, not a crash.

Fix: reuse the existing PublishBlockedError (already mapped to 409 in the handler) instead of a bare Error for this refusal — one-line change, no behavior change to the message or control flow. Regression test added: publish route returns 409 (not 500) for a tokenless github origin in git.test.ts, mirroring the existing publish route returns 409 (not 500) for a protected-branch refusal test right above it.

Reviewer command: bun test packages/sandbox/daemon/routes/git.test.ts

Locally ran: bun run fmt, bunx tsc --noEmit (packages/sandbox), and the single targeted test file above (38 pass). Full CI validates the rest.


Summary by cubic

Return 409 Conflict instead of 500 from sandbox git/publish when the GitHub remote has no credentials. This turns a client-actionable state into a clear UI message instead of a generic server error.

  • Bug Fixes
    • Throw PublishBlockedError for tokenless GitHub origins so the route returns 409.
    • Added test to assert 409 and verify the error message.

Written for commit 5591464. Summary will update on new commits.

Review in cubic

@pedrofrxncx
pedrofrxncx enabled auto-merge (squash) July 24, 2026 17:47
@pedrofrxncx
pedrofrxncx merged commit 5640f23 into main Jul 24, 2026
15 checks passed
@pedrofrxncx
pedrofrxncx deleted the fix/publish-github-auth-required-409-w3 branch July 24, 2026 17:52
decocms Bot pushed a commit that referenced this pull request Jul 24, 2026
PR: #5211 fix(sandbox): return 409 instead of 500 for tokenless GitHub publish
Bump type: patch

- @decocms/sandbox (packages/sandbox/package.json): 1.22.6 -> 1.22.7
- deploy/helm/sandbox-env (chart 0.9.25) (deploy/helm/sandbox-env/values.yaml deploy/helm/sandbox-env/Chart.yaml): image.tag/appVersion -> 1.22.7

Deploy-Scope: both
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