[feat] Connect a ChatGPT subscription from the UI and run agents on it - #6674
Conversation
…dence Adds the Codex and Pi authentication research, the product code map, the shared implementation contract with the amendments from the Codex review, the real-provider refresh probe scripts, and the log entries for the first day of the exploration. Claude-Session: https://claude.ai/code/session_01DHWJ9gGjwuYe4d4wnf29DC
A self_managed connection may now carry a slug. The vault resolver selects the subscription_provider secret with that slug, requires a ready login, and emits a runtime_provided connection with a subscription block on the wire. A not-ready login fails before the run with failure_code subscription_login_required, on the invoke error envelope and on the in-stream error frame alike. Login values are masked in dumps and seeded into the per-run redactor. Claude-Session: https://claude.ai/code/session_01DHWJ9gGjwuYe4d4wnf29DC
…n attempts Adds a project-scoped vault secret kind that holds a hosted ChatGPT login, with login state, version, and generation counters. Login attempts start, poll, and cancel a device-code login through the runner. Two runner-facing routes accept a refreshed login (generation, account, refresh token, then expiry) and a failure report that answers stale with the current login when a newer one exists. The login is redacted on every browser read. Claude-Session: https://claude.ai/code/session_01DHWJ9gGjwuYe4d4wnf29DC
…he picker Adds the ChatGPT card on the AI providers page and in the provider drawer, with a device-code login panel (code, copy, open link, countdown, cancel) driven by an attempt poll that stops on a terminal state. A ready subscription secret appears in the model picker as a Subscription row with its own models and slug; a subscription that needs a sign-in shows a disabled row with the reason. The chat error card offers Sign in again for subscription_login_required, read from the 422 envelope and from the in-stream frame, and Try again for subscription_login_refreshed. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
…he cell driver Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
The run request may carry a subscription block with a Pi-format login. The runner keeps one directory per connection on local disk, or on in-VM disk on Daytona, materializes the login under Pi's own lock with a generation-then-expiry rule, and points the Pi subscription branch at that directory. A file watch, or a polling fallback when inotify is exhausted, publishes a refreshed login back to the API as soon as Pi writes it, with a turn-end and session-end push as well. On an authentication failure the runner exchanges the local refresh token through the official client to classify the failure: a transient answer writes the new pair and retries, a stale answer from the API adopts the newer login and replays the turn once when no output was emitted, and a provider refusal reports the failure and ends with subscription_login_required. Device-code login attempts run in the runner behind the runner token and hand the login to the API until it acknowledges with a delete. Codex is out of this slice: Codex 0.145.0 refuses a login file without id_token, which Pi never stores. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
A dev runner that mounts an operator login folder next to a hosted ChatGPT connection showed two identical "ChatGPT" rows in the model picker. The mounted row now reads "ChatGPT (deployment login)"; the hosted row keeps the connection name. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
A poll response that arrives after the user cancelled and started a new attempt could install the cancelled login or clear the new attempt. The store and clear writes now recheck the attempt id inside the row lock and become a no-op when it no longer matches. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
…x reads Two defects from the live cells. The runner published whatever the local login file held, so a corrupt file overwrote the good stored login. Every publish path now checks that the access token is a JWT carrying the account claim that matches the login, before any push. The failure report quotes the version and generation delivered to the run, so a newer stored login is still recognized as stale. Sandbox file reads return bytes, not text; the read-back path decodes them, so a refresh inside a Daytona sandbox is published instead of dropped. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
…oken A pushed login must carry an access token that decodes as a JWT with the account claim of the stored connection, a refresh token, and a future expiry. Anything else answers updated false with reason invalid_login and leaves the row alone. A completed device attempt that hands back an unusable login ends as failed. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
The publisher started with a turn and stopped in its finally, so a refresh that Pi persisted around the end of a turn, or during a park, went unpublished. It now runs from environment acquire to teardown. Every skipped publish logs the moment and the three expiries that decided it. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
… states The provider answers refresh_token_reused first and invalid_refresh_token later for the same dead token. Both now classify as refresh_token_rejected, from an explicit code list rather than a prefix, so the stored login_error is stable. Claude-Session: https://claude.ai/code/session_018bgL2qRpTRn5raKuEN2fuh
|
@coderabbitai review All nine threads are answered above and the head is 0275f3e. Posted by the AI agent (Fable) on behalf of Mahmoud. |
|
|
Corrects the inverted mount-rejection sentence, drops two leftover editing fragments and the empty open-questions heading, points the validator citation at validate.ts, names the Grok vendor xAI, repairs one malformed prompt line, and replaces the routable dev address with a placeholder.
…rsor The subscriptions section can no longer render a border and a label over nothing. A disabled provider row now shows the default cursor, which cursor-pointer on the shared row class was winning by stylesheet order.
…beside the final one
…nged-while-writing
|
@coderabbitai review Head Posted by the AI agent (Fable) on behalf of Mahmoud. |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/runner/src/engines/sandbox_agent/subscription-login/publisher.ts (1)
329-329: 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick winSensitive Data Exposure
Reachability: External
Exploitability: Moderate
CWE: CWE-319 — Cleartext Transmission of Sensitive InformationReject non-HTTPS API bases before sending credentials.
apiBase()acceptshttp://values from environment variables and request-derived telemetry endpoints.pushLoginthen sendsdeps.authorizationto that base. Validate credential-bearing API bases as HTTPS and block credential-bearing redirects.
🧹 Nitpick comments (1)
docs/design/hosted-subscription-connections/research/experiments/reuse_window_probe.py (1)
71-80: 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🔵 Trivial | 💤 Low valueSensitive Data Exposure
Reachability: Internal
Exploitability: Difficult
CWE: CWE-732 — Incorrect Permission Assignment for Critical ResourceSet restrictive permissions before writing token files.
Under a permissive umask, the stash and both write-back files can contain tokens before
os.chmodruns. Use an opener that applies0o600beforejson.dump, and use it for all three writes.🔒 Proposed fix
def secure_open(path, flags): fd = os.open(path, flags, 0o600) os.fchmod(fd, 0o600) return fdPass
opener=secure_opento each write-modeopen()call and write through a context manager. Remove the post-writeos.chmodcalls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: e84e32c1-6d47-4db7-a0c1-6f8844b1e132
📒 Files selected for processing (49)
.agents/skills/agent-release-gate/resources/coverage.md.agents/skills/agent-release-gate/resources/path_triggers.py.gitleaksignoreapi/oss/src/apis/fastapi/vault/models.pyapi/oss/src/apis/fastapi/vault/router.pyapi/oss/src/core/secrets/redaction.pyapi/oss/src/core/secrets/subscription_rules.pyapi/oss/src/core/secrets/subscription_service.pyapi/oss/tests/pytest/unit/providers/test_provider_probe.pyapi/oss/tests/pytest/unit/secrets/test_subscription_login_service.pyapi/oss/tests/pytest/unit/secrets/test_write_only.pyapi/oss/tests/pytest/unit/vault/test_subscription_login_routes.pydocs/design/hosted-subscription-connections/communication-log.mddocs/design/hosted-subscription-connections/fable-prompt.mddocs/design/hosted-subscription-connections/research/experiments/hosted_subscription_cells.pydocs/design/hosted-subscription-connections/research/experiments/refresh_rotation_probe.pydocs/design/hosted-subscription-connections/research/experiments/reuse_window_probe.pydocs/design/hosted-subscription-connections/research/other-providers.mddocs/design/hosted-subscription-connections/research/storage-mounts.mddocs/design/hosted-subscription-connections/status.mddocs/design/hosted-subscription-connections/v0/plan.mddocs/design/hosted-subscription-connections/v0/research.mddocs/design/hosted-subscription-connections/v0/status.mdsdks/python/agenta/sdk/agents/wire_models.pysdks/python/oss/tests/pytest/unit/agents/golden/run_request.subscription_connection.jsonsdks/python/oss/tests/pytest/unit/agents/test_wire_contract.pyservices/runner/src/engines/sandbox_agent/pi-assets.tsservices/runner/src/engines/sandbox_agent/run-plan.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/subscription-login/publisher.tsservices/runner/src/server.tsservices/runner/tests/unit/sandbox-agent-pi-assets.test.tsservices/runner/tests/unit/subscription-login-files.test.tsservices/runner/tests/unit/subscription-login-publish.test.tsservices/runner/tests/unit/subscription-login-routes.test.tsservices/runner/tests/unit/subscription-login-run-boundary.test.tsservices/runner/tests/unit/subscription-recovery-throws.test.tsservices/runner/tests/unit/subscription-recovery.test.tsservices/runner/tests/unit/subscription-run-plan.test.tsservices/runner/tests/unit/wire-contract.test.tsweb/packages/agenta-entities/src/secret/api/loginAttempts.tsweb/packages/agenta-entities/src/secret/core/subscriptionConnections.tsweb/packages/agenta-entities/src/secret/state/subscriptionLogin.tsweb/packages/agenta-entities/tests/unit/subscription-login-attempt.test.tsweb/packages/agenta-entity-ui/src/secretProvider/PlaygroundProviderSections.tsxweb/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsxweb/packages/agenta-entity-ui/src/secretProvider/SubscriptionConnectionCard.tsxweb/packages/agenta-entity-ui/tests/unit/subscriptionPickerRow.test.tsweb/packages/agenta-ui/src/SelectLLMProvider/SelectLLMProviderBase.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- docs/design/hosted-subscription-connections/status.md
- docs/design/hosted-subscription-connections/v0/status.md
- api/oss/src/apis/fastapi/vault/models.py
- web/packages/agenta-entities/tests/unit/subscription-login-attempt.test.ts
- web/packages/agenta-entities/src/secret/state/subscriptionLogin.ts
- docs/design/hosted-subscription-connections/research/experiments/refresh_rotation_probe.py
- docs/design/hosted-subscription-connections/fable-prompt.md
- services/runner/tests/unit/subscription-login-run-boundary.test.ts
- web/packages/agenta-entity-ui/src/secretProvider/SubscriptionConnectionCard.tsx
- web/packages/agenta-entity-ui/tests/unit/subscriptionPickerRow.test.ts
- web/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsx
- docs/design/hosted-subscription-connections/v0/research.md
- web/packages/agenta-entities/src/secret/core/subscriptionConnections.ts
- docs/design/hosted-subscription-connections/research/storage-mounts.md
- web/packages/agenta-ui/src/SelectLLMProvider/SelectLLMProviderBase.tsx
- web/packages/agenta-entities/src/secret/api/loginAttempts.ts
- docs/design/hosted-subscription-connections/v0/plan.md
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review.
Five files conflicted, all additions on both sides at the same place: the release-gate coverage table and path rules (kept both rule sets, and joined the two globs both sides declare into one tuple so neither rule is dropped), the vault router (the starter-credits reconcile import and the subscription exception handler), the secrets service (the managed update and the atomic update), and the SDK connection resolver. The release-added reconcile router test now builds the router with the subscription login service the constructor requires.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.agents/skills/agent-release-gate/resources/coverage.md (1)
302-305: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the path-trigger documentation.
path_triggers.pynow usesPATH_TRIGGERSandPATH_TRIGGER_JOURNEYS. Hosted-subscription changes also require bothH1andH2. The current text can cause an incomplete manual release check.
.agents/skills/agent-release-gate/resources/coverage.md#L302-L305: document the separate mandatory-cell and mandatory-journey mappings..agents/skills/agent-release-gate/resources/coverage.md#L310-L310: listH1andH2and align the path list withHOSTED_SUBSCRIPTION.services/runner/src/engines/sandbox_agent/run-plan.ts (1)
174-174: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject non-string subscription fields.
The
/runJSON body is cast without runtime validation.isUsableSubscriptionsendssubscription.idtoRegExp.test, so numeric or boolean IDs can pass. The accepted ID then reaches localpath.join. It also does not validatesubscription.providerbefore.trim(), so a non-string provider can throw. Validate these types before regex or string operations and returnSUBSCRIPTION_INVALID_MESSAGE.api/oss/src/core/secrets/services.py (1)
550-551: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake subscription-provider uniqueness atomic.
_name_and_slug_subscription_providerchecks existing rows beforeSecretsDAO.createinserts through a separate transaction. The database only enforces unique(project_id, slug), so different caller-provided slugs can persist duplicateSUBSCRIPTION_PROVIDERrows. Always useprovider_kind.valueas the slug, or add a provider-level unique constraint. Translate the resulting unique violation toSubscriptionProviderConflictso the router returns409.services/runner/src/engines/sandbox_agent/environment.ts (1)
1527-1537: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winContain subscription recovery failures.
recoverSubscriptionAuthFailure()can reject during refresh, API reporting, or login-file operations. The rejection exitsacquireEnvironmentOnce()beforeenvironment.destroy(), so cleanup is skipped and the recovery error replaces the classified acquire error. Use the non-throwing recovery wrapper fromrun-turn.tsso cleanup runs and the original classification remains.web/packages/agenta-entity-ui/tests/unit/connectionPicker.test.ts (1)
124-136: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReduce this comment to one short line.
Move the detailed rationale to a design document or issue.
As per coding guidelines, “Hard rule. At most ONE short line per comment.”
Source: Coding guidelines
🧹 Nitpick comments (1)
web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts (1)
46-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce new code comments to one short line.
Replace these explanatory blocks with one short why-comment, or remove self-evident comments.
web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts#L46-L53: reduce the custom-kind comment.web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts#L57-L68: reduce the namespace comment.web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts#L86-L93: reduce the route-family comment.web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts#L105-L120: reduce the route-admission comment.web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts#L132-L138: reduce the route-provider comment.web/packages/agenta-entities/src/secret/core/agentModelCandidates.ts#L358-L361: reduce the custom-policy comment.web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx#L174-L180: reduce the retry-code comment.web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx#L667-L668: reduce the pending-send comment.web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx#L764-L765: reduce the file-only-send comment.As per coding guidelines, “Hard rule. At most ONE short line per comment.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 89b472a9-13c2-43a4-9135-ccd2318f9928
📒 Files selected for processing (26)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/coverage.md.agents/skills/agent-release-gate/resources/path_triggers.pyapi/entrypoints/routers.pyapi/oss/src/apis/fastapi/vault/router.pyapi/oss/src/core/secrets/services.pyapi/oss/tests/pytest/unit/secrets/test_vault_router_reconcile.pydocs/design/hosted-subscription-connections/status.mdsdks/python/agenta/sdk/agents/handler.pysdks/python/agenta/sdk/agents/platform/connections.pysdks/python/agenta/sdk/agents/wire_models.pysdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.pysdks/python/oss/tests/pytest/unit/agents/test_wire_contract.pyservices/runner/docker/Dockerfile.ghservices/runner/src/engines/sandbox_agent/environment.tsservices/runner/src/engines/sandbox_agent/run-plan.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/environment/mount-lifecycle.tsservices/runner/src/protocol.tsservices/runner/tests/unit/wire-contract.test.tsweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/packages/agenta-chat/tests/unit/hooks/useServerSessionInputs.test.tsweb/packages/agenta-entities/src/secret/core/agentModelCandidates.tsweb/packages/agenta-entities/src/secret/core/connections.tsweb/packages/agenta-entity-ui/tests/unit/connectionPicker.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- services/runner/docker/Dockerfile.gh
- docs/design/hosted-subscription-connections/status.md
- web/packages/agenta-entities/src/secret/core/connections.ts
- services/runner/src/engines/sandbox_agent/session-identity.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
Takes the release's version of the session inputs test mock comment; both sides spread the real module and override the same atoms.
2 participants
Context
Until now a ChatGPT subscription only worked on a self-hosted runner where an operator mounted a login folder by hand, and it did not work on Daytona at all. A cloud user had no way to use a ChatGPT plan with an agent.
This PR lets a user sign in to ChatGPT from the AI providers page with a device code, pick the subscription in the model picker like any other connection, and run any number of agents and parallel sessions on it, locally and on Daytona. The runner keeps the login fresh, publishes a renewed token back to the store, and the UI asks for a new sign-in only when the provider has really rejected the login.
Changes
API. A new vault secret kind,
subscription_provider, holds the login inside the encryptedsecretsrow with a version, a generation, and a login state. Three routes start, poll, and cancel a device login through the runner. Two runner-facing routes accept a renewed login and a failure report; both need the runtimesecret-resolvegrant. A pushed login is accepted only when it is a usable token for the same account, the same generation, and a later or equal expiry, so a corrupt file can never overwrite a good login. Public create and update routes reject the server-owned login fields.Before, a subscription had no record at all. After:
The login itself never appears in a read.
SDK. A
self_managedconnection may carry a slug. The resolver reads the subscription secret and delivers the login to the runner in asubscriptionblock onmodelConnection. A not-ready login fails before the run withstatus.failure_code = "subscription_login_required".Runner. Device-code logins run through Pi's own OAuth library behind the runner token. A run materializes the delivered login into one directory per connection (local disk, or in-VM disk on Daytona), and Pi refreshes it under its own lock. One reconciliation loop per session publishes anything the API has not acknowledged, every 5 seconds locally and every 30 seconds on Daytona, and drains on shutdown. On an authentication failure the runner classifies it with a real refresh call: transient failures retry, a rejected refresh token reports the failure and ends with
subscription_login_required, and a newer login in the store is adopted for the next turn. Every decision logs one structured line (event=subscription.materialize|publish|recovery|attempt) with ids and reasons, never a token.Web. A ChatGPT card on the AI providers page and in the provider drawer runs the sign-in: code, copy button, "Open ChatGPT" link, countdown, cancel, and the card flips to Connected on its own. The picker shows the subscription as "ChatGPT · Subscription" with its models, disabled with "Sign in needed" when the login is dead. A mounted operator login now reads "ChatGPT (deployment login)". The chat error card offers "Sign in again" for the new code.
Hosting. The runner gets a
runner-statevolume for the per-connection login copy, because Pi rotates the refresh token on every exchange and the local copy is the only holder until the next publish.Not in this PR. The Codex harness cannot use this login (Codex 0.145.0 needs an
id_tokenthat Pi never stores). Grok and other providers are a follow-up; the design folder has the research.Tests
oss/tests/pytest/unit/secretsandunit/vault, including the attempt state machine, the push ordering rules, the start race, and the grant check on the runner-facing routes.oss/tests/pytest/unit/agents, including the wire contract and the 422 envelope./runover HTTP with a fake API.docs/design/hosted-subscription-connections/status.md.parallel,refresh, anddeadjourneys.CI: every job is green except the web acceptance job, which fails the same way on the other open PRs against this release branch (#6664, #6643) and is unrelated to this change.
Known limits: a pending device login lives in the runner process and dies with a runner restart; with several runner replicas behind one URL a poll can miss the attempt and the card asks to try again. Both are recorded in
status.md.What to QA