Skip to content

fix(server): renew managed ChatGPT tokens in active Codex turns - #19895

Closed
Timpan4 wants to merge 3 commits into
pingdotgg:mainfrom
Timpan4:fix/managed-chatgpt-refresh
Closed

Timpan4 wants to merge 3 commits into
pingdotgg:mainfrom
Timpan4:fix/managed-chatgpt-refresh

Conversation

@Timpan4

@Timpan4 Timpan4 commented Oct 15, 2026 •

Copy link
Copy Markdown

Problem

T3 can renew saved managed ChatGPT credentials while an already-running Codex app-server keeps its launch-time bearer. Subsequent conversation requests can continue to fail with an expired token even though Settings has valid saved credentials. Restarting the app-server is not necessary when the installed Codex already supports native auth.command recovery.

Change

  • Replace the static managed bearer environment variable with Codex's native auth.command, using T3's existing Node/Electron/asar/SEA self-invocation conventions. No owned bearer is placed in launch arguments or the child environment.
  • Keep the helper transport-only. A single server-owned private named pipe/Unix socket invokes the existing credential access, cross-process lock and atomic persistence against the captured server store.
  • Let admitted renewal jobs finish if the native helper, turn, socket or provider controller disappears. Normal server-scope shutdown stops admission and drains those jobs before interrupting their fibers.
  • Bind requests to the environment, instance and captured credential profile under the existing lock. Keep native provider configuration profile-specific so shared recursive configuration cannot redirect the owned token.
  • Check both the catalog's captured label profile and its fresh credentials before making catalog HTTP requests, preventing account replacement from mixing an old label with another account's token.
  • Validate a short Unix socket location independently when the ambient temp directory is unsuitable. Contain acquisition failures to managed runtime resolution, before credential rotation, rather than preventing unrelated providers or the whole server from starting.

Scope and approval

Addresses the stale-launch-bearer mechanism in the maintainer triage for #16132 and the expiry/401 reports in #17445. It configures native auth.command, already supported by the pinned Codex 0.156.1, for the existing T3-owned ChatGPT connection. It does not change terminal CLI authentication or claim to fix failed OAuth refresh grants, every 401, or unrelated historical hourly failures.

The SQLite shadow-home fix is independent: #19861. Neither PR depends on the other.

The private broker is necessary because actual native Stop and process-tree teardown can kill a direct credential-writing helper after OAuth accepts a rotation but before the atomic rename. Moving persistence into the existing server fixes that lifetime boundary without adding a daemon, duplicate credential store, public TCP/RPC/MCP endpoint, periodic refresh timer, dependency, installer-pin change or desktop termination-policy change. The four live-provider test edits only supply the required typed service fixture.

The channel has bounded requests, responses, connections and admitted jobs, with immediate overload rejection and no waiting queue. These are local resource-policy choices, not Codex protocol requirements or a promise of recovery under arbitrary load. Windows protection relies on default pipe WRITE/duplex access, not a literally owner-only ACL: read-only peers receive no token bytes. Unix uses a trusted parent, an owned non-symlink 0700 directory and a 0600 socket.

Both requested independent reviewers, GPT-6.1 Sol xhigh and Astra xhigh, approved the complete 21-file feature at b4e4b0dc against main d8e9fd26 through the normal Codex provider. Each inspected every changed hunk, current callers/interfaces and installed dependency semantics, with no necessary findings or omitted files. Beginning and ending integrity checks verified all 21 disk/index/committed hashes and the complete 100,115-byte diff (SHA256 c953e05e584a9437900c9032c852a78a40b3e639f8cc306138f619fe32141392). This is a fresh full-feature review, including all six added files.

The current normal merge integrates main d8e9fd26 at b4e4b0dc. Its only manual conflict resolution retains both the upstream startup-failure fixture and the profile-change catalog regression. Automatic merges preserve upstream workspace-skill scan error propagation, full-CLI stdin handling and server workspace-service wiring. This branch remains independent of the SQLite PR.

Verification

./node_modules/.bin/vp test run \
  apps/server/src/provider/CodexChatGptAuth.test.ts \
  apps/server/src/provider/CodexManagedRuntime.test.ts \
  apps/server/src/provider/CodexManagedTokenBroker.test.ts \
  apps/server/src/provider/CodexManagedTokenProtocol.test.ts \
  apps/server/src/cli/managedChatGptToken.test.ts \
  apps/server/src/provider/Drivers/CodexDriver.test.ts \
  apps/server/src/provider/ProviderInstanceRegistry.test.ts \
  apps/server/src/provider/ProviderRegistry.test.ts \
  apps/server/src/orchestration-v2/AcpRegistryOrchestratorV2.live.test.ts \
  apps/server/src/orchestration-v2/CursorOrchestratorV2.live.test.ts \
  apps/server/src/orchestration-v2/GrokOrchestratorV2.live.test.ts \
  apps/server/src/orchestration-v2/OpenCode2OrchestratorV2.live.test.ts

