Skip to content

codex auth recovery follow-ups (deferred from #402 fix review) #404

Description

@justrach

Deferred minors from the adversarial review of the #402 fix (branch fix/402-codex-auth-hot-reload, merged for v0.0.239). None block the fix; all are real.

  1. Failed persist can strand the only live credential. After a refresh grant succeeds but both persist attempts fail, the fresh token exists only in memory — and the next proactive per-request re-read adopts the older on-disk token, reverting the live session to a dead credential. When persist_error is set, the proactive path should not adopt an on-disk token older than the in-memory one (or the warning should say the on-disk credential is now dead: run graff login codex).
  2. adoptFreshAuth releases the WS latch but keeps the old-bearer socket. The codex WS carries Authorization only in the handshake; on credential change the held socket should be torn down (closeCodexWs) like the reactive path does, so the next request handshakes with the new bearer.
  3. writeCodexAuthAt degrades to a destructive write when the existing auth.json is unreadable. readCodexJson returns null on any failure (torn read while the real codex CLI is mid-write, EIO, over-cap), and the read-modify-write then writes a fresh minimal object — losing the co-owned fields it exists to preserve. Consider one re-read retry and refusing the destructive fallback when the file exists but cannot be parsed.
  4. Two duplicate $CODEX_HOME resolvers survived the one-resolver mandate: learn_credentials.zig:86-88 (reads auth.json!) and imagegen_skill.zig:41-50. Route both through oauth_helpers.codexHomeDir.
  5. Smaller deferred items: no cross-process lock around the refresh (lost race costs one extra POST); an expired bearer failing the WS handshake still burns two transport attempts before the SSE 401 body reaches the recovery arm; the refresh-grant POST itself (rotation, error parsing) is unit-tested only around, not through, the network seam.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions