openab-agent: multi-vendor OAuth ADR + cross-process auth.json locking (codex + MCP)

[brettchien]

What problem does this solve?

openab-agent's auth.json is a shared, multi-writer credential file with an unlocked read-modify-write, and openab-agent runs one process per Discord thread. So ordinary concurrent usage = concurrent processes refreshing the same OAuth token → refresh-token-rotation reuse → worst case OAuth 2.1 §10.4 token-family revocation = fleet-wide logout. API-key users never hit this; OAuth adoption (PR #1187, Anthropic) is what activates it.

This PR lands the ADR that sets the multi-vendor OAuth pattern (so every future provider stops hand-rolling its own flow) and the concurrency fix for the release-blocker-class storage race — for both the codex tenant and the MCP CredentialStore.

Closes #

Discord Discussion URL: https://discord.com/channels/1490643539682529300/1519372543377539173

At a Glance

auth.json  (multi-tenant: codex / anthropic-oauth / mcp:<server>×N, one process per Discord thread)

 (a) file integrity — ONE global lock          (b) refresh-token rotation — ONE lock per tenant
 ┌─ with_auth_locked(path, f) ──────────┐       ┌─ lock_tenant_refresh(path, tenant) ───────────┐
 │ flock EX  auth.json.global.lock       │       │ try-lock  auth.json.refresh.<tenant>.lock     │
 │ re-read → f(&mut map) → gc_pending →   │       │  + async backoff + 10s timeout (never wedge)  │
 │ atomic tmp+rename                      │       │ held ACROSS the network refresh               │
 └───────────────────────────────────────┘       └───────────────────────────────────────────────┘
   ▲ save_tokens (codex)                            ▲ get_valid_token / force_refresh (codex)
   ▲ McpCredentialStore::save / clear (MCP)         ▲ resolve_oauth_dial → get_access_token (MCP)
   = writers MERGE, never lost-update                = exactly ONE network refresh per tenant; the
                                                        lock-loser re-load()s the winner's token and
                                                        skips its own refresh (no RT_old reuse)

Prior Art & Industry Research

OpenClaw: API keys + subscription OAuth; stores per-profile {access,refresh,expires,accountId} and treats the profile file as a token sink refreshed under a file lock — direct corroboration for the locked-RMW decision.

Hermes Agent (NousResearch): PROVIDER_REGISTRY dataclasses declare each provider's auth type + URLs + env vars behind one resolve_runtime_provider() entry point; auth.json guarded with fcntl/msvcrt file locks — corroborates both the registry shape (OAuthVendor) and the file-lock decision.

Other references:

• Pi (earendil-works/pi) — ported flow for feat(native-agent): Anthropic OAuth (Claude Pro/Max) login for openab-agent #1187; CLAUDE_CODE_OAUTH_TOKEN (#3591); Kiro/Cursor/xAI provider extensions.
• rclone — rcloneEncryptedClientSecret + runtime obscure.MustReveal(): precedent for the ADR §9 Q2 encode-at-rest of a non-confidential bundled OAuth secret (scanner-evasion, explicitly not encryption).
• Antigravity (agy) OAuth ecosystem — GitHub survey 2026-06-24 (≥326 repos hardcode the same client_id/secret; 99/107 sampled commit it plaintext) grounding the agy go/no-go + secret-storage decisions.

Proposed Solution

ADR (docs/adr/openab-agent-oauth.md): a two-axis model — auth (OAuthVendor descriptor + a shared driver on the official oauth2 crate) kept orthogonal to inference (one provider per wire format) — plus a 14-variant vendor feasibility matrix, the CLAUDE_CODE_OAUTH_TOKEN env route, the /auth two-step UX (per-user-keyed PKCE pending state), and the storage invariant below.

Implementation (this PR):

• with_auth_locked — global flock(2) on an auth.json.global.lock sidecar across re-read → mutate → atomic write. All writers funnel through it (codex save_tokens + MCP McpCredentialStore::save/clear), so they merge onto the latest on-disk state instead of lost-updating.
• lock_tenant_refresh — per-tenant flock (non-blocking try + async backoff + 10s timeout) held across the network refresh, with a double-checked re-read, so concurrent processes do exactly one real refresh per tenant. Codex uses it in get_valid_token/force_refresh; MCP uses it in resolve_oauth_dial around get_access_token().
• flock(2) on sidecar files (kernel auto-releases on fd close / process death — no stale lock); libc::flock under cfg(unix), non-unix no-op.
• ADR §7 pending-entry GC: created_at + a 15-min sweep inside with_auth_locked.

Why this approach?

• Two locks, two hazards. The global lock fixes file lost-updates; the per-tenant lock fixes RTR revocation. A single global lock held across the network refresh would serialize one tenant's slow refresh in front of every other tenant (head-of-line blocking MCP servers / Codex), so refresh I/O is held under the per-tenant lock only.
• flock(2) over a sentinel lockfile — the kernel releases it on process death, so a crashed holder never wedges the file (no orphan cleanup / manual timeout).
• MCP single-flight without an rmcp fork. rmcp's CredentialStore exposes no pre-refresh hook, but openab drives the MCP refresh explicitly in resolve_oauth_dial; rmcp's get_access_token re-load()s auth.json and skips the network refresh when the token is already fresh, so the per-tenant lock + rmcp's own fresh-load gives cross-process single-flight with no extra double-check code.
• Known limitation: rustix is not in-tree (the ADR text was optimistic), so this uses libc::flock directly.

Alternatives Considered

• In-process Mutex / tokio single-flight — useless across the per-thread processes.
• Sentinel lockfile (create→delete) — deadlocks if a holder dies; flock(2) auto-releases.
• Refresh outside the lock, re-read on commit (an earlier ADR draft) — rejected: every process has already sent the refresh with the same RT_old before it commits, so RTR still fires.
• Forcing everything through rmcp CredentialStore — lossy; the shared layer sits below both stores (file RMW).

Validation

Rust changes:

• cargo check passes
• cargo test passes (192 passed, 0 failed, 11 ignored) — incl. new with_auth_locked_merges_concurrent_tenants_no_lost_update and with_auth_locked_gcs_stale_pending_but_keeps_fresh_and_tokens
• cargo clippy -- -D warnings clean
• cargo fmt --check clean

All PRs:

• Manual testing — ran the full gate from openab-agent/ (the crate is built standalone, matching ci-openab-agent.yml). Reviewed end-to-end with a second agent across two ADR-review rounds and two code-review rounds; all findings (RTR refresh race, per-user PKCE keying, pending GC, portable WouldBlock, MCP cross-process refresh) addressed and re-verified.

[chaodu-agent]

LGTM ✅ — Well-designed cross-process locking for auth.json with thorough ADR documentation and correct concurrency semantics.

What This PR Does

Introduces a multi-vendor OAuth Architecture Decision Record and implements cross-process auth.json locking to prevent OAuth 2.1 §10.4 token-family revocation caused by concurrent refresh-token rotation across openab-agent's one-process-per-Discord-thread model.

How It Works

Two-lock architecture: (a) a global flock(2) sidecar lock (auth.json.global.lock) serialises all read-modify-write operations to prevent lost updates, and (b) per-tenant refresh locks (auth.json.refresh.<tenant>.lock) serialise network refreshes so only one process presents RT_old per expiry cycle. Both codex save_tokens and MCP McpCredentialStore funnel through the same global lock. The per-tenant lock uses non-blocking try + async backoff with a 10s timeout to avoid wedging.

Findings

#	Severity	Finding	Location
1	🟢	Correct two-lock separation — global for file integrity, per-tenant for refresh serialisation avoids head-of-line blocking	auth.rs:227-400
2	🟢	Double-check pattern after acquiring tenant lock prevents redundant refreshes	auth.rs:530-538
3	🟢	MCP integration leverages rmcp's own re-load() as implicit double-check — elegant zero-code single-flight	mcp/runtime.rs:634-651
4	🟢	Pending-entry GC with created_at timestamp prevents stale PKCE verifier accumulation	auth.rs:309-320
5	🟢	Comprehensive ADR covering prior art, vendor matrix, rejected alternatives, and open questions with clear decisions	docs/adr/openab-agent-oauth.md
6	🟢	Test coverage for both the merge-not-clobber invariant and the GC sweep logic	auth.rs:1306-1372

Finding Details

🟢 F1: Two-lock design

The global lock (with_auth_locked) serialises only the fast in-memory mutate + atomic write — no network I/O inside. The per-tenant lock (lock_tenant_refresh) serialises the slow network refresh per-tenant only. This avoids head-of-line blocking where a stalled refresh for one tenant would block all other writers.

🟢 F2: Double-check after tenant lock acquisition

get_valid_token re-loads and checks expiry after acquiring the tenant lock, so a process that waited while another refreshed skips the redundant network call. Correct single-flight pattern.

🟢 F3: MCP cross-process single-flight without rmcp fork

rmcp's get_access_token re-load()s auth.json and returns early when the token is already fresh. Combined with the per-server tenant lock, this gives cross-process single-flight with no additional double-check code in openab — clean integration.

🟢 F4: Pending-entry GC

gc_stale_pending sweeps abandoned PKCE pending entries (>15 min) on every locked write. created_at = 0 (legacy) is treated as ancient and swept — safe backward compat. Fresh pending entries are preserved.

🟢 F5: ADR quality

The ADR documents: (1) the two-axis auth/inference model, (2) a 14-variant vendor feasibility matrix with concrete OAuth values, (3) the concurrency invariant with pseudocode, (4) prior-art survey (OpenClaw, Hermes, Pi, rclone), (5) rejected alternatives with clear rationale, (6) explicit decisions with owner attribution.

🟢 F6: Test coverage

Two focused tests verify the core invariants: with_auth_locked_merges_concurrent_tenants_no_lost_update confirms the merge-not-clobber semantics, with_auth_locked_gcs_stale_pending_but_keeps_fresh_and_tokens confirms GC correctness.

Baseline Check

• PR opened: 2026-06-24
• Main already has: Unlocked save_tokens + McpCredentialStore RMW, atomic tmp+rename write, PendingPasteLogin struct (without created_at)
• Net-new value: (1) Cross-process file locking via flock(2) sidecar — with_auth_locked funnel + lock_tenant_refresh per-tenant serialisation. (2) Pending-entry GC with TTL. (3) Comprehensive multi-vendor OAuth ADR. (4) CI workflow fix for standalone crate workspace resolution.

What's Good (🟢)

• Two-lock design correctly separates file-integrity (fast, no I/O) from refresh-serialisation (slow, network I/O)
• flock(2) on sidecar files: kernel auto-releases on process death — no stale locks, no orphan cleanup
• clear() acquires lock_global directly rather than using with_auth_locked — correct for delete-on-empty semantics
• WouldBlock handling in lock_tenant_refresh gracefully degrades (proceeds unserialised) rather than failing hard
• Refresh-token splice logic now executes inside the lock, closing the race window
• CI fix (printf '\n[workspace]\n' >> Cargo.toml) mirrors Dockerfile.unified approach — correct standalone crate resolution
• ACP smoke test improvements: captures stderr separately, extends timeout to 30s, adds diagnostics on failure