Skip to content

fix(cli): serialize Codex OAuth refresh across processes - #10758

Merged
catrielmuller merged 7 commits into
Kilo-Org:mainfrom
cooper-oai:codex/codex-refresh-process-lock
Jun 17, 2026
Merged

fix(cli): serialize Codex OAuth refresh across processes#10758
catrielmuller merged 7 commits into
Kilo-Org:mainfrom
cooper-oai:codex/codex-refresh-process-lock

Conversation

@cooper-oai

Copy link
Copy Markdown
Contributor

Issue

Fixes #10757

Context

Kilo already coalesces simultaneous Codex OAuth refreshes inside one process and can recover from some cross-process 401 races after another process persists rotated credentials. Separate Kilo processes still had a window where both could submit the same single-use refresh token before either persisted the replacement.

Implementation

Serialize the Codex refresh transaction with Kilo's existing heartbeat-backed Flock utility. The lock is acquired before re-reading stored auth and held through the token POST and rotated credential persistence, so waiters adopt the winner's valid credential without posting the old refresh token again.

The refresh POST now sends User-Agent: kilo/<version> for attribution. The existing in-process pending map and post-401 recovery remain in place as fast paths and defense in depth.

Screenshots / Video

N/A: no visual changes.

How to Test

Manual/local verification

Executed by the agent:

  • bun install --frozen-lockfile
  • cd packages/opencode && bun run typecheck
  • cd packages/opencode && bun test ./test/kilocode/codex-auth-refresh.test.ts ./test/kilocode/oauth-branding.test.ts ./test/plugin/codex.test.ts
    • Passed: 24 tests.
    • Includes five early-401 and five delayed-401 two-process trials sharing one Kilo home. Every trial submitted the starting refresh token exactly once.
    • Includes a delayed token response longer than the injected stale-lock interval to verify heartbeat ownership.
    • Includes a runtime assertion for User-Agent: kilo/<version> on the OAuth refresh POST.
  • cd packages/core && bun test ./test/util/flock.test.ts
    • Passed: 10 tests, including process contention, timeout, crashed-owner recovery, stale cleanup, heartbeat metadata, and compromised-lock handling.
  • bun run script/check-opencode-annotations.ts
  • bunx changeset status --output /tmp/kilo-changeset-status.json
  • bun run lint
    • Completed with zero errors. The repository currently reports existing warnings outside this change.

Reviewer test steps

  1. Run cd packages/opencode && bun test ./test/kilocode/codex-auth-refresh.test.ts ./test/kilocode/oauth-branding.test.ts ./test/plugin/codex.test.ts.
  2. Run cd packages/core && bun test ./test/util/flock.test.ts.
  3. Run cd packages/opencode && bun run typecheck.

Blocked checks and substitute verification

  • None.

Checklist

  • Issue linked above, or exception explained
  • Tests/verification described
  • Screenshots/video included for visual changes, or marked N/A
  • Changeset considered for user-facing changes
  • I personally reviewed the diff and can explain the changes, including any AI-assisted work.

@cooper-oai
cooper-oai marked this pull request as ready for review May 30, 2026 01:52
@kilo-code-bot

kilo-code-bot Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The new commit (f202002c) is a fork-hygiene cleanup: the User-Agent test and the Codex refresh worker fixture have been moved from shared upstream paths (test/fixture/, test/plugin/codex.test.ts) into Kilo-owned paths (test/kilocode/fixture/, test/kilocode/codex-refresh-user-agent.test.ts). Import paths are updated correctly. No logic changes — the test code is identical to what it replaced.

Both previously flagged issues are on files that were removed or had their Kilo-specific content extracted; they are effectively resolved by the relocation.

Files Reviewed (8 files)
  • .changeset/steady-codex-refresh.md
  • packages/opencode/src/kilocode/provider/codex-refresh.ts
  • packages/opencode/src/plugin/codex.ts
  • packages/opencode/test/kilocode/fixture/codex-auth-refresh-worker.ts — moved from test/fixture/, import path corrected
  • packages/opencode/test/kilocode/codex-auth-refresh.test.ts — worker path reference corrected
  • packages/opencode/test/kilocode/codex-refresh-user-agent.test.ts — new, extracted from shared upstream test
  • packages/opencode/test/kilocode/oauth-branding.test.ts
  • packages/opencode/test/plugin/codex.test.ts — User-Agent test removed (now in Kilo-owned file)

Reviewed by claude-sonnet-4.6 · 308,757 tokens

Review guidance: REVIEW.md from base branch main

@cooper-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14cc0e76c3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/opencode/test/plugin/codex.test.ts Outdated
@cooper-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@lambertjosh
lambertjosh requested a review from catrielmuller June 2, 2026 13:25
@kilo-code-bot

