chore: merge upstream provider-account routing into the deployment line - #3
Merged
Conversation
## Problem `Test (control-plane integration 1/2)` and `2/2` are currently dying on the 10-minute job timeout with no test output on every run — e.g. [run 32274136465](https://github.com/ColeMurray/background-agents/actions/runs/32274136465) (both shards, `The job has exceeded the maximum execution time of 10m0s`). The tests are not the problem. The `Install workerd runtime dependency` step is: ``` Ign:2 http://azure.archive.ubuntu.com/ubuntu noble InRelease ... Get:5 https://archive.ubuntu.com/ubuntu noble-security InRelease [126 kB] 2026-08-19T16:18:06Z ##[error]The operation was canceled. ``` `apt-get update` stalls for ~9 minutes: the runners' Azure mirror ignores every Ubuntu index and the `archive.ubuntu.com` fallback hangs mid-`InRelease`. Third-party repos (`packages.microsoft.com`, `dl.google.com`) fetch fine in the same step, so this is Ubuntu-archive-specific. ## Why removal rather than a retry wrapper `libc++1` is no longer needed. The pinned `workerd` (1.20260617.1) linux-64 binary declares only three `DT_NEEDED` entries: ``` libm.so.6 libc.so.6 ld-linux-x86-64.so.2 ``` (read from the ELF dynamic section of the `@cloudflare/workerd-linux-64@1.20260617.1` npm tarball — Cloudflare links libc++ statically now). The step dates from ColeMurray#73, when it was genuinely required. I first tried hardening the step with bounded, retried `apt-get` calls; that run showed three consecutive 60s `apt-get update` attempts failing and confirmed the runner image does **not** ship libc++, which is what prompted checking whether the dependency still exists at all. ## Verification Applied on a downstream fork of this repo, on the same workflow and the same `workerd` pin, with the step removed: - both integration shards pass — 2m19s and 2m09s, versus 4m22s + timeout before - no other native binary in the integration path links libc++; `@cloudflare/workerd-linux-64` ships only `bin/workerd` Net effect: the shards no longer depend on Ubuntu mirror availability, and each one gets ~2 minutes faster. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Simplified control-plane integration test setup by removing an unnecessary runtime dependency installation step. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - add shared provider-account contracts and model metadata - add D1 account, credential, default, automation, and session-auth persistence - encrypt provider credentials with a dedicated Terraform-managed key - cover schema, stores, atomic writes, and public exports ## Stack Stack 1 of 7. The next branch is `feat/provider-accounts-api`. ## Validation - workspace typecheck - shared: 47 files, 650 tests - focused control-plane: 4 files, 82 tests - focused D1 integration: 4 files, 112 tests - ESLint, Prettier, Terraform fmt, and diff checks at the combined stack head <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added provider account management, including secure credential storage, connection, reconnection, defaults, archiving, and status controls. * Added provider authentication selections for automations and sessions, with validation and persistence. * Added support for securely encrypting provider credentials. * Added model-to-provider resolution for supported subscription providers. * **Bug Fixes** * Improved validation to reject unsupported, duplicate, incomplete, or invalid provider authentication configurations. * **Tests** * Expanded coverage for encryption, account workflows, authentication persistence, automation responses, and deployment configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - add provider adapters, credential exchange, and refresh fencing - add account lifecycle and default-selection services - expose human-only management routes with private no-store responses - retain legacy credential discovery for migration ## Stack Stack 2 of 7. Base: ColeMurray#1524. The next branch is `feat/provider-accounts-runtime`. ## Validation - workspace typecheck - focused control-plane: 7 files, 136 tests - management API integration: 9 tests - combined stack validation is recorded on the final PR <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added model provider account management, including creation, listing, renaming, verification, reconnection, status changes, archiving, and provider defaults. - Added OpenAI and xAI account connections, token refresh, identity validation, and legacy credential discovery. - Added validation for provider accounts, request data, lifecycle states, and default selections. - **Bug Fixes** - Improved concurrent refresh handling and recovery from expired or invalid credentials. - Added safer authentication error responses and private, no-store caching for account routes. - Improved handling of archived, disabled, reconnect-required, and unavailable accounts. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - resolve immutable provider-auth snapshots for sessions and automations - route sandbox access through the trusted account broker - propagate provider auth through child sessions, scheduling, and lifecycle paths - configure managed provider credentials across Modal, OpenComputer, and sandbox-runtime ## Stack Stack 3 of 7. Base: ColeMurray#1525. The next branch is `feat/openai-provider-device-auth`. ## Validation - workspace typecheck - focused control-plane: 6 files, 266 tests - focused integration: 4 files, 48 tests - focused sandbox-runtime: 11 tests - focused Modal environment propagation: 33 tests <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Automations can now pin, update, and clear model-provider accounts. * Sessions and scheduled automation runs now resolve and preserve provider authentication settings. * Added secure provider-account access for sandbox sessions, including legacy OAuth support. * Added shared token brokering for OpenAI and xAI authentication. * **Bug Fixes** * Improved authentication inheritance for child sessions and fail-closed behavior when credentials are unavailable. * Session setup now applies transactional persistence for improved consistency. * **Chores** * Updated sandbox runtime to generation 60 with centralized compatibility checks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - add durable OpenAI device-authorization transactions - encrypt pending authorization state and finalize credentials atomically - handle OpenAI token response variants and reconnect flows - add D1 migration and integration coverage for transaction fencing ## Stack Stack 4 of 7. Base: ColeMurray#1526. The next branch is `feat/xai-provider-device-auth`. ## Validation - workspace typecheck - focused control-plane: 6 files, 54 tests - device-authorization integration: 16 tests - combined stack validation is recorded on the final PR <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added device authorization flows for OpenAI provider accounts, including account creation and reconnection. * Added endpoints to start, poll, and cancel authorization transactions. * Added verification URLs, user codes, status updates, expiry handling, retries, and user-friendly errors. * Added secure authorization-state protection and validation. * Added account lifecycle tracking to support safe reconnects and concurrent authorization attempts. * **Bug Fixes** * Improved OpenAI refresh-error handling and account identity validation. * Prevented sensitive provider response bodies from being exposed. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Upstream ColeMurray#1524-ColeMurray#1527 replace secret-presence inference with explicit per-provider session auth modes, so the fork's OPENAI_API_KEY precedence loop in prepareManagedProviderEnv is dropped in favour of upstream's SessionProviderAuthMode: it also strips a managed provider's API key from the sandbox env instead of merely skipping the marker. The codex auth proxy keeps the fork's spillover and usage-ceiling logic on top of upstream's provider-token-broker refactor, matching the shape already proposed upstream in ColeMurray#1532. The fork's github-review supersession migration is renumbered 0064 -> 0066 because upstream's numbering wins; production's _schema_migrations row needs the matching relabel before the next deploy.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @rhlsthrm, Action: |
rhlsthrm
pushed a commit
that referenced
this pull request
Sep 4, 2026
…ds (ColeMurray#1736) Finding #4 from the control-plane Hono cleanup review: handler wiring uniformity and dead code inside handlers. Pure refactor, no behavior change; both integration snapshots are byte-identical. ## One way to wire a route Every registration now goes through `dispatch(c, handler)` or `dispatchSession(c, handler)`, so a Hono context is unpacked in exactly one place (`routing/admit.ts`). Converted: - 17 inline closures of the form `(c) => handleX(c.var.admitted.request, c.env, c.var.admitted.ctx)` across `automation-crud`, `automation-list`, `automation-slack-settings`, `analytics`, `autofix`, `environments`, `image-builds`, `model-preferences`, `repos`, `secrets`, `session-create`, `session-index`, and `webhooks/{github,slack}`. - 5 inline handler bodies in `audit-events`, `keyboard-shortcuts` (2), `sign-in-providers`, and `health`, each now a named handler. Handlers take the standard `(request, env, params, ctx)` shape, with `_params: object` where the path has no parameters (the spelling `browser-auth.ts` already used; its one `Record<string, never>` is normalized too). This also retires the two-argument shape in `analytics`/`autofix`/`model-preferences` and the one-argument `getModelPreferences`. ``` $ grep -rn "c\.var\.admitted" src --include='*.ts' | grep -v '\.test\.' src/routing/admit.ts:73: return handler(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx); ``` ## Unreachable path-parameter guards deleted (21) `if (!id) return error("... ID required")` on a value that comes from a `:param` segment can never fire: Hono does not match `:param` to an empty segment, and `dispatch` types the parameter `string`. | file | guards | | --- | --- | | `session-diffs.ts` | 4 | | `environments.ts` | 3 | | `environment-secrets.ts` | 3 | | `session-index.ts` | 2 | | `image-builds.ts`, `session-attachments.ts`, `session-child-spawn.ts`, `session-children.ts`, `session-media-upload.ts`, `session-prompt.ts`, `session-pull-requests.ts`, `session-ws-token.ts`, `slack-notify.ts` | 1 each | The `integration-settings.ts` guards that look similar are kept: their `id` is the result of `integrationId(params.id)`, which is null for an unknown integration. One unit test exercised a deleted guard by calling `handlePatchReadState` directly with `{ id: "" }`; it is removed along with the helper parameter that existed only for it. ## No-op admission assertions deleted (3) `admittedAutomation(ctx);` with the result discarded in `automation-crud.ts` (delete) and `automation-lifecycle.ts` (pause, resume). The route policy already guarantees admission resolved the automation. ## Not in this PR Body and query parsing (findings #2 and #3) are separate PRs; the eslint `args` setting (finding #5) is untouched. ## Verification - `tsc -p tsconfig.json`, `-p tsconfig.test.json`, `-p test/integration` - eslint + prettier on all 32 touched files - control-plane unit: 241 files / 3563 tests (one unreachable test removed); integration: 96 files / 1129 tests - `git diff --stat -- test/integration/__snapshots__` empty https://claude.ai/code/session_01KdDpTgGEjXpBA9SaGQVUH1 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved consistency when handling route-based requests across analytics, sessions, automations, environments, secrets, webhooks, and related endpoints. - Requests with valid route parameters now proceed without redundant missing-identifier errors. - Preserved existing response formats, authentication behavior, validation, and health-check results. - **Refactor** - Standardized request handling across control-plane endpoints, improving reliability and maintainability without changing supported routes or core functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
2 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.
Merges
ColeMurray/background-agentsmain (43ceb361) into the deployment line, bringing ColeMurray#1524–ColeMurray#1527 (model provider accounts: data foundation, management API, session routing, OpenAI device authorization).Conflicts resolved
sandbox/managed-provider-env.ts+ testSessionProviderAuthMode(provider_account/legacy_scoped_oauth/api_key) replaces the fork's "anOPENAI_API_KEYsecret disables broker mode" inference from #2, and it additionally deletes a managed provider's API key from the sandbox env rather than merely skipping the marker. The fork's precedence loop and its two tests are dropped.plugins/codex-auth-plugin.jsprovider-token-broker.jsrefactor — the same reconciliation proposed upstream in ColeMurray#1532.router.tsgithubReviewRoutes+modelProviderAccountRoutes).test/integration/cleanup.tsgithub_review_sessions/github_review_state.terraform/d1/migrations/0064. Upstream's numbering wins; the fork'sgithub_review_supersessionmigration moves to0066.Follow-up required before the next deploy
Production's
_schema_migrationsstill records the supersession migration as0064_github_review_supersession.sql.scripts/d1-migrate.shrefuses to run when a recorded filename differs from disk, so the row must be relabelled to0066(oneUPDATE, applied separately from this source sync).Verification
npm run typecheck— 0 errors.npm run test:integration -w @open-inspect/control-plane— 928 pass across 74 files, which is the gate that would have caught the migration collision.npm run test:sandbox-runtime-js— 28 pass.pytest testsinpackages/sandbox-runtime— 753 pass.packages/web— 1083 pass, 1 fail:use-session-rename.test.tsx > keeps the latest overlay until its authoritative title is cached. Pre-existing and unrelated: it fails identically (3/3 runs) on the pre-merge fork lined3d1fc0f, and the test arrived with upstream Centralize session rename mutation ownership ColeMurray/background-agents#1486. CI reports web green, so it looks environment-specific to this machine.