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 servefiring 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_grantand never reachesadopt(). 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
mainwas 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_grantrecovery 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-keychainshows none after ~70 probes across sequential, concurrent, and logout paths. - Logout (7). The keyring delete path moved into the new
syncKeyringhelper, 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.