[aggieben]

Implements the tenant onboarding wizard (STE-47). Multi-step flow from registration through first feed connection and budget setup.

Ready for CTO review.

[aggieben]

CTO Review — REQUEST CHANGES

Three critical issues, three high, five medium. Critical issues block merge.

---

[CRITICAL] Migration number collision — 0025-onboarding.sql duplicates 0025-categories-enhance.sql

0025-categories-enhance.sql already exists on master. A second 0025-onboarding.sql will cause the migration runner to skip or fail. The tenant_onboarding table will never be created in production.

Fix: Rename to 0026-onboarding.sql (or next available number).

---

[CRITICAL] GET /internal/connections/{id}/credentials is unauthenticated

src/BitThicket.Steward.Api/Program.fs — route is registered bare with no requireServiceToken.

This endpoint calls vault.LoadAsync and returns the decrypted Plaid access token. Every other high-value internal endpoint uses a service-token guard. Add the same:

get "/internal/connections/{connectionId:guid}/credentials"
    (AuthHelpers.requireServiceToken internalConnectionCredentialsHandler)

---

[CRITICAL] patchOnboardingHandler overwrites StartedAt on every PATCH

src/BitThicket.Steward.Api/Program.fs:895

StartedAt = DateTimeOffset.UtcNow   // always set to now

upsertAsync uses ON CONFLICT ... DO UPDATE and overwrites started_at unconditionally. Every PATCH resets the onboarding start time. Fix: Read existing record first and preserve the original StartedAt, or exclude started_at from the DO UPDATE SET clause.

---

[HIGH] 0001-onboarding-and-tenant.sql must be deleted

This is a developer test fixture, not a deployable migration. It recreates users and tenants from scratch, uses the wrong GUC namespace (app.current_tenant_id vs steward.tenant_id used everywhere else), and will fail on any DB that ran 0001-baseline.sql. Delete it — the only migration this PR needs is 0026-onboarding.sql.

---

[HIGH] upsertAsync constructs a fake context with UserId = Guid.Empty

src/BitThicket.Steward.Api/OnboardingRepository.fs:751

upsertAsync creates { TenantId = state.TenantId; UserId = Guid.Empty } rather than accepting the caller's real TenantContext. Any audit trigger or user-scoped GUC gets a null identity. Fix: accept TenantContext as a parameter.

---

[HIGH] Auth check is dead code inside requireAuth-wrapped handlers

getOnboardingHandler and patchOnboardingHandler both manually check accessor.Context = None and return 401, but they're already wrapped with AuthHelpers.requireAuth. The inner 401 branch is unreachable and misleading. Remove it.

---

[MEDIUM] onMount race — loadOnboarding never called if auth.loading is true

portal/src/routes/welcome/+page.svelte:248

onMount checks auth.user synchronously. If auth.loading is still true on mount (common on first load), neither branch fires and the user sees a spinner forever. Replace with a $effect that waits for !auth.loading && auth.user.

---

[MEDIUM] Wizard renders steps 3–5 only; steps 1–2 show a blank card

portal/src/routes/welcome/+page.svelte:426

Template has {#if step === 3}, {:else if step === 4}, {:else if step === 5} — nothing for 1 or 2. Progress bar shows 5 bars. Either render those steps or redirect on step < 3.

---

[MEDIUM] openPlaidLink swallows exchange errors — wizard advances on failure

portal/src/routes/welcome/+page.svelte:336

No if (!res.ok) check on the /api/connections/plaid/exchange response. On failure, connectionsCount is set to 1 and the wizard advances to step 4 with no connection created. Check res.ok and surface error via the error state variable.

---

[MEDIUM] skipBudget truncates completedSteps to [1, 2]

portal/src/routes/welcome/+page.svelte:389

Hard-coded completedSteps: [1, 2] drops step 3 if the user connected a bank. Maintain an accumulated completedSteps state variable and pass it instead.

---

[MEDIUM] StubPlaidClient is the only registered IPlaidClient

src/BitThicket.Steward.Ingestion.Plaid/Program.fs:1629

The stub returns canned data in all environments including production. This should be gated with an environment flag rather than silently shipping stub data.

---

Please fix all three critical issues and the 0001 deletion before re-review. High/medium items should also be addressed in this PR.