Skip to content

Fix/ connection duplication, github sso login and some more - #534

Closed
jplorier wants to merge 3 commits into
jonwiggins:mainfrom
jplorier:main
Closed

Fix/ connection duplication, github sso login and some more#534
jplorier wants to merge 3 commits into
jonwiggins:mainfrom
jplorier:main

Conversation

@jplorier

Copy link
Copy Markdown

Summary

Trying to use the app I found multiple issues I'm addressing from my fork.

Changes

Testing

  • Tests pass (pnpm turbo test)
  • Typechecks pass (pnpm turbo typecheck)

Related

Closes #

Screenshots

jplorier and others added 3 commits May 7, 2026 18:30
…irect

- Allow unauthenticated setup wizard requests until first user logs in
  (hasAnyUser gate replaces per-endpoint isSetupComplete bypasses)
- Redirect to /login after setup completion instead of directly into app
- Fix listSecrets and task/workflow/config list queries excluding NULL
  workspace_id rows, making secrets and jobs invisible after login
- Increase API readiness/liveness probe delays to prevent premature restarts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jonwiggins

Copy link
Copy Markdown
Owner

Thanks for taking the time to use Optio in anger and send fixes back — that's genuinely appreciated, and skimming the diff there does look to be real value in here (the connection duplication handling and the SSO login path in particular).

That said, we can't review this as-is: it touches 11 files across five separate areas (DB schema, auth plugin, connection/secret/task-config/task/workflow services, web setup + OAuth callback, and the helm chart) and the PR description template is empty, so we have no way to tell which change fixes which problem, what the observed bugs were, or how any of it was tested.

Could you:

  1. Fill in the PR template — for each fix: what the symptom was, what the root cause was, and how you verified the fix (the "Testing" checklist is unchecked too — pnpm turbo test / pnpm turbo typecheck results would help).
  2. Ideally, split this into one PR per fix (e.g. "fix: connection duplication", "fix: GitHub SSO login", ...). Small scoped PRs with a clear description we can usually review and merge same-day; a multi-area grab-bag we can't.

Note the schema.ts change will also need a Drizzle migration (npx drizzle-kit generate — new migrations use unix-timestamp prefixes), which is another reason to keep that one isolated.

Looking forward to the split-up versions — thanks again for contributing.

@jonwiggins

Copy link
Copy Markdown
Owner

Closing: this bundles several unrelated changes from a main-branch head, and two are blockers — the hasAnyUser() gate fails open (a transient DB error re-opens unauthenticated access to POST /api/secrets and /api/repos on a fully set-up instance), and the nullsNotDistinct schema change ships without a migration. The genuinely live bug it found (seedBuiltInProviders duplicating built-in providers on every restart because ON CONFLICT never fires on NULL workspace_id) was real and has been fixed on main with a dedup migration + partial unique index. Thanks for finding it!

@jonwiggins jonwiggins closed this Aug 8, 2026
jonwiggins added a commit that referenced this pull request Aug 8, 2026
seedBuiltInProviders() upserted with ON CONFLICT (slug, workspace_id),
but built-in providers have NULL workspace_id and the composite unique
constraint treats NULLs as distinct, so the conflict never fired and each
API restart inserted a fresh copy of every built-in provider.

Add a migration that re-points connections at the oldest copy per slug,
deletes the newer duplicates, and creates a partial unique index on
(slug) WHERE workspace_id IS NULL; the seeder now targets that index.

Diagnosed in #534.
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.

2 participants