feat(config): serialize credential writes across processes

Stan Hurequested to merge
sh-keyring-write-lock into main

What does this MR do?

Mechanism A of #8476, building on the self-healing refresh (mechanism B), which merged in !3678 (merged).

Note

Depends on mechanism B (!3678 (merged)), now merged to main. This MR is rebased onto main, so its diff is A-only (the internal/config/ lock primitive, merge-fresher, and tests).

Why

Even with B, a long-lived process (for example glab mcp serve) that writes its whole in-memory config for an unrelated reason (an update-check timestamp bump) can still roll back OAuth credentials another process just rotated. For plaintext hosts that is the single-use refresh token itself; the next refresh fails with invalid_grant and the user must re-login (#8390). B does not reach this path because it only intercepts writes that go through the token source, not an unrelated config.Write().

What changed

Adds a cross-process advisory lock (unix.Flock / windows.LockFileEx) on <config-dir>/config.lock and:

  • fileConfig.Write() is now a locked read-merge-write. Before serializing, it re-reads config.yml and, for any host whose on-disk oauth2_expiry_date is newer than the in-memory one, adopts the on-disk OAuth credentials. Only non-empty on-disk values are adopted and only the YAML document is touched, so a keyring-backed host keeps just its fresher expiry and never gains a plaintext token. renameio already made the write atomic; the lock makes the surrounding read-merge-write atomic too.
  • The keyring write in fileConfig.Set() is serialized under the same lock, so login / logout / config set cannot interleave with a refresh (mechanism C).

The OS releases the lock on process exit, so a crashed holder leaves no stale lock (the approach requested on the closed !3501 (closed), over an O_EXCL lock file). If the lock cannot be acquired within a bounded timeout, the write proceeds unlocked rather than failing the command.

Design note

A uses the lock-around-I/O design: the lock is not held across the refresh network call. So B's adopt-on-invalid_grant retry is still needed for the concurrent-refresh race and is left unchanged.

How to test

go test ./internal/config/... -race

Coverage:

  • flock_test.go — a real cross-process lock test (re-executes the test binary, so flock arbitration is exercised between two actual OS processes), plus the timeout/degrade path.
  • write_merge_test.go — reproduces the #8390 clobber and asserts the rotated credentials survive an unrelated stale write, plus keyring-host and don't-regress cases. These dir-backed configs run the real lock code in Write()/Set().
  • Builds on unix and GOOS=windows.

Each mechanism is covered on its own, and the one part that needs real OS processes (the lock) is tested cross-process, so no heavyweight multi-process harness is required.

Related

Edited by Stan Hu

Merge request reports