Buffer remote command output in the watch daemon

[jesseworld22]

2 of 4 — splitting #548. Base: main — #551 has landed, and this is rebased on #543 (the internal/tui → internal/ui collapse).

Summary

The watch daemon owns a registered command's output stream, so output outlives the process that submitted it — for a hook-driven run, that process exits immediately.

• Per-command buffer capped in bytes, keeping the tail and reporting truncation.
• Registration is best-effort and never starts the daemon: it sits on the hook path in front of a command the developer is waiting on.
• chunk validate --remote and chunk sidecar exec both register through one helper.
• Buffers are in memory; a daemon restart loses them.
• The daemon's CircleCI client is resolved once by the _daemon subcommand and passed into RunDaemon, rather than resolved lazily inside the daemon behind a mutex.

Carries the fixes for all four review threads on #548.

Credential resolution

The first cut resolved the client lazily inside the daemon, behind a mutex, retrying at most once every 30s. The daemon is detached with no controlling terminal, so it can never approve a keychain prompt — lazy or eager, an unauthenticated machine gets ErrNeedsAuth and no output either way. The machinery only bought cost: a lock wrapping keychain I/O on the registration path, in front of a hook the developer is waiting on, under a 2s RegisterCommand timeout.

Resolution now happens once, up front, and the client (which may be nil) is passed in. Nothing new was needed to support the unauthenticated case — outputStore.register already treats a nil streamFn as "record the command, stream nothing", and Snapshot.AuthError already carries the reason. With no credentials mutex left, snapshot reads authError under the existing project lock and the lock-ordering note goes away.

Resolving once means a daemon holds its client — nil included — for its whole life, so chunk auth login would otherwise have no effect on one already running. EnsureRunning reuses a reachable daemon of the same build, so re-running chunk watch would not have helped either. Storing a CircleCI token now stops a running daemon (watchd.StopForCredentialChange, best-effort and silent): a chunk watch already on screen relaunches it through EnsureLaunched on its next poll, and otherwise the next chunk watch starts one that can authenticate.

Token reload on 401 (FACT-460)

A client captured its token at construction and kept it for life, so a process outliving a login never saw the new credential. On a 401 the client now re-reads the stored credential and retries once if it changed. Opt-in via httpcl.Config.ReloadToken, threaded through circleci.Config, wired in authprompt.ResolveCircleCIClient. Since the SSE stream goes through Call, StreamOutput inherits it.

Deliberately a reload, not a refresh: internal/oauth requests grant_type=authorization_code only, stores no refresh token, and discards expires_in, so there is no grant to call. This covers "logged in elsewhere, this process holds a stale token"; it cannot cover "token aged out with nobody re-authenticating".

Review notes:

• Bounded to one retry and gated on the token actually changing, so a revoked token costs one extra read rather than one per request.
• Concurrent 401s collapse into a single reload — the token is behind a mutex and later arrivals see it has already moved. A keychain read is a process spawn, and the daemon can 401 from several stream goroutines at once.
• Request bodies are retained rather than streamed once, since the first attempt drains the reader.
• ValidateCircleCIToken deliberately does not set ReloadToken: it validates a token the user just typed, and reloading could "validate" a different stored one.
• Blast radius: only clients from ResolveCircleCIClient get this. The GitHub, Anthropic and token-validation clients are unchanged.
• Two accepted costs: a 401 now pays one credential re-read before failing (a security subprocess when the token came from the keychain, worst case the 3s keyring.Get timeout), and the retry shares the first attempt's deadline, so a late 401 leaves the retry little room.

Test plan

• task test, task lint, task build
• Live against a real sidecar: registered before output was consumed, output survived the CLI exiting — verified before the credential refactor
• Stop-hook path — registered while running, replayable after the hook exited — verified before the credential refactor
• Re-run both live paths against the resolved-in-cmd client
• Unauthenticated daemon: command still recorded, output pane explains why it is empty, AuthError surfaced
• chunk auth login with a chunk watch on screen: daemon is replaced and output starts streaming
• Expired/rotated token mid-session: 401 reloads and the stream resumes without restarting the daemon

[michael-webster]

The credentials type does a lot of work to lazily resolve a CircleCI client inside the daemon — but a background process with no terminal can't approve keychain prompts anyway, so the lazy init machinery handles a case that can't succeed. Whether resolution happens lazily on first command or eagerly at startup, the outcome on a keychain-approval-required machine is the same: ErrNeedsAuth, no output streaming.

I think the simpler design is to resolve credentials in the cmd/ layer before calling RunDaemon, pass the resolved *circleci.Client in (or nil if unauthenticated), and delete credentials entirely. The daemon records commands either way; a nil client just means no output is streamed, which the existing AuthError field on Snapshot already surfaces to the user. Token refresh on expiry belongs inside circleci.Client (transparent 401 retry) rather than here.

This also eliminates the mutex-around-slow-keychain-I/O problem on the hook path. Tracked in FACT-460.

[hanabel1]

reload() acquires the write lock before calling c.reloadToken(), which reads from disk or the OS keychain. While that lock is held, every goroutine calling c.token() (read lock) blocks - including all concurrent in-flight streaming connections in the daemon. The collapse logic is correct, but the I/O can happen outside the lock:

func (c *Client) reload(used string) bool {
    if c.reloadToken == nil {
        return false
    }
    c.mu.RLock()
    cur := c.authToken
    c.mu.RUnlock()
    if cur != used {
        return true
    }
    tok, err := c.reloadToken() // I/O outside the lock
    if err != nil || tok == "" || tok == used {
        return false
    }
    c.mu.Lock()
    defer c.mu.Unlock()
    if c.authToken != used {
        return true // lost the race
    }
    c.authToken = tok
    return true

[hanabel1]

reload() acquires the write lock before calling c.reloadToken(), which reads from disk or the OS keychain. While that lock is held, every goroutine calling c.token() (read lock) blocks - including all concurrent in-flight streaming connections in the daemon. The collapse logic is correct, but the I/O can happen outside the lock:

func (c *Client) reload(used string) bool {
    if c.reloadToken == nil {
        return false
    }
    c.mu.RLock()
    cur := c.authToken
    c.mu.RUnlock()
    if cur != used {
        return true
    }
    tok, err := c.reloadToken() // I/O outside the lock
    if err != nil || tok == "" || tok == used {
        return false
    }
    c.mu.Lock()
    defer c.mu.Unlock()
    if c.authToken != used {
        return true // lost the race
    }
    c.authToken = tok
    return true