From 910315ade81b19988ee836576c37f3ef8391d4fd Mon Sep 17 00:00:00 2001 From: Guillaume Gay Date: Fri, 5 Jun 2026 22:28:46 +0200 Subject: [PATCH 01/31] docs: global provider connections design (PR 1 of Seb #2061 decomposition) --- ...6-05-global-provider-connections-design.md | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-05-global-provider-connections-design.md diff --git a/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md b/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md new file mode 100644 index 0000000000..c3c05712df --- /dev/null +++ b/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md @@ -0,0 +1,111 @@ +# Global Provider Connections — Design (PR 1) + +- **Date:** 2026-06-05 +- **Status:** Draft — awaiting approval +- **Author:** Guillaume (with Claude) +- **Slice of:** Seb's PR #2061 (`coal-borogovia`, "subscription maximization pivot"), decomposed into small PRs. +- **Target branch:** new branch off `upstream/main` (fresh worktree). The existing `feat/global-provider-connections` / PR #2131 worktree is abandoned — it reimplemented the connection layer with a `scope` flag threaded through every OAuth controller and kept breaking. This design replaces that approach. + +--- + +## 1. Motivation + +Today every provider connection is **agent-scoped**: a row in `user_providers` is keyed to one `agent_id`, and the whole connect flow (`POST /api/v1/routing/:agentName/providers`, every OAuth controller, `provider.service`) resolves an agent first. The same subscription/API key connected to three agents is three independent rows. + +Seb's pivot reframes providers as a **user-level resource shared across agents**. This PR ships the first, foundational slice of that: a **global (user-level) connection pool**. Existing agent connections are migrated up into it. Every agent then draws from the shared pool. + +## 2. Goals + +1. Introduce **global provider connections**: `user_providers` rows with `agent_id IS NULL`, owned by the user, managed in a dedicated sidebar area. +2. **Migrate** existing agent-scoped connections into the global pool — **moving** them (one row each, single credential), relabeled by source agent to disambiguate. +3. Reuse the **existing, proven connect components and OAuth flows** (no `scope`-flag rewrite). Connection mechanics are unchanged; only the *target* of a connection becomes "the user" instead of "an agent." +4. Make connected global providers **usable by all of the user's agents** at routing time. +5. Keep the migration **non-destructive and reversible**. + +## 3. Non-goals (explicitly deferred — NOT in this PR) + +- **Per-agent provider scoping / access control** (the `agent_provider_access` junction and the agent "Providers" ON/OFF toggle). **This PR ships without per-agent scoping — every agent can use every global provider.** This is the accepted trade-off; the junction is a *possible* future PR but is **not committed and not part of this work**. +- Provider analytics endpoints, usage charts, sparklines. +- Rate-limit snapshot capture / display. +- Global Playground. +- Custom-provider lifting to user level. +- Disable-impact preview. + +## 4. Accepted consequence + +Because connections are **moved** to a single shared pool and the access junction is **not** added, **every agent uses every connected provider**. There is no per-agent on/off in this PR. This is intentional and approved. + +## 5. Data model + +Single schema change: + +``` +user_providers.agent_id : varchar NOT NULL → varchar NULL +``` + +- `agent_id IS NULL` ⇒ **global** connection (the only kind created going forward). +- The column is **retained** (not dropped) so the migration is reversible and we keep source-agent context during the transition. +- New user-scoped uniqueness: `UNIQUE (user_id, provider, auth_type, LOWER(label))` (replaces the old agent-scoped unique index). + +### Migration `LiftAgentProvidersToGlobal` + +Port Seb's `LiftProvidersToUserLevel` relabel logic, **minus** the `agent_provider_access` junction (out of scope). Steps: + +1. Drop the old agent-scoped unique index `IDX_user_providers_agent_provider_auth_label`. +2. `ALTER COLUMN agent_id DROP NOT NULL`. +3. **Relabel** rows that would collide on the new user-scoped key, **never deleting** (AES-256-GCM keys with random IV can't be proven equal in SQL, so "duplicates" may hold distinct secrets): + - `label = 'Default'` → `` + - custom label → `" - "` + - still-colliding → append a deterministic `[]` suffix. +4. **Move** rows to global: `UPDATE user_providers SET agent_id = NULL`. +5. Create user-scoped unique index `IDX_user_providers_user_provider_auth_label`. +6. `down()`: restore `agent_id` from the original mapping (kept via a temp table captured in `up()`), drop the user-scoped index, recreate the agent-scoped one. + +> **Open item for planning:** Seb captures the agent→provider mapping in the junction. Since we skip the junction, `up()` must stash the original `(id → agent_id)` mapping (temp table or a retained column snapshot) so `down()` can reverse the move. Resolve exact mechanism in the plan. + +## 6. Backend + +### 6.1 Connection management (reuse Seb's user-scoped controller) +- `GET/POST /api/v1/providers` — list/upsert global connections (port `GlobalProvidersController` shape; user-scoped, no `agentName`). +- `PATCH/PUT/DELETE` for rename / reorder / remove keys, user-scoped. +- `provider.service`: the connection path keys on `userId` with `agent_id = NULL` writes. Reuse the existing upsert/rename/reorder/remove logic; the only change is the scope key (user vs agent), not the mechanics. + +### 6.2 OAuth (user-scoped target) +Every subscription OAuth controller (openai, anthropic, gemini, kiro, minimax, xai, copilot device) must accept a **user-scoped** connection target so a subscription can be connected globally. Port Seb's user-scoped versions. **Do not** reintroduce the `#2131` `scope: 'agent' | 'global'` dual-flag — global is the only target now, so the controllers simply key on the authenticated user. + +### 6.3 Routing consumption (the one unavoidable rewire) +Provider reads switch from agent-scoped to the user's global pool: +- `provider.service.getProviders(agentId)` → read the **user's** providers (`user_id = …, agent_id IS NULL`). +- Update the read paths that consume it: proxy provider resolution, tier auto-assign / recalculation, `available-models`. Port Seb's user-scoped versions of these (`user_id`-threaded). +- Routing/tier caches: key by user instead of agent where they hold provider data; invalidate on global connect/disconnect. + +> Each agent, when resolving providers, now sees the user's full global pool. No access filtering (deferred). + +## 7. Frontend + +- **Sidebar:** add the global provider area — Subscriptions / BYOK / Local entries (port `Sidebar` + routes from Seb/#2131: `/providers`, `/providers/subscriptions`, `/providers/byok`, `/providers/local`). +- **Pages:** `GlobalProviders` + `providers/{Subscriptions,Byok,Local}` rendering the category page. Prefer porting **Seb's** working `GlobalProviderCategory` over #2131's (which is the broken 505-line variant) — confirm during planning which is healthier. +- **Connect UI:** reuse existing `ProviderSelectModal`, `ProviderKeyForm`, `OAuthDetailView`, `ProviderDetailView`. Connect calls target the user-scoped endpoints (drop `agentName`); **no `scope` param**. +- **Agent flow:** the per-agent "connect a provider" affordance now points at the **global** connect surface (you connect once, globally). The agent's provider list reflects the shared pool (read-only view of what the agent will use). Exact placement confirmed in planning. + +## 8. Reuse strategy (porting source) + +Primary source is **Seb's #2061** (`upstream/coal-borogovia`, head `c3dd81b0`) — it's the coherent, working version of this design. We take its user-scoped controller/service/OAuth/routing pieces and its migration relabel logic, and we **drop** everything in §3. We do **not** build on #2131's `scope`-flag implementation. + +## 9. Testing + +- **Migration:** unit test the relabel/move on fixtures — collisions (two agents, same provider+`Default`), custom labels, distinct keys preserved (count rows before/after; assert no deletions), `down()` restores `agent_id`. +- **Backend:** global connect (API key + at least one OAuth provider) creates `agent_id IS NULL` rows; list returns user pool; rename/reorder/remove user-scoped; routing for an agent resolves the user's global providers. +- **Frontend:** sidebar entries render; category pages list connected providers; connect flow (reusing existing components) succeeds against user-scoped endpoints; **regression**: connecting a provider actually works end-to-end (the failure that motivated the rebuild). +- **Regression guard:** existing agent that had a provider still routes after migration (now via the global pool). + +## 10. Risks + +- **Credential duplication (avoided):** we **move**, never copy. Copying would put the same rotating OAuth refresh token in two rows and brick accounts on refresh (cf. PR #2113 single-flight fix). Single row per connection eliminates this. +- **OAuth controller blast radius:** every subscription OAuth controller changes. Mitigated by porting Seb's proven user-scoped versions verbatim rather than re-deriving. +- **Routing cache correctness:** switching provider reads to user scope requires matching cache key/invalidation changes; a missed invalidation = stale provider list. Covered by porting Seb's cache split + tests. +- **Migration reversibility without the junction:** `down()` needs the original `agent_id` mapping; must be captured in `up()` (see §5 open item). + +## 11. Out of scope / future (not committed) + +Per-agent access scoping (junction + toggle), analytics/charts, rate-limits, Playground, custom-provider lifting. **No follow-up PR is promised or planned as part of this work.** From 5bb83ec657d64cbd14f395f10a24bb4fd9b58806 Mon Sep 17 00:00:00 2001 From: Guillaume Gay Date: Fri, 5 Jun 2026 22:38:22 +0200 Subject: [PATCH 02/31] docs: refine global providers spec from code map (no scope flag, no agent_id null, slim list endpoint, 100% coverage) --- ...6-05-global-provider-connections-design.md | 76 +++++++++++-------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md b/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md index c3c05712df..25274f0663 100644 --- a/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md +++ b/docs/superpowers/specs/2026-06-05-global-provider-connections-design.md @@ -51,42 +51,51 @@ user_providers.agent_id : varchar NOT NULL → varchar NULL Port Seb's `LiftProvidersToUserLevel` relabel logic, **minus** the `agent_provider_access` junction (out of scope). Steps: +**Refinement (vs. earlier draft): we do NOT null `agent_id`.** "Global" comes from user-scoped reads + a user-scoped unique index, not from emptying `agent_id`. Keeping `agent_id` on lifted rows costs nothing (reads ignore it), keeps the migration trivially reversible, and matches Seb exactly. New connections insert with `agent_id NULL` (the service omits it). This is lower-risk than a bulk null-update. + +`up()` steps (port Seb's `LiftProvidersToUserLevel`, dropping only the two junction statements): 1. Drop the old agent-scoped unique index `IDX_user_providers_agent_provider_auth_label`. -2. `ALTER COLUMN agent_id DROP NOT NULL`. -3. **Relabel** rows that would collide on the new user-scoped key, **never deleting** (AES-256-GCM keys with random IV can't be proven equal in SQL, so "duplicates" may hold distinct secrets): - - `label = 'Default'` → `` - - custom label → `" - "` - - still-colliding → append a deterministic `[]` suffix. -4. **Move** rows to global: `UPDATE user_providers SET agent_id = NULL`. -5. Create user-scoped unique index `IDX_user_providers_user_provider_auth_label`. -6. `down()`: restore `agent_id` from the original mapping (kept via a temp table captured in `up()`), drop the user-scoped index, recreate the agent-scoped one. +2. `ALTER COLUMN agent_id DROP NOT NULL` (so new global rows can omit it). +3. **Relabel** rows that would collide on the new user-scoped key, **never deleting** (AES-256-GCM keys with random IV can't be proven equal in SQL, so "duplicates" may hold distinct secrets): `label='Default'` → ``; custom → `" - "`; still-colliding → deterministic `[]` suffix. (CTE JOINs `agents ON agents.id = user_providers.agent_id`; safe because we never null `agent_id`.) +4. Create user-scoped unique index `IDX_user_providers_user_provider_auth_label` on `(user_id, provider, auth_type, LOWER(label))`. + +**Dropped from Seb's `up()`** (out of scope): the `CREATE TABLE agent_provider_access` + its index, and the `INSERT INTO agent_provider_access … SELECT agent_id, id …` backfill. -> **Open item for planning:** Seb captures the agent→provider mapping in the junction. Since we skip the junction, `up()` must stash the original `(id → agent_id)` mapping (temp table or a retained column snapshot) so `down()` can reverse the move. Resolve exact mechanism in the plan. +`down()` (best-effort, schema-reversing): drop the user-scoped unique index; recreate the agent-scoped one. **Do not** re-add `NOT NULL` — post-migration global rows legitimately have `agent_id NULL`. Documented inline as a standard lossy-down. No junction to drop, no `agent_id` to restore (we never moved it). ## 6. Backend -### 6.1 Connection management (reuse Seb's user-scoped controller) -- `GET/POST /api/v1/providers` — list/upsert global connections (port `GlobalProvidersController` shape; user-scoped, no `agentName`). -- `PATCH/PUT/DELETE` for rename / reorder / remove keys, user-scoped. -- `provider.service`: the connection path keys on `userId` with `agent_id = NULL` writes. Reuse the existing upsert/rename/reorder/remove logic; the only change is the scope key (user vs agent), not the mechanics. +> **Key correction from the code map:** Seb makes connections "global" entirely in the **service layer** — `user_providers` rows are written **user-scoped** (insert omits `agent_id` → NULL) and every read is `user_id`-keyed. The connect **API stays agent-scoped** (`POST /api/v1/routing/:agentName/providers`, and the OAuth controllers keep `agentName`); the resolved agent is used only as *tier-recompute context*, not for row identity. So **we do NOT change the connect API surface or add any `scope` flag** — we re-key the service and thread `user.id` into it. + +### 6.1 Service-layer user re-keying (the core of the PR) +Port Seb's user-scoped versions of (agent_id → user_id reads; `userId` threaded): +`routing-core/provider.service.ts`, `provider-key.service.ts`, `model-discovery/model-discovery.service.ts`, `routing-core/tier-auto-assign.service.ts`, `routing-core/tier.service.ts`, `routing/resolve/resolve.service.ts`, `routing/proxy/proxy.service.ts`, `routing/proxy/proxy-fallback.service.ts`, `routing-core/routing-cache.service.ts` (split agent- vs user-scoped caches + new `invalidateUser`), `routing-core/routing-invalidation.service.ts` (Set→Map), and `oauth/core/oauth-refresh-coordinator.ts` (`oauthRefreshKey` drops the `agentId` segment → `${provider}:${userId}:${label}`). +- New helpers to port: `recalculateTiersForUser(userId)`, `listOwnedAgentIds(userId)`, `afterProviderChange(agentId|null, userId)`. +- **Behavior to keep:** `upsertProviderWithLabel` now *updates in place* on a same-value duplicate key instead of throwing (needed for OAuth reconnect). +- **Connect endpoints (`provider.controller.ts`):** keep routes; swap each service call from `agent.id` → `user.id` (and add `user.id` as the 2nd arg to `removeProvider`/`recalculateTiers`). **Strip** the `AgentProviderAccess` import + optional `accessRepo` + the junction-write block in the connect handler. + +### 6.2 DROP the access-control layer (out of scope, must be stripped on port) +Both copies of `private filterProvidersForAgent(providers, agentId?)` (in `provider-key.service.ts` and `model-discovery.service.ts`), their `@Optional() accessRepo` injections, the trailing optional `agentId?` params on the provider-key/discovery read methods, `invalidateProviderAccess`, and the discovery custom-provider `if (agentId && !customAuthTypes.has(cpKey)) continue;` gate. **Realization:** consumption callers simply **omit `agentId`** so every read returns *all* of the user's providers. Do **not** import the `agent_provider_access` entity/table/controller. -### 6.2 OAuth (user-scoped target) -Every subscription OAuth controller (openai, anthropic, gemini, kiro, minimax, xai, copilot device) must accept a **user-scoped** connection target so a subscription can be connected globally. Port Seb's user-scoped versions. **Do not** reintroduce the `#2131` `scope: 'agent' | 'global'` dual-flag — global is the only target now, so the controllers simply key on the authenticated user. +### 6.3 OAuth controllers (minimal call-site edits) +Controllers/services keep `agentName`/`@Query`; only the downstream call sites change: `getProviderKeys`/`nextOAuthLabel`/`getFreshSubscriptionCredential` take `user.id`; `oauthRefreshKey` drops the `agentId` arg; `removeProvider`/`recalculateTiers` gain `user.id`. Files: `redirect-pkce-oauth.base.ts` (shared), `openai-oauth.controller.ts`, `xai/*`, `gemini-oauth.controller.ts`, `anthropic/*`, `kiro-oauth.*`, `minimax-oauth.*`, `copilot.controller.ts`. **Bug to fix on port:** `copilot.controller.ts` must pass `user.id` (not `agent.id`) to `nextOAuthLabel`. No `scope` flag (the abandoned #2131 approach) anywhere. -### 6.3 Routing consumption (the one unavoidable rewire) -Provider reads switch from agent-scoped to the user's global pool: -- `provider.service.getProviders(agentId)` → read the **user's** providers (`user_id = …, agent_id IS NULL`). -- Update the read paths that consume it: proxy provider resolution, tier auto-assign / recalculation, `available-models`. Port Seb's user-scoped versions of these (`user_id`-threaded). -- Routing/tier caches: key by user instead of agent where they hold provider data; invalidate on global connect/disconnect. +### 6.4 Slim user-scoped list endpoint +Add `GET /api/v1/providers` returning the user's connections for the management pages. Seb's `user-providers.controller.ts` is analytics-heavy (30-day token/cost aggregates, 7-day sparkline) — **out of scope**. Build a **slim** controller that returns connection rows only (id, provider, auth_type, label, key_prefix, priority, region, is_active, connected_at, cached_model_count, models_fetched_at), grouped/shaped for the pages. No `agent_messages`/`Tenant`/pricing deps. -> Each agent, when resolving providers, now sees the user's full global pool. No access filtering (deferred). +### 6.5 available-models filter removal +`model.controller.getAvailableModels` calls `getModelsForAgent(user.id)` (drop the `agent.id` 2nd arg) → returns models for all the user's providers. This is the one explicit per-agent narrowing to remove. + +> Each agent, resolving providers, now sees the user's full global pool. No access filtering (deferred, not promised). ## 7. Frontend -- **Sidebar:** add the global provider area — Subscriptions / BYOK / Local entries (port `Sidebar` + routes from Seb/#2131: `/providers`, `/providers/subscriptions`, `/providers/byok`, `/providers/local`). -- **Pages:** `GlobalProviders` + `providers/{Subscriptions,Byok,Local}` rendering the category page. Prefer porting **Seb's** working `GlobalProviderCategory` over #2131's (which is the broken 505-line variant) — confirm during planning which is healthier. -- **Connect UI:** reuse existing `ProviderSelectModal`, `ProviderKeyForm`, `OAuthDetailView`, `ProviderDetailView`. Connect calls target the user-scoped endpoints (drop `agentName`); **no `scope` param**. -- **Agent flow:** the per-agent "connect a provider" affordance now points at the **global** connect surface (you connect once, globally). The agent's provider list reflects the shared pool (read-only view of what the agent will use). Exact placement confirmed in planning. +> **Correction from the code map:** Seb's frontend connect pages do **not** use a user-scoped connect API or a `scope` flag — they call the existing **agent-scoped** endpoints passing `firstAgentName()`, and the backend (now user-scoped) makes the row global. We follow that: **connect components stay unchanged on the agentName front**, and the list side uses the new slim `GET /api/v1/providers`. Seb's `index.tsx`/`Sidebar.tsx` are on a much newer base and **cannot be cherry-picked wholesale** — we surgically add routes/nav into main's existing structure. + +- **Pages:** add `pages/providers/{Subscriptions,Byok,Local}.tsx` (category hardcoded per file — there is no shared `category=` prop component). Port Seb's pages **stripped** of: usage charts, sparklines, `getOverview`/savings/cost/last-used cells, custom-provider management (the "Add custom provider" button + branch on BYOK), and the row→`ConnectionDetail` navigation. The connect+list core (`connectedRows`/`connectedMap`/`getModelCount` + Supported-providers table + ``) survives cleanly. **Do not** port `ConnectionDetail.tsx` (1115 lines, analytics-saturated). +- **Routes (`index.tsx`):** add `/providers/{subscriptions,byok,local}` + `/providers` → `` (add `Navigate` import). Do **not** add the `ConnectionDetail` route. +- **Sidebar:** add a `PROVIDERS` section (Subscriptions / BYOK / Local) + the `isGlobalActive` helper into main's existing `Sidebar.tsx`. Strip Seb's broader sidebar rewrite (TOOLS/Playground, global Overview/Messages, HARNESSES list). +- **Connect components (small additive props to port):** `ProviderSelectModal` (+`initialTab`), `ProviderSelectContent` (+`initialTab`, deeplink `authType`/`addKey`/`closeOnBack`, `detailBack`), `ProviderDetailView` (+`local` mode), `services/routing-params.ts` `ProviderDeepLink` (+`authType?`,`closeOnBack?`,`addKey?`). Optional: `ProviderKeyForm` focus fix. `OAuthDetailView`, `services/api/oauth.ts`, `services/api/routing.ts` connect fns, `services/providers.ts`, and the provider CSS are **byte-identical to main** — no changes. ## 8. Reuse strategy (porting source) @@ -94,17 +103,20 @@ Primary source is **Seb's #2061** (`upstream/coal-borogovia`, head `c3dd81b0`) ## 9. Testing -- **Migration:** unit test the relabel/move on fixtures — collisions (two agents, same provider+`Default`), custom labels, distinct keys preserved (count rows before/after; assert no deletions), `down()` restores `agent_id`. -- **Backend:** global connect (API key + at least one OAuth provider) creates `agent_id IS NULL` rows; list returns user pool; rename/reorder/remove user-scoped; routing for an agent resolves the user's global providers. -- **Frontend:** sidebar entries render; category pages list connected providers; connect flow (reusing existing components) succeeds against user-scoped endpoints; **regression**: connecting a provider actually works end-to-end (the failure that motivated the rebuild). -- **Regression guard:** existing agent that had a provider still routes after migration (now via the global pool). +**Hard constraint (CLAUDE.md): 100% line coverage on every changed/new file.** Every ported file's re-keyed branches and every new file must be covered. Run `cd packages/backend && npx jest --coverage` and `cd packages/frontend && npx vitest run --coverage` before the PR. + +- **Migration:** unit-test relabel on fixtures — two agents same provider+`Default` (relabel to agent names), custom labels (`" - "`), distinct encrypted keys preserved (row count unchanged; no deletions), user-scoped unique index enforced, new rows insert with `agent_id NULL`. Add a `*.spec.ts` next to the migration (port Seb's spec, drop junction assertions). +- **Backend:** connect (API-key + ≥1 OAuth provider) writes a `user_id`-scoped row; `GET /api/v1/providers` returns the user pool; rename/reorder/remove are user-scoped; routing/`available-models` for an agent resolve the user's full pool (no access filter); `recalculateTiersForUser` fans across owned agents; cache `invalidateUser` clears the user pool. Register the migration in `database.module.ts` + the e2e entities helper if any entity changes. +- **Frontend:** Vitest for each page (connected list renders, Supported-providers table, modal opens with correct `initialTab`); sidebar `PROVIDERS` entries + `isGlobalActive`; component prop additions. **Regression:** connecting a provider works end-to-end (the failure that motivated the rebuild). +- **Regression guard:** an agent that had a provider pre-migration still routes after (now via the global pool). ## 10. Risks - **Credential duplication (avoided):** we **move**, never copy. Copying would put the same rotating OAuth refresh token in two rows and brick accounts on refresh (cf. PR #2113 single-flight fix). Single row per connection eliminates this. -- **OAuth controller blast radius:** every subscription OAuth controller changes. Mitigated by porting Seb's proven user-scoped versions verbatim rather than re-deriving. -- **Routing cache correctness:** switching provider reads to user scope requires matching cache key/invalidation changes; a missed invalidation = stale provider list. Covered by porting Seb's cache split + tests. -- **Migration reversibility without the junction:** `down()` needs the original `agent_id` mapping; must be captured in `up()` (see §5 open item). +- **OAuth controllers:** small call-site edits across all of them; mitigated by porting Seb's proven versions. Includes fixing the `copilot.controller.ts` `nextOAuthLabel(agent.id)` bug → `user.id`. +- **Routing cache correctness:** switching provider reads to user scope requires the cache split (`invalidateUser`) + dual invalidation; a missed invalidation = stale provider list. Covered by porting Seb's cache split + tests. +- **Migration reversibility:** trivial now — we never null `agent_id`, so `down()` only reverses the index swap (documented lossy on `NOT NULL` because post-migration global rows exist). No junction, no mapping to stash. +- **Coverage gate:** the wide file surface must all hit 100% line coverage; budget test time accordingly. ## 11. Out of scope / future (not committed) From be5245ff77cd15e778d8d0474a77e2433443ce5a Mon Sep 17 00:00:00 2001 From: Guillaume Gay Date: Fri, 5 Jun 2026 22:41:01 +0200 Subject: [PATCH 03/31] docs: implementation plan for global provider connections (port of Seb #2061 slice) --- .../2026-06-05-global-provider-connections.md | 486 ++++++++++++++++++ 1 file changed, 486 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-05-global-provider-connections.md diff --git a/docs/superpowers/plans/2026-06-05-global-provider-connections.md b/docs/superpowers/plans/2026-06-05-global-provider-connections.md new file mode 100644 index 0000000000..f9c975c53d --- /dev/null +++ b/docs/superpowers/plans/2026-06-05-global-provider-connections.md @@ -0,0 +1,486 @@ +# Global Provider Connections Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Lift provider connections from agent-scoped to user-scoped (global), so a user connects each provider/subscription once and every one of their agents can use it — shipping the first slice of Seb's PR #2061. + +**Architecture:** Connections become "global" purely in the **service layer**: `user_providers` rows are written user-scoped (insert omits `agent_id`) and every read is `user_id`-keyed. A non-destructive migration relabels existing rows so they're unique per-user. The connect API and OAuth controllers keep `agentName` (used only as tier-recompute context). The per-agent access-control junction, analytics, charts, rate-limits, Playground, and custom-provider lifting from Seb's PR are explicitly **out of scope**. + +**Tech Stack:** NestJS 11 + TypeORM 0.3 + PostgreSQL 16 (backend, Jest), SolidJS + Vite (frontend, Vitest), npm workspaces + Turborepo. **100% line coverage is mandatory.** + +**Reference implementation:** Seb's branch at SHA `c3dd81b0ac6bce0dc4f8af1f81c6f1d889cd25cb` (also `upstream/coal-borogovia`). Read his version of any file with `git show c3dd81b0:`. Diff against current `main` (this worktree, off `upstream/main`) with `git diff 4ff43b83a c3dd81b0 -- `. **Always strip the out-of-scope symbols listed per task** — Seb's base is newer and his files carry the junction/analytics code we are NOT porting. + +**Branch/worktree:** `feat/global-providers` at `…/worktrees/global-providers` (already created off `upstream/main`). The design spec lives at `docs/superpowers/specs/2026-06-05-global-provider-connections-design.md`. + +**Dev DB for tests/e2e:** follow CLAUDE.md — fresh uniquely-named Postgres DB per run via the `postgres_db` Docker container. + +**Out of scope — never import or port these:** `agent_provider_access` entity/table/controller, `provider-analytics.controller.ts`, `ProviderRateLimit`/`rate-limit-tracker.service.ts`, `pages/providers/ConnectionDetail.tsx`, custom-provider user-scope refactor, all chart/sparkline/savings code, `1791100000000-AddProviderRateLimits.ts`, `1791200000000-LiftCustomProvidersToUserLevel.ts`. + +--- + +## Phase ordering & dependencies + +1. **Phase A — Schema + migration** (entity nullable, lift migration). Foundation. +2. **Phase B — Service-layer user re-keying** (the core; large). Depends on A. +3. **Phase C — OAuth controller call-site edits**. Depends on B. +4. **Phase D — Slim `GET /api/v1/providers` list endpoint**. Depends on B. +5. **Phase E — Frontend pages, routes, sidebar, component props**. Depends on D. +6. **Phase F — Integration + e2e + changeset + coverage gate**. Depends on A–E. + +Commit after every task. Run the relevant package's tests after every implementation step. + +--- + +## Phase A — Schema + Migration + +### Task A1: Make `user_providers.agent_id` nullable + +**Files:** +- Modify: `packages/backend/src/entities/user-provider.entity.ts` +- Test: `packages/backend/src/entities/__tests__/user-provider.entity.spec.ts` (create if absent; otherwise add a case) + +- [ ] **Step 1: Write the failing test** — assert the column metadata allows null. + +```ts +// user-provider.entity.spec.ts +import { getMetadataArgsStorage } from 'typeorm'; +import { UserProvider } from '../user-provider.entity'; + +test('agent_id column is nullable', () => { + const col = getMetadataArgsStorage().columns.find( + (c) => c.target === UserProvider && c.propertyName === 'agent_id', + ); + expect(col?.options.nullable).toBe(true); +}); +``` + +- [ ] **Step 2: Run it, expect FAIL** + +Run: `cd packages/backend && npx jest src/entities/__tests__/user-provider.entity.spec.ts` +Expected: FAIL (nullable is currently undefined/false). + +- [ ] **Step 3: Edit the entity** + +```ts +// replace the agent_id column declaration + @Column('varchar', { nullable: true, default: null }) + agent_id!: string | null; +``` + +- [ ] **Step 4: Run it, expect PASS** + +Run: `cd packages/backend && npx jest src/entities/__tests__/user-provider.entity.spec.ts` + +- [ ] **Step 5: Commit** + +```bash +git add packages/backend/src/entities/user-provider.entity.ts packages/backend/src/entities/__tests__/user-provider.entity.spec.ts +git commit -m "feat(providers): make user_providers.agent_id nullable for global connections" +``` + +### Task A2: Lift migration `LiftAgentProvidersToGlobal` + +**Files:** +- Create: `packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.ts` +- Modify: `packages/backend/src/database/database.module.ts` (import + add to `migrations` array) +- Test: `packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts` + +Reference: `git show c3dd81b0:packages/backend/src/database/migrations/1791000000000-LiftProvidersToUserLevel.ts` and its `.spec.ts`. **Port `up()` keeping steps 1, 3, 4, 5, 6 (drop old index → DROP NOT NULL → relabel CTE → create user-scoped unique index). DELETE Seb's step-1 `CREATE TABLE agent_provider_access` (+ its index) and step-2 junction `INSERT`. Do NOT add any `UPDATE … SET agent_id = NULL`.** For `down()`: drop the user-scoped unique index, recreate `IDX_user_providers_agent_provider_auth_label` on `(agent_id, provider, auth_type, LOWER(label))`; **do not** re-add `NOT NULL`; drop Seb's junction-restore `UPDATE` and the `DROP TABLE agent_provider_access`. + +- [ ] **Step 1: Write the failing migration spec** (port + trim Seb's spec). Use a fresh test DB per CLAUDE.md. Cover: (a) two agents with the same provider + `Default` label → after `up()`, both rows survive (count unchanged) and are relabeled to the agents' display names; (b) a custom label `mykey` on an agent named `Bot` → becomes `mykey - Bot` only if it collides, else unchanged; (c) the user-scoped unique index rejects a duplicate `(user_id, provider, auth_type, LOWER(label))`; (d) no rows deleted. + +```ts +// skeleton — fill assertions from Seb's spec, minus junction checks +import { DataSource } from 'typeorm'; +import { LiftAgentProvidersToGlobal1791000000000 } from '../1791000000000-LiftAgentProvidersToGlobal'; +// build a DataSource against a fresh DB, seed colliding user_providers rows, run up(), assert. +``` + +- [ ] **Step 2: Run it, expect FAIL** (migration file does not exist yet) + +Run: `cd packages/backend && npx jest src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts` + +- [ ] **Step 3: Write the migration** — port Seb's relabel CTE verbatim into `up()`, with the junction statements removed as described above. Class name `LiftAgentProvidersToGlobal1791000000000`, `name = 'LiftAgentProvidersToGlobal1791000000000'`. Add an inline comment on `down()` noting the lossy `NOT NULL` reversal. + +- [ ] **Step 4: Register the migration** + +In `packages/backend/src/database/database.module.ts`: add `import { LiftAgentProvidersToGlobal1791000000000 } from './migrations/1791000000000-LiftAgentProvidersToGlobal';` and append it to the `migrations: [...]` array (keep timestamp order). + +- [ ] **Step 5: Run it, expect PASS** + +Run: `cd packages/backend && npx jest src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts` + +- [ ] **Step 6: Commit** + +```bash +git add packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.ts packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts packages/backend/src/database/database.module.ts +git commit -m "feat(providers): add LiftAgentProvidersToGlobal migration (relabel + user-scoped unique index)" +``` + +--- + +## Phase B — Service-layer user re-keying (core) + +> Strategy: port Seb's user-scoped version of each file (reference SHA), applying the exact signature changes below and **stripping** the access-control symbols. Existing `*.spec.ts` files call the old `agentId` signatures and will fail to compile/pass — update them to the new signatures in the same task (Seb's branch has updated specs you can diff: `git show c3dd81b0:`, minus access-control cases). Each task ends green + 100% covered for the touched file. + +### Task B1: `routing-cache.service.ts` — split agent vs user caches + +**Files:** +- Modify: `packages/backend/src/routing/routing-core/routing-cache.service.ts` +- Test: `packages/backend/src/routing/routing-core/__tests__/routing-cache.service.spec.ts` + +Changes (from map): re-key `getProviders/setProviders`, `getCustomProviders/setCustomProviders`, `getProviderKeys/setProviderKeys` from `agentId` → `userId` (providerKeys cache key `${userId}:${provider}:${authType}`). `invalidateAgent(agentId)` must **stop** clearing providers/customProviders/providerKeys (only `tiers`, `specificity`, `headerTiers`, `modelParams` + listeners). Add `invalidateUser(userId)` that deletes `providers[userId]`, `customProviders[userId]`, and every `providerKeys` entry with prefix `${userId}:`. + +- [ ] **Step 1:** Port the test (`git show c3dd81b0:…/routing-cache.service.spec.ts`) — add `invalidateUser` clears the user caches and `invalidateAgent` no longer clears providers. +- [ ] **Step 2:** Run, expect FAIL. `cd packages/backend && npx jest routing-cache.service` +- [ ] **Step 3:** Port the implementation per the changes above. +- [ ] **Step 4:** Run, expect PASS (+ `--coverage` on this file = 100%). +- [ ] **Step 5:** Commit `feat(routing): split routing cache into agent- and user-scoped with invalidateUser`. + +### Task B2: `tier-auto-assign.service.ts` — `recalculate(agentId, userId)` + +**Files:** +- Modify: `packages/backend/src/routing/routing-core/tier-auto-assign.service.ts` +- Test: its `__tests__` spec. + +Change: `recalculate(agentId)` → `recalculate(agentId, userId)`; body calls `discoveryService.getModelsForAgent(userId, agentId)` → **for our slice call `getModelsForAgent(userId)` (no agentId)** so tiers compute from all user providers. + +- [ ] **Step 1:** Update spec to new signature + assert it reads user-scoped models. +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementation (2-arg signature; call `getModelsForAgent(userId)`). +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(routing): thread userId through tier auto-assign`. + +### Task B3: `model-discovery.service.ts` — user-scoped, drop access filter + +**Files:** +- Modify: `packages/backend/src/model-discovery/model-discovery.service.ts` +- Test: its spec. + +Changes: `getModelsForAgent(agentId)` → `getModelsForAgent(userId, agentId?)` (provider reads `where: { user_id: userId, is_active: true }`); `getModelForAgent(userId, model, agentId?)`; `discoverAllForAgent(userId)`; `refreshProvider(userId, …)`. **STRIP:** the `private filterProvidersForAgent(...)`, the `@Optional() accessRepo` injection, `invalidateProviderAccess`, and the `if (agentId && !customAuthTypes.has(cpKey)) continue;` custom gate. Keep the `if (!agentId) return this.fetchModelsForAgent(userId)` early path — our callers won't pass `agentId`, so it returns all user providers. + +- [ ] **Step 1:** Port/trim the spec (drop access-filter cases; assert user-scoped reads). +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementation with strips. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(discovery): user-scope model discovery, drop per-agent access filter`. + +### Task B4: `provider-key.service.ts` — user-scoped, drop access filter + +**Files:** +- Modify: `packages/backend/src/routing/routing-core/provider-key.service.ts` +- Test: its spec. + +Changes: all read methods (`getProviderKeys`, `getDefaultKeyLabel`, `getProviderApiKey`, `getAuthType`, `hasActiveProvider`, `getProviderRegion`, `findProviderForModel`, `getEffectiveModel`, `resolveProviderKeys`, `isModelAvailable`) take `userId` first and query `where: { user_id }`. **STRIP:** the `@Optional() accessRepo`, `private filterProvidersForAgent(...)`, and the trailing optional `agentId?` param on each (callers will not pass it). Cache reads/writes go through the user-keyed `routingCache.getProviderKeys(userId, …)`. + +- [ ] **Step 1:** Port/trim spec to user-scoped signatures, no access cases. +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementation with strips. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(routing): user-scope provider-key lookups, drop access filter`. + +### Task B5: `provider.service.ts` — user-scoped mutations + fan-out helpers + +**Files:** +- Modify: `packages/backend/src/routing/routing-core/provider.service.ts` +- Test: `packages/backend/src/routing/routing-core/__tests__/provider.service.spec.ts` + +Changes (verbatim target signatures from the map): +```ts +recalculateTiers(agentId: string, userId: string): Promise +recalculateTiersForUser(userId: string): Promise // NEW — fans over listOwnedAgentIds +getProviders(userId: string): Promise // where: { user_id } +getFreshSubscriptionCredential(userId: string, provider: string, label?: string): Promise +upsertProvider(agentId: string | null, userId: string, provider: string, apiKey?: string, authType?: AuthType, region?: string, label?: string): Promise<{ provider: UserProvider; isNew: boolean }> +renameKey(agentId: string, userId: string, provider: string, authType: AuthType, currentLabel: string, newLabel: string): Promise +reorderKeys(agentId: string, userId: string, provider: string, authType: AuthType, orderedLabels: string[]): Promise +retagAuthType(agentId: string | null, userId: string, provider: string, nextAuthType: AuthType): Promise +removeProvider(agentId: string | null, userId: string, provider: string, authType?: AuthType, label?: string): Promise<{ notifications: string[] }> +deactivateAllProviders(agentId: string, userId: string): Promise +nextOAuthLabel(userId: string, provider: string): Promise +listOwnedAgentIds(userId: string): Promise // NEW — agents JOIN tenant WHERE tenant.name = userId AND deleted_at IS NULL +``` +Private: `afterProviderChange(agentId: string | null, userId)` (null agentId ⇒ `recalculateTiersForUser`), `renumberPriorities(userId, …)`, `cleanupAgentAfterRemoval(agentId, userId, …)`. **Inserts omit `agent_id`** (so new rows are NULL). **Keep** the `upsertProviderWithLabel` in-place-update-on-duplicate behavior (OAuth reconnect). New constructor dep: `@InjectRepository(Agent) agentRepo`. Mutations call **both** `routingCache.invalidateAgent(agentId)` and `routingCache.invalidateUser(userId)`. **STRIP** any access-control references. + +- [ ] **Step 1:** Port/trim the spec (large) to the new signatures; add cases: `getProviders` reads by user; insert sets `agent_id` NULL; `removeProvider` fans cleanup across owned agents; `recalculateTiersForUser` iterates `listOwnedAgentIds`. +- [ ] **Step 2:** Run, expect FAIL. `cd packages/backend && npx jest provider.service` +- [ ] **Step 3:** Port the implementation per signatures above. +- [ ] **Step 4:** Run, expect PASS + coverage 100% on the file. +- [ ] **Step 5:** Commit `feat(routing): user-scope provider service with cross-agent fan-out`. + +### Task B6: `tier.service.ts` + `resolve/resolve.service.ts` — thread userId + +**Files:** +- Modify: `packages/backend/src/routing/routing-core/tier.service.ts`, `packages/backend/src/routing/resolve/resolve.service.ts` +- Test: their specs. + +Changes: `resolve`, `resolveForTier`, `resolveHeaderTier`, `resolveSpecificity`, `buildResolvedRoute`, `enrichRouteKeyLabel`, `resolveProviderForModel` gain `userId`; downstream calls become `isModelAvailable(userId, model)`, `hasActiveProvider(userId, prefix)`, `getAuthType(userId, …)`, `getDefaultKeyLabel(userId, …)`, `getModelForAgent(userId, model)` — **omit `agentId`** (no access filter). `tier.service.ts` provider reads → `getProviders(userId)`. + +- [ ] **Step 1:** Port/trim specs to new signatures. +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementations. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(routing): user-scope tier + resolve services`. + +### Task B7: `proxy.service.ts` + `proxy-fallback.service.ts` — thread userId (strip rate-limit) + +**Files:** +- Modify: `packages/backend/src/routing/proxy/proxy.service.ts`, `packages/backend/src/routing/proxy/proxy-fallback.service.ts` +- Test: their specs. + +Changes: pass `userId` into `resolveRouting`/credential lookups; calls become `getProviderApiKey(userId, provider, authType, label)` / `getProviderRegion(userId, …)` and the fallback equivalents — **omit the trailing `agentId`**. Keep the `proxy-fallback` custom-provider lookup `user_id` scoping (security). **STRIP:** `RateLimitTrackerService` injection + the `captureFromResponse(...)` block in `proxy.service.ts` (out of scope) and any related `proxy.module.ts` wiring. + +- [ ] **Step 1:** Port/trim specs; assert no rate-limit capture, user-scoped credential reads. +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementations with rate-limit strip. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(proxy): user-scope credential resolution`. + +### Task B8: `routing-invalidation.service.ts` + `oauth-refresh-coordinator.ts` + +**Files:** +- Modify: `packages/backend/src/routing/routing-core/routing-invalidation.service.ts`, `packages/backend/src/routing/oauth/core/oauth-refresh-coordinator.ts` +- Test: their specs. + +Changes: invalidation collects `Map` (from `tier.user_id`) and calls `autoAssign.recalculate(agentId, userId)`. `oauthRefreshKey(providerId, userId, agentId, label?)` → `oauthRefreshKey(providerId, userId, label?)` returning `${provider}:${userId}:${label}`. + +- [ ] **Step 1:** Port/trim specs. +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementations. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(routing): user-keyed invalidation + oauth refresh key`. + +### Task B9: `provider.controller.ts` + `model.controller.ts` + module wiring + +**Files:** +- Modify: `packages/backend/src/routing/provider.controller.ts`, `packages/backend/src/routing/model.controller.ts`, `packages/backend/src/routing/routing.module.ts`, `packages/backend/src/routing/routing-core/routing-core.module.ts` +- Test: controller specs. + +Changes: in `provider.controller.ts` swap each service call `agent.id` → `user.id` and add `user.id` to `removeProvider`/`recalculateTiers`. **STRIP** the `AgentProviderAccess` import + `@Optional() accessRepo` + junction-write block. In `model.controller.ts` `getAvailableModels`: call `getModelsForAgent(user.id)` (drop `agent.id`); `refreshModels` → `discoverAllForAgent(user.id)` + `recalculateTiers(agent.id, user.id)`. In `routing-core.module.ts`: add `Agent` to `forFeature` if not already present (provider.service injects it); **do not** add `AgentProviderAccess`. In `routing.module.ts`: do **not** register `AgentProviderAccessController` or the analytics controllers. + +- [ ] **Step 1:** Port/trim controller specs (no junction assertions; user-scoped calls; available-models returns all user providers). +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Port implementations + module edits. +- [ ] **Step 4:** Run, expect PASS + coverage. Then `cd packages/backend && npx tsc --noEmit` to confirm the whole package compiles after the re-keying. +- [ ] **Step 5:** Commit `feat(routing): user-scope provider + model controllers`. + +--- + +## Phase C — OAuth controller call-site edits + +> One task per provider; each is a tiny, mechanical edit. Reference Seb's version per file. For every controller/service the handler signatures and `agentName` query stay the same — only the downstream calls change. + +### Task C1: Shared `redirect-pkce-oauth.base.ts` + +**Files:** Modify `packages/backend/src/routing/oauth/redirect-pkce-oauth.base.ts`; Test its spec. + +Changes: `nextOAuthLabel(pending.userId)`; `recalculateTiers(pending.agentId, pending.userId)`; `oauthRefreshKey(providerId, userId, keyLabel)` (drop agentId); `getFreshSubscriptionCredential(pending.userId, …)`. `PendingState` already has `agentId` + `userId`. + +- [ ] **Step 1:** Update spec. **Step 2:** Run→FAIL. **Step 3:** Apply edits. **Step 4:** Run→PASS + coverage. **Step 5:** Commit `feat(oauth): user-scope redirect-pkce base`. + +### Task C2: OpenAI + xAI + +**Files:** Modify `oauth/openai-oauth.controller.ts`, `oauth/xai/xai-oauth.controller.ts`, `oauth/xai/xai-oauth.service.ts`; Tests. + +Changes: controllers `revoke`: `getProviderKeys(user.id, …)` + `removeProvider(agent.id, user.id, …)`. `xai-oauth.service.ts`: `nextOAuthLabel(pending.userId)`, `recalculateTiers(pending.agentId, pending.userId)`, `oauthRefreshKey('xai', userId, keyLabel)`, `getFreshSubscriptionCredential(userId, …)`. + +- [ ] Steps 1-5 as in C1. Commit `feat(oauth): user-scope openai + xai`. + +### Task C3: Gemini + +**Files:** Modify `oauth/gemini-oauth.controller.ts`; Test. + +Changes: `getProviderKeys(user.id, …)`, `removeProvider(agent.id, user.id, …)`. + +- [ ] Steps 1-5. Commit `feat(oauth): user-scope gemini`. + +### Task C4: Anthropic + +**Files:** Modify `oauth/anthropic/anthropic-oauth.controller.ts`, `oauth/anthropic/anthropic-oauth.service.ts`; Tests. + +Changes: controller disconnect `removeProvider(agent.id, user.id, 'anthropic', 'subscription', keyLabel)`. Service: `nextOAuthLabel(pending.userId, PROVIDER)`, `recalculateTiers(pending.agentId, pending.userId)`, `oauthRefreshKey('anthropic', userId, keyLabel)`, `getFreshSubscriptionCredential(userId, …)`. + +- [ ] Steps 1-5. Commit `feat(oauth): user-scope anthropic`. + +### Task C5: Kiro + MiniMax + +**Files:** Modify `oauth/kiro-oauth.controller.ts`, `oauth/kiro-oauth.service.ts`, `oauth/minimax-oauth.controller.ts`, `oauth/minimax-oauth.service.ts`; Tests. + +Changes: controllers `removeProvider(agent.id, user.id, …)`. Services: `nextOAuthLabel(pending.userId, '')`, `recalculateTiers(pending.agentId, pending.userId)`, `oauthRefreshKey('', userId, keyLabel)`, `getFreshSubscriptionCredential(userId, …)`. + +- [ ] Steps 1-5. Commit `feat(oauth): user-scope kiro + minimax`. + +### Task C6: Copilot device flow (+ bug fix) + +**Files:** Modify `packages/backend/src/routing/copilot.controller.ts`; Test. + +Changes: `recalculateTiers(agent.id, user.id)` AND **fix the bug** — `nextOAuthLabel(user.id, 'copilot')` (Seb left it as `agent.id`, which now resolves zero rows). + +- [ ] **Step 1:** Add a spec asserting `nextOAuthLabel` is called with `user.id`. **Step 2:** Run→FAIL. **Step 3:** Apply edits. **Step 4:** Run→PASS + coverage. **Step 5:** Commit `fix(oauth): copilot nextOAuthLabel must be user-scoped`. + +--- + +## Phase D — Slim `GET /api/v1/providers` list endpoint + +### Task D1: `GlobalProvidersController` (slim, connections only) + +**Files:** +- Create: `packages/backend/src/routing/global-providers.controller.ts` +- Modify: `packages/backend/src/routing/routing.module.ts` (register controller) +- Test: `packages/backend/src/routing/global-providers.controller.spec.ts` + +Do **not** port Seb's analytics-heavy `user-providers.controller.ts`. Build a slim controller: + +```ts +import { Controller, Get } from '@nestjs/common'; +import { CurrentUser } from '../auth/current-user.decorator'; +import type { AuthUser } from '../auth/auth.instance'; +import { ProviderService } from './routing-core/provider.service'; + +@Controller('api/v1/providers') +export class GlobalProvidersController { + constructor(private readonly providerService: ProviderService) {} + + @Get() + async list(@CurrentUser() user: AuthUser) { + const providers = await this.providerService.getProviders(user.id); + return providers.map((p) => ({ + id: p.id, + provider: p.provider, + auth_type: p.auth_type ?? 'api_key', + is_active: p.is_active, + has_api_key: !!p.api_key_encrypted, + key_prefix: p.key_prefix ?? null, + label: p.label, + priority: p.priority, + region: p.region ?? null, + connected_at: p.connected_at, + models_fetched_at: p.models_fetched_at ?? null, + cached_model_count: Array.isArray(p.cached_models) ? p.cached_models.length : 0, + })); + } +} +``` + +- [ ] **Step 1:** Write the spec — mock `ProviderService.getProviders` returning two user rows; assert the projected shape and that `getProviders` is called with `user.id`. +- [ ] **Step 2:** Run, expect FAIL. `cd packages/backend && npx jest global-providers.controller` +- [ ] **Step 3:** Create the controller (above) and register it in `routing.module.ts` controllers array. +- [ ] **Step 4:** Run, expect PASS + 100% coverage. +- [ ] **Step 5:** Commit `feat(providers): add slim GET /api/v1/providers list endpoint`. + +--- + +## Phase E — Frontend + +### Task E1: Connect-component prop additions + +**Files:** +- Modify: `packages/frontend/src/services/routing-params.ts` (extend `ProviderDeepLink` with `authType?`, `closeOnBack?`, `addKey?`) +- Modify: `packages/frontend/src/components/ProviderSelectModal.tsx` (+`initialTab?: 'subscription'|'api_key'|'local'`) +- Modify: `packages/frontend/src/components/ProviderSelectContent.tsx` (+`initialTab`, read `deepLink.authType/addKey/closeOnBack`, thread `detailBack`) +- Modify: `packages/frontend/src/components/ProviderDetailView.tsx` (+`local` connected/mode branch + header restyle) +- Test: the matching `tests/components/*.test.tsx` + +Reference each via `git show c3dd81b0:`. These are additive (existing behavior unchanged when the new props are absent). + +- [ ] **Step 1:** Port/extend the component tests asserting: modal forwards `initialTab` to content; content defaults its tab from `initialTab`; deeplink `authType` selects the detail auth type; `ProviderDetailView` shows the local branch when `provider.localOnly`. +- [ ] **Step 2:** Run, expect FAIL. `cd packages/frontend && npx vitest run ProviderSelect` +- [ ] **Step 3:** Port the prop additions. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(frontend): add initialTab + deeplink fields to provider connect components`. + +### Task E2: Three provider pages (stripped) + +**Files:** +- Create: `packages/frontend/src/pages/providers/Subscriptions.tsx`, `Byok.tsx`, `Local.tsx` +- Test: `packages/frontend/tests/pages/providers/{Subscriptions,Byok,Local}.test.tsx` + +Port each from `git show c3dd81b0:packages/frontend/src/pages/providers/.tsx`, **stripping** every item in the spec §7 strip-list: `Sparkline`/`InfoTooltip`/`getOverview` imports, all chart-card/savings/cost/last-used memos + table cells, and (Byok) the custom-provider button/branch/`getCustomProviders`. Keep: `createResource(fetchJson('/providers'))`, `getAgents()`→`firstAgentName()`, the connected-list table (Provider / Models / Name / Status), the Supported-providers list with per-provider connection count + Add button, and ``. Drop row→`/providers/connections/:id` navigation (no ConnectionDetail). Each page's connected `interface` drops `consumption_*`/`sparkline_7d`/`last_used_at`. + +- [ ] **Step 1:** Write Vitest tests per page: mocks `fetchJson('/providers')` → 2 connections; asserts the connected table renders provider + model count + status, the Supported-providers Add button opens the modal with the right `initialTab`, and that **no** chart/sparkline/savings text is present. +- [ ] **Step 2:** Run, expect FAIL. +- [ ] **Step 3:** Create the three stripped pages. +- [ ] **Step 4:** Run, expect PASS + 100% coverage. +- [ ] **Step 5:** Commit `feat(frontend): add Subscriptions/BYOK/Local global provider pages`. + +### Task E3: Routes + Sidebar + +**Files:** +- Modify: `packages/frontend/src/index.tsx` (lazy imports + 3 routes + `/providers`→Navigate; import `Navigate` from `@solidjs/router`) +- Modify: `packages/frontend/src/components/Sidebar.tsx` (PROVIDERS section + `isGlobalActive` helper) +- Test: `packages/frontend/tests/components/Sidebar.test.tsx` (+ a routing test if present) + +```tsx +// index.tsx — additions +const Subscriptions = lazyReload(() => import('./pages/providers/Subscriptions.jsx')); +const Byok = lazyReload(() => import('./pages/providers/Byok.jsx')); +const LocalProviders = lazyReload(() => import('./pages/providers/Local.jsx')); +// inside : + + + + } /> +``` + +```tsx +// Sidebar.tsx — PROVIDERS section (place per existing sidebar structure) +
PROVIDERS
+Subscriptions +BYOK +Local +``` +Add `const isGlobalActive = (route: string) => location.pathname === route;` (match Seb's helper). + +- [ ] **Step 1:** Update Sidebar test asserting the 3 PROVIDERS links render with correct hrefs and active state. +- [ ] **Step 2:** Run, expect FAIL. `cd packages/frontend && npx vitest run Sidebar` +- [ ] **Step 3:** Apply route + sidebar edits. +- [ ] **Step 4:** Run, expect PASS + coverage. +- [ ] **Step 5:** Commit `feat(frontend): add global provider routes + sidebar section`. + +--- + +## Phase F — Integration, e2e, changeset, coverage gate + +### Task F1: Backend e2e — connect once, all agents see it + +**Files:** +- Create/Modify: `packages/backend/test/global-providers.e2e-spec.ts` +- Modify (if needed): `packages/backend/test/helpers.ts` (entities array — only if an entity was added; none added here beyond the modified `UserProvider`, so likely no change — verify) + +- [ ] **Step 1:** Write an e2e (supertest, fresh DB): create user + two agents; connect an API-key provider via `POST /api/v1/routing/:agentA/providers`; assert `GET /api/v1/providers` returns it; assert `GET /api/v1/routing/:agentB/available-models` includes that provider's models (proves the global pool reaches a *different* agent). +- [ ] **Step 2:** Run, expect FAIL (if any wiring gap). `cd packages/backend && npm run test:e2e` +- [ ] **Step 3:** Fix wiring surfaced by the e2e. +- [ ] **Step 4:** Run, expect PASS. +- [ ] **Step 5:** Commit `test(providers): e2e global connection visible across agents`. + +### Task F2: Full coverage + typecheck + lint gate + +- [ ] **Step 1:** `cd packages/backend && npx jest --coverage` → 100% lines on all changed files. Fix gaps. +- [ ] **Step 2:** `cd packages/frontend && npx vitest run --coverage` → 100%. Fix gaps. +- [ ] **Step 3:** `npx tsc --noEmit` in backend and frontend; `npm run lint` at root. Fix. +- [ ] **Step 4:** Commit any test/coverage additions `test(providers): close coverage gaps`. + +### Task F3: Changeset + manual smoke + +**Files:** Create `.changeset/global-provider-connections.md`. + +```md +--- +'manifest': minor +--- + +Add global (user-level) provider connections: connect each provider or subscription once and every one of your agents can use it. Existing agent-scoped connections are migrated up to the global pool (relabeled per source agent, never duplicated). New Subscriptions / BYOK / Local pages in the sidebar manage connections. +``` + +- [ ] **Step 1:** Add the changeset (target `manifest`). +- [ ] **Step 2:** Manual smoke per CLAUDE.md (fresh cloud DB, `SEED_DATA=true`): start backend + frontend, open the Subscriptions page, connect an API-key provider, confirm it appears and that a second agent's routing sees its models. (This is the failure that motivated the rebuild — confirm it now works.) +- [ ] **Step 3:** Commit `chore: changeset for global provider connections`. +- [ ] **Step 4:** Open the PR against `upstream/main`. + +--- + +## Self-review notes (author) + +- **Spec coverage:** A (schema+migration §5) ✓; B (service re-keying §6.1, access-strip §6.2, available-models §6.5) ✓; C (OAuth §6.3 + copilot fix) ✓; D (slim list §6.4) ✓; E (frontend §7) ✓; F (testing §9, changeset §Release) ✓. Non-goals (§3) are enforced by explicit strip lists in B/C/E and the "never import" header. +- **Type consistency:** signatures for `recalculateTiers(agentId, userId)`, `getProviders(userId)`, `removeProvider(agentId, userId, …)`, `nextOAuthLabel(userId, provider)`, `oauthRefreshKey(provider, userId, label?)`, `getModelsForAgent(userId, agentId?)` are used identically across B, C, D. +- **Known residual to verify during execution:** (a) whether `routing-core.module.ts` already registers `Agent` (map said yes) — confirm before adding; (b) whether `test/helpers.ts` needs changes (no new entity, so likely not) — confirm in F1; (c) exact placement of the PROVIDERS section within main's current `Sidebar.tsx` structure. From 9b60db067f80902b90aa797f635cc19cd4efd7bb Mon Sep 17 00:00:00 2001 From: Guillaume Gay Date: Fri, 5 Jun 2026 22:45:41 +0200 Subject: [PATCH 04/31] feat(providers): make user_providers.agent_id nullable for global connections --- .../backend/src/entities/user-provider.entity.spec.ts | 8 ++++++++ packages/backend/src/entities/user-provider.entity.ts | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/entities/user-provider.entity.spec.ts b/packages/backend/src/entities/user-provider.entity.spec.ts index 55740003d2..c330ccca18 100644 --- a/packages/backend/src/entities/user-provider.entity.spec.ts +++ b/packages/backend/src/entities/user-provider.entity.spec.ts @@ -1,6 +1,14 @@ +import { getMetadataArgsStorage } from 'typeorm'; import { UserProvider } from './user-provider.entity'; describe('UserProvider entity', () => { + it('agent_id column is nullable', () => { + const col = getMetadataArgsStorage().columns.find( + (c) => c.target === UserProvider && c.propertyName === 'agent_id', + ); + expect(col?.options.nullable).toBe(true); + }); + it('should instantiate with all fields assignable', () => { const entity = new UserProvider(); entity.id = 'p1'; diff --git a/packages/backend/src/entities/user-provider.entity.ts b/packages/backend/src/entities/user-provider.entity.ts index 3b5e9b1b2f..604c39ad4a 100644 --- a/packages/backend/src/entities/user-provider.entity.ts +++ b/packages/backend/src/entities/user-provider.entity.ts @@ -11,8 +11,8 @@ export class UserProvider { @Column('varchar') user_id!: string; - @Column('varchar') - agent_id!: string; + @Column('varchar', { nullable: true, default: null }) + agent_id!: string | null; @Column('varchar') provider!: string; From b8b431351f0f6303657d0559bdb1dce35dad20dc Mon Sep 17 00:00:00 2001 From: Guillaume Gay Date: Fri, 5 Jun 2026 22:50:40 +0200 Subject: [PATCH 05/31] feat(providers): add LiftAgentProvidersToGlobal migration (relabel + user-scoped unique index) --- .../backend/src/database/database.module.ts | 2 + ...0000000-LiftAgentProvidersToGlobal.spec.ts | 225 ++++++++++++++++++ ...791000000000-LiftAgentProvidersToGlobal.ts | 184 ++++++++++++++ 3 files changed, 411 insertions(+) create mode 100644 packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts create mode 100644 packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.ts diff --git a/packages/backend/src/database/database.module.ts b/packages/backend/src/database/database.module.ts index 5b2e472644..808789dede 100644 --- a/packages/backend/src/database/database.module.ts +++ b/packages/backend/src/database/database.module.ts @@ -112,6 +112,7 @@ import { AddReasoningContentCache1790100000000 } from './migrations/179010000000 import { AddDedupCompositeIndex1790200000000 } from './migrations/1790200000000-AddDedupCompositeIndex'; import { AddErrorsPartialIndex1790300000000 } from './migrations/1790300000000-AddErrorsPartialIndex'; import { DropRedundantAgentApiKeyPrefixIndex1790400000000 } from './migrations/1790400000000-DropRedundantAgentApiKeyPrefixIndex'; +import { LiftAgentProvidersToGlobal1791000000000 } from './migrations/1791000000000-LiftAgentProvidersToGlobal'; const entities = [ AgentMessage, @@ -225,6 +226,7 @@ const migrations = [ AddDedupCompositeIndex1790200000000, AddErrorsPartialIndex1790300000000, DropRedundantAgentApiKeyPrefixIndex1790400000000, + LiftAgentProvidersToGlobal1791000000000, ]; @Module({ diff --git a/packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts b/packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts new file mode 100644 index 0000000000..ef40e4ac9c --- /dev/null +++ b/packages/backend/src/database/migrations/1791000000000-LiftAgentProvidersToGlobal.spec.ts @@ -0,0 +1,225 @@ +import { Client } from 'pg'; +import { DataSource, QueryRunner } from 'typeorm'; +import { LiftAgentProvidersToGlobal1791000000000 } from './1791000000000-LiftAgentProvidersToGlobal'; + +/** + * This spec executes the real migration against a live PostgreSQL database so + * that both up() and down() are exercised end-to-end. It creates a throwaway + * database, builds the minimal schema the migration touches (user_providers + + * agents), seeds collision scenarios, and asserts on the resulting rows and + * indexes via pg_indexes. + */ + +const ADMIN_URL = + process.env['MIGRATION_DATABASE_URL'] ?? + process.env['DATABASE_URL'] ?? + 'postgresql://myuser:mypassword@localhost:5432/postgres'; + +function baseUrlFor(dbName: string): string { + const url = new URL(ADMIN_URL); + url.pathname = `/${dbName}`; + return url.toString(); +} + +const OLD_INDEX = 'IDX_user_providers_agent_provider_auth_label'; +const NEW_INDEX = 'IDX_user_providers_user_provider_auth_label'; + +describe('LiftAgentProvidersToGlobal1791000000000 (live DB)', () => { + const migration = new LiftAgentProvidersToGlobal1791000000000(); + const dbName = `manifest_mig_${Date.now()}_${Math.floor(Math.random() * 1e6)}`; + let dataSource: DataSource; + let runner: QueryRunner; + + async function indexExists(name: string): Promise { + const rows: Array<{ indexname: string }> = await runner.query( + `SELECT indexname FROM pg_indexes WHERE tablename = 'user_providers' AND indexname = $1`, + [name], + ); + return rows.length === 1; + } + + async function seedSchema(): Promise { + await runner.query(` + CREATE TABLE "agents" ( + "id" varchar PRIMARY KEY, + "name" varchar, + "display_name" varchar + ) + `); + await runner.query(` + CREATE TABLE "user_providers" ( + "id" varchar PRIMARY KEY, + "user_id" varchar NOT NULL, + "agent_id" varchar NOT NULL, + "provider" varchar NOT NULL, + "auth_type" varchar NOT NULL DEFAULT 'api_key', + "label" varchar NOT NULL DEFAULT 'Default', + "priority" integer NOT NULL DEFAULT 0, + "connected_at" timestamp NOT NULL DEFAULT now() + ) + `); + // The old agent-scoped unique index that up() must drop. + await runner.query(` + CREATE UNIQUE INDEX "${OLD_INDEX}" + ON "user_providers" ("agent_id", "provider", "auth_type", LOWER("label")) + `); + } + + beforeAll(async () => { + const admin = new Client({ connectionString: ADMIN_URL }); + await admin.connect(); + await admin.query(`CREATE DATABASE "${dbName}"`); + await admin.end(); + + dataSource = new DataSource({ type: 'postgres', url: baseUrlFor(dbName) }); + await dataSource.initialize(); + runner = dataSource.createQueryRunner(); + await seedSchema(); + }); + + afterAll(async () => { + if (runner) await runner.release(); + if (dataSource?.isInitialized) await dataSource.destroy(); + const admin = new Client({ connectionString: ADMIN_URL }); + await admin.connect(); + // Terminate stragglers then drop the throwaway DB. + await admin.query( + `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = $1 AND pid <> pg_backend_pid()`, + [dbName], + ); + await admin.query(`DROP DATABASE IF EXISTS "${dbName}"`); + await admin.end(); + }); + + beforeEach(async () => { + await runner.query(`DELETE FROM "user_providers"`); + await runner.query(`DELETE FROM "agents"`); + // Reset the schema to the pre-up() baseline (old agent-scoped index present, + // new user-scoped index absent) so each test starts from the same state. + await runner.query(`DROP INDEX IF EXISTS "${NEW_INDEX}"`); + await runner.query(`DROP INDEX IF EXISTS "${OLD_INDEX}"`); + await runner.query(` + CREATE UNIQUE INDEX "${OLD_INDEX}" + ON "user_providers" ("agent_id", "provider", "auth_type", LOWER("label")) + `); + }); + + it('relabels colliding Default rows to agent display names, keeps every row, and swaps the index', async () => { + await runner.query( + `INSERT INTO "agents" ("id", "name", "display_name") VALUES ($1, $2, $3), ($4, $5, $6)`, + ['agent-1', 'agent-one', 'Sales Bot', 'agent-2', 'agent-two', 'Support Bot'], + ); + await runner.query( + `INSERT INTO "user_providers" ("id", "user_id", "agent_id", "provider", "auth_type", "label") + VALUES ($1, $2, $3, $4, $5, $6), ($7, $8, $9, $10, $11, $12)`, + [ + 'up-1', + 'user-1', + 'agent-1', + 'openai', + 'api_key', + 'Default', + 'up-2', + 'user-1', + 'agent-2', + 'openai', + 'api_key', + 'Default', + ], + ); + + const before: Array<{ c: string }> = await runner.query( + `SELECT COUNT(*)::text AS c FROM "user_providers"`, + ); + + await migration.up(runner); + + const after: Array<{ c: string }> = await runner.query( + `SELECT COUNT(*)::text AS c FROM "user_providers"`, + ); + // No rows deleted. + expect(after[0].c).toBe(before[0].c); + expect(after[0].c).toBe('2'); + + const rows: Array<{ id: string; label: string; agent_id: string | null }> = await runner.query( + `SELECT "id", "label", "agent_id" FROM "user_providers" ORDER BY "id"`, + ); + const labels = rows.map((r) => r.label).sort(); + expect(labels).toEqual(['Sales Bot', 'Support Bot']); + // agent_id is never nulled by this migration. + expect(rows.every((r) => r.agent_id !== null)).toBe(true); + + expect(await indexExists(NEW_INDEX)).toBe(true); + expect(await indexExists(OLD_INDEX)).toBe(false); + }); + + it('relabels a colliding custom label as "