kilo-code-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental Review

Commit 64ff1084c0 (fix: fix tests) changes AbortSignal.timeout() to a manual AbortController + setTimeout/clearTimeout in packages/opencode/src/kilocode/provider/codex-refresh.ts:89-94. This is functionally equivalent and likely fixes a test timing issue with AbortSignal.timeout() in Bun's test environment. The finally(() => clearTimeout(timer)) correctly prevents timer leaks on both success and failure paths.

No new issues introduced.

Files Reviewed (8 files)
  • .changeset/steady-codex-refresh.md — Concise user-facing changeset, patch level
  • packages/opencode/src/kilocode/provider/codex-refresh.ts — Core implementation: Flock-based process lock, AbortController timeout, U-A header passing
  • packages/opencode/src/plugin/codex.ts — Adds signal and User-Agent to refreshAccessToken, properly marked with kilocode_change
  • packages/opencode/test/kilocode/codex-auth-refresh.test.ts — 10 cross-process race trials, lock release/retry, timeout abort, stale-lock heartbeat
  • packages/opencode/test/kilocode/codex-refresh-user-agent.test.ts — Confirms U-A header branding on refresh POST
  • packages/opencode/test/kilocode/fixture/codex-auth-refresh-worker.ts — Worker process for cross-process test
  • packages/opencode/test/kilocode/oauth-branding.test.ts — Static assertion for U-A branding in source
  • packages/opencode/test/kilocode/agent-permission-overrides.test.ts — Unrelated test utility rename (WithInstance.provideprovideTestInstance)

Key design points

  • Cross-process serialization: Flock.withLock on a single key ensures only one Kilo process calls the OAuth token endpoint with the current refresh token. Losers read the winning process's persisted credential from disk.
  • Timeout safety: AbortController + setTimeout creates a cancellable timer (via finally(() => clearTimeout(timer))) that prevents a stalled network call from holding the lock indefinitely.
  • Recovery: On 401, the lock holder re-reads stored auth to check if another process already refreshed. Non-401 errors propagate to the caller.
  • Fork hygiene: Shared file changes (codex.ts) are minimal, scoped to kilocode_change markers. All new logic and tests live in Kilo-owned paths.

Minor observation

  • agent-permission-overrides.test.ts changes WithInstance.provideprovideTestInstance on two test cases. This appears to be an unrelated test fixture migration. File is Kilo-owned, no behavioral impact.
Previous Review Summary (commit e9bea70)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e9bea70)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (8 files)
  • .changeset/steady-codex-refresh.md — Concise user-facing changeset, patch level
  • packages/opencode/src/kilocode/provider/codex-refresh.ts — Core implementation: Flock-based process lock, AbortSignal timeout, U-A header passing
  • packages/opencode/src/plugin/codex.ts — Adds signal and User-Agent to refreshAccessToken, properly marked with kilocode_change
  • packages/opencode/test/kilocode/codex-auth-refresh.test.ts — 10 cross-process race trials, lock release/retry, timeout abort, stale-lock heartbeat
  • packages/opencode/test/kilocode/codex-refresh-user-agent.test.ts — Confirms U-A header branding on refresh POST
  • packages/opencode/test/kilocode/fixture/codex-auth-refresh-worker.ts — Worker process for cross-process test
  • packages/opencode/test/kilocode/oauth-branding.test.ts — Static assertion for U-A branding in source
  • packages/opencode/test/kilocode/agent-permission-overrides.test.ts — Unrelated test utility rename (WithInstance.provideprovideTestInstance)

Key design points

  • Cross-process serialization: Flock.withLock on a single key ensures only one Kilo process calls the OAuth token endpoint with the current refresh token. Losers read the winning process's persisted credential from disk.
  • Timeout safety: AbortSignal.timeout (30s default) prevents a stalled network call from holding the lock indefinitely.
  • Recovery: On 401, the lock holder re-reads stored auth to check if another process already refreshed. Non-401 errors propagate to the caller.
  • Fork hygiene: Shared file changes (codex.ts) are minimal, scoped to kilocode_change markers. All new logic and tests live in Kilo-owned paths.

Minor observation

  • agent-permission-overrides.test.ts changes WithInstance.provideprovideTestInstance on two test cases. This appears to be an unrelated test fixture migration. File is Kilo-owned, no behavioral impact.

Reviewed by deepseek-v4-pro-20260423 · 397,244 tokens

Review guidance: REVIEW.md from base branch main

@catrielmuller
catrielmuller merged commit deb5024 into Kilo-Org:main Jun 17, 2026
19 checks passed
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…-process-lock

fix(cli): serialize Codex OAuth refresh across processes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: serialize Codex OAuth refresh across Kilo processes

2 participants