Current integrated source b4e4b0dc, with frozen upstream dependencies including Effect/platform-node 4.0.5: 176 passed, 30 skipped, one failed across the 12 selected files. The failure is the existing default-Cursor registry fixture's Windows temporary-directory cleanup (rmdir EBUSY), reproduced in isolation and in the independent SQLite checkout where the registry source and test are byte-identical to main and the managed-token feature is absent. The skip count includes platform and live-test exclusions. Server typecheck, all 21 changed-file lint/format checks, scoped Knip exports and feature diff checks passed. This is not a full-suite-green claim.

The earlier 7241aeea integration passed its separate 11-file selection, 145 passed and 17 platform skips, plus server pack. That earlier selection included store, session-lock and self-invocation regressions; it was not rerun solely for this merge. Both listener-failure cases preserve exact diagnostic causes while keeping public message/detail safe.

Coverage includes concurrent rotation, disconnect during OAuth and before rename, scope drain, logout without credential resurrection, reconnect/cancel/controller replacement, profile-switch contention, catalog replacement races, request framing/admission/deadlines and failed listener setup before rotation.

At the earlier 7241aeea integration, the actual-source bundles and packed helper were rebuilt and these disposable proofs with mock OAuth/model endpoints passed. They were not rerun at b4e4b0dc; the current merge is covered by the focused tests and static checks above:

  • On Codex 0.156.1 and 0.162.1, all six Stop/process-tree teardown/reconnect cases passed at an actual server-writer pre-rename barrier. The helper disappeared before the barrier was released; the rotated refresh token persisted, leases/temp files cleared and the consumed refresh token was not replayed.
  • Genuine Node and Electron 44.4.5 with a disposable asar each ran a pinned 0.156.1 app-server that completed the identical started active turn after an initial 401 and renewed success, in the same process/thread/turn without restarting. Captured native children were terminated only after completion. Mock /models 404 diagnostics are not catalog-success evidence; catalog binding is covered by targeted regressions.
  • Actual Windows restricted-token probes on Node and Electron denied duplex/write access and delivered zero token bytes to a read-only peer; an owner request succeeded. The intended boundary is the server's OS user, not an individually authenticated managed child. The default ACL is not owner-only, selectors are not authentication secrets, and unrelated-user/service/remote-SMB identities remain untested.
  • An actual-source Linux harness reproduced the old long-TMPDIR startup failure, then verified protected short fallback, unavailable-parent containment and partial acquisition cleanup. This is not a full Linux Vitest or Linux-native Codex run.

Retained verification limits: the broader settings suite produced 59 passed / one failed dangling-settings-link watcher test, also reproduced with both settings implementation and test byte-identical to main 86534db4 in the independent checkout. A separate earlier registry run has an unchanged Windows EBUSY fixture-cleanup failure reproduced on its original baseline. An unchanged hard-link-alias test failed once in an earlier run, then passed in isolated and full retries; its transient cause is unproven. These failures were not silently fixed or counted as passing runs. This is not a whole-suite-green claim.

Human IPC/auth review and genuinely executed upstream CI are still required. Fork workflow runs with action_required and zero jobs are maintainer approval gates, not passing or failing test executions. Model approvals and automated review success do not establish merge readiness.

No real OAuth, live settings/database changes, UI, macOS or built-SEA execution was performed. The durability guarantee is ordinary consumer cancellation while the server lives. Whole-server kill, power loss and the desktop's unchanged five-second forced termination are outside that guarantee; graceful drain is not unconditional app-exit durability.

Model: GPT-6.1 Sol. Harness: Pi via T3 Code for the initial implementation; Codex via T3 Code for the current main integration. Current independent review models: GPT-6.1 Sol (gpt-6.1-sol) and Astra (gpt-6-astra), both xhigh through the normal Codex provider.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 15, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR not approvable — The change replaces static managed Codex bearer injection with a new server-owned IPC broker and native auth.command flow, spanning token renewal, credential persistence, process lifetime, and profile binding. It also adds production static-analysis suppressions, so the authentication-sensitive and cross-process behavior warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: daee110f-d8ad-45c9-9e45-1f81997387d8
📥 Commits

Reviewing files that changed from the base of the PR and between 7241aee and b4e4b0d.

📒 Files selected for processing: 4
  • apps/server/src/provider/Drivers/CodexDriver.test.ts
  • apps/server/src/provider/ProviderInstanceRegistry.test.ts
  • apps/server/src/provider/ProviderRegistry.test.ts
  • apps/server/src/server.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Managed Codex runtimes now request profile-bound access tokens through a local IPC broker. The change adds the broker protocol, server-side token handling, runtime and CLI integration, and tests for token refresh, credential changes, and broker lifecycle.

Changes: Managed Codex token flow

Layer / File(s) Summary
Define the managed-token protocol
apps/server/src/provider/CodexManagedTokenProtocol.ts, apps/server/src/provider/CodexManagedTokenProtocol.test.ts
Adds versioned request and response schemas, bounded frame handling, and timeout checks.
Serve broker requests
apps/server/src/provider/CodexManagedTokenBroker.ts, apps/server/src/provider/CodexManagedTokenBroker.test.ts, apps/server/src/server.ts, apps/server/src/provider/Provider*Registry.test.ts, apps/server/src/orchestration-v2/*OrchestratorV2.live.test.ts, apps/server/src/provider/Drivers/CodexDriver.ts, apps/server/src/provider/Drivers/CodexDriver.test.ts
Adds the local token broker, listener lifecycle management, request limits, and runtime service wiring. Tests cover broker lifecycle, input validation, permissions, cleanup, overload, and broker service setup.
Bind runtime token access to a profile
apps/server/src/provider/CodexChatGptAuth.ts, apps/server/src/provider/CodexChatGptAuth.test.ts, apps/server/src/provider/CodexManagedRuntime.ts, apps/server/src/provider/CodexManagedRuntime.test.ts, apps/server/src/provider/Drivers/CodexManagedProvider.ts, apps/server/src/provider/Drivers/CodexDriver.test.ts
Adds profile-key checks to auth and provider model checks. Managed runtime launch configuration uses the broker command and removes ACCESS_TOKEN from the child environment. Tests cover profile replacement and launch behavior.
Dispatch the managed-token CLI
apps/server/src/bin.ts, apps/server/src/cli/managedChatGptToken.ts, apps/server/src/cli/managedChatGptToken.test.ts
Dispatches the token command before the full CLI module graph. The command exchanges a framed request with the broker and returns the token or a failure status.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CodexRuntime as Managed Codex runtime
  participant TokenCLI as runManagedChatGptTokenCli
  participant Broker as CodexManagedTokenBroker
  participant Auth as makeCodexChatGptAuth
  CodexRuntime->>TokenCLI: Invoke managed-token command
  TokenCLI->>Broker: Send request with instance, environment, and profile identifiers
  Broker->>Auth: Request access token for the instance and profile
  Auth-->>Broker: Return access token
  Broker-->>TokenCLI: Return token response frame
  TokenCLI-->>CodexRuntime: Write token to stdout
Loading

Possibly related PRs

  • pingdotgg/t3code#17920: Introduced the earlier auth.command flow and loopback token source. This change replaces that source with a local IPC broker.

Suggested labels: macroscope-review

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to b4e4b

The reviewed changes only wire the managed-token broker into the server and test layers. No actionable merge-blocking risk was identified in this scope.

Security architecture risk: 🟡 Moderate · up to b4e4b

The new handoff removes the launch-time bearer and supports renewal during active turns. It also makes local IPC part of credential access. The reviewed controls provide profile binding and restrict access to the local operating-system boundary, but the Windows pipe boundary and cross-instance assumptions warrant further assurance. No new exploitable path was confirmed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A peer able to write to the shared local endpoint and supply valid selectors can ask for the selected instance’s token. The broker does not separately authenticate the peer to that instance; the effective boundary is local OS access plus the saved-profile check, not isolation between processes of the same OS user.

Trust Boundaries and Controls

  • observed — Malformed or excess-property requests are rejected before job admission; an environment mismatch receives no token. Unix directory and socket permissions restrict endpoint access, while the Windows code relies on a different, documented pipe-permission boundary.

Resilience and Maintainability Implications

  • observed — Credential access serializes refresh under its existing lock, saves a successful rotation before replying, and permits an admitted job to finish after its socket disappears. The store writes through a temporary file and rename.

Hardening Proposals

  • proposed — If separate local processes or provider instances are intended to be independent security principals, authenticate that principal at the broker rather than treating a profile digest as authorization. Verify the effective Windows named-pipe access rules with unrelated-principal tests.
Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: renewing managed ChatGPT tokens during active Codex turns.
Description check Passed The description includes all required sections. It explains the stale-token problem, implementation, scope and approval, focused verification results, known failures, and remaining limitations.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@Timpan4

Timpan4 commented Oct 15, 2026

Copy link
Copy Markdown
Author

Clarifying the Windows security-boundary note in the CodeRabbit summary for d45d1cb80755f279a623c692884f077c1ecfb872.

The intended boundary is the server's OS user, not an individually authenticated managed child. Environment/instance/profile identifiers select and bind credentials; they are not caller-authentication secrets. The implementation does not claim a literally owner-only pipe ACL or use an unpredictable pipe name as authorization.

The disposable Windows checks described in the PR's Verification section exercised the actual bundled broker on both Node and Electron. The observed security descriptor granted full access to the owner, Administrators and SYSTEM, and read access to Everyone/Anonymous, consistent with the documented Windows default. An Everyone-restricted token produced:

duplex open: ACCESS_DENIED
read-only connection: allowed
write: ACCESS_DENIED
response bytes: 0
owner, complete valid request: success

Read access is therefore not described as denied. The broker sends nothing on connection and requires a complete valid request before returning a token. The observed write restriction and request-before-response behavior work together; profile binding alone is not the peer boundary.

These are the earlier startup-corrected-source probes, not newly claimed executions following this comment. I rechecked that the published broker, protocol and transport helper are byte-identical to those probe sources, and all 21 published source hashes still match the approved snapshot. The final subsequent correction only preserved a runtime setup error's cause and added its assertions.

A separately launched unrelated-user process, Windows service hosting, and remote-SMB identities were not exercised. Those remain explicit verification limits for human review; the restricted-token result is not presented as coverage of every Windows identity/deployment. No ACL/account changes or new code changes were made in response to this inferred concern. Macroscope's human-approval requirement and the upstream fork-workflow approval gates remain pending.

Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 18, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch codex-09. I will rebuild the token-renewal fix from current main as reviewable broker and runtime-integration layers, trim redundant tests, and retain author credit. Nothing will be merged by this batch.

@juliusmarminge

Copy link
Copy Markdown
Member

Thanks @Timpan4. You identified the stale launch-time bearer correctly and established that native auth.command can recover active turns. The server-owned refresh lifetime and reuse of the existing credential lock/store are retained.

I rebuilt this from current architecture as two reviewable layers:

  • #21786: profile-bound credential access and the private server-owned token broker.
  • #21787: native helper/runtime integration and catalog account-race protection.

I compared your complete original diff with the replacement stack. No required renewal, persistence, listener-failure containment, environment/profile binding, catalog-race, or service-fixture behavior is intentionally omitted. The standalone broker/protocol/helper permutation suites were reduced to seven added behavior cases in existing files. Independent review found that profile-specific provider names break native saved-thread resume after upgrade or sign-in; the replacement keeps the stable provider name and retains binding in the helper/broker. The helper also avoids loading the contracts graph.

The replacement passed 173 focused tests and scoped static checks. Native Codex 0.156.1 completed a synthetic 401→renewal→200 in the same process/thread/turn, and a legacy thread preserved its history across upgrade, broker/runtime restart, and controlled sign-in replacement. Opus 5.5 and GPT-6 Sol reviewed the full stack in two rounds; round two found no code blockers.

Closing this in favor of those unmerged layers. Their bodies preserve attribution and state the pending human auth/IPC, current-head CI, and platform-verification gates; model review does not waive those gates.

Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 19, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 20, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 21, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 21, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 21, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 21, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Pipyakas added a commit to Pipyakas/t3code that referenced this pull request Oct 22, 2026
Managed ChatGPT Codex passed its access token to the shared app-server
once, at launch. T3 renewed the saved token, but the running process kept
the old one, so long turns (and every turn after the hour was up) failed
with 401 "Provided authentication token is expired".

Backport of pingdotgg#19895 (unmerged, head 7241aee): Codex now
fetches the bearer through its native auth.command, which asks a private
T3 socket for a current token, so renewal reaches turns already running.

Fork fixup: the broker tests spied on ESM namespace exports, which vitest
rejects; they now mock node:fs/promises and node:os instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants