fix(config): give each keyring probe its own sentinel

What does this MR do?

KeyringAvailable() wrote and deleted a single shared sentinel keychain item, glab:__keyring_probe__. Every concurrent glab process raced that one item.

The macOS backend shells out to security add-generic-password -U, which searches for the item and then creates it as two separate steps. Two processes probing the same name interleave between those steps, and the loser gets errSecDuplicateItem (-25299) on a perfectly healthy keychain.

Because !3759 (merged) wired this probe into the pre-refresh gate, concurrent glab invocations on a keyring-mode host refused to refresh their OAuth token:

Not refreshing the OAuth token for "gitlab.com", because the refreshed
credentials could not be saved: host "gitlab.com" stores credentials in the
operating system keyring, but the keyring is not accepting writes.

The keyring was accepting writes. glab was contending with itself.

Why it mattered

  • It hit the concurrent-agent workload of #8390 — an AI agent or glab mcp serve firing several commands at once.
  • It bypassed the self-healing from !3678 (merged): the probe fails before the token endpoint is called, so the command never receives invalid_grant and never reaches adopt(). Before !3759 (merged) these commands would have raced, lost, and recovered.
  • The message misdirects, reading as a permissions or ACL problem — the theory being pursued in #8390 for sandboxed/agent environments.

What changed

Scope the sentinel name to the process, and to the probe within it. With no shared item there is nothing to race on.

const keyringProbePrefix = "glab:__keyring_probe__"
var keyringProbeSeq atomic.Uint64

func keyringProbeService() string {
	return fmt.Sprintf("%s:%d:%d", keyringProbePrefix, os.Getpid(), keyringProbeSeq.Add(1))
}

A sentinel leaked by a process that died mid-probe is harmless: -U updates an existing entry, so a later probe reusing the name still succeeds. The duplicate error only ever came from the concurrent interleaving, never from a pre-existing item.

Report the backend's own error, so a genuine failure is diagnosable rather than an unexplained refusal. KeyringAvailable() bool stays as a thin wrapper, so login.go needs no change.

How to test

go test -race ./internal/config/

The regression is only reproducible against a real keychain, so it was verified end-to-end against real gitlab.com on macOS 25.6.0: a keyring-backed OAuth login, access token forced expired, then 8 concurrent glab api user per round over 3 rounds, alternating binaries against the same session.

build commands failed of which "keyring is not accepting writes"
main (cf473dc4d) 16 / 24 16
this branch 0 / 24 0

Two things that table shows beyond the headline:

  • Every failure on main was this bug — 16 of 16, not a mix. On a keyring host, concurrent refresh was failing for exactly one reason, and it was not the OAuth race.
  • The fixed build had zero failures of any kind, not merely zero probe failures. With the probe out of the way, the adopt-on-invalid_grant recovery from !3678 (merged) handles the concurrent refresh cleanly and all 8 processes end up with a working token. Previously the probe failed before the token endpoint was reached, so that recovery never ran.

The session was still healthy afterwards, and security dump-keychain shows no leaked sentinel entries.

Manual verification

Full battery on a keyring-backed gitlab.com session, macOS 25.6.0, this branch:

# test result
1 single command, no forced expiry pass
2 glab auth status Logged in to gitlab.com as <user> (keyring)
3 30 sequential forced refreshes 30/30 pass
4 leaked sentinels after those 0
5 32 concurrent glab api user, token forced expired 0/32 failed, 0 probe-related
6 leaked sentinels after 32-way run 0
7 glab auth logout clean; 4 keychain entries → 0

Three of these target what the change itself could plausibly break rather than the bug it fixes:

  • Leaked sentinels (4, 6). Unique names could in principle accumulate keychain entries, and that is the one risk the unit tests cannot cover, since they run against the mock. security dump-keychain shows none after ~70 probes across sequential, concurrent, and logout paths.
  • Logout (7). The keyring delete path moved into the new syncKeyring helper, so it is worth confirming it still removes every entry.
  • 32-way (5). Four times the concurrency of the A/B above, with zero failures of any kind — not merely zero probe failures.
Edited by Jay McCure

Merge request reports

Loading
Loading