[krisiasty]

Pull request

Summary

An upstream agent that accepts a connection but never answers the initial identity listing kept the proxy from binding any group socket, so clients saw a missing socket until an operator stopped or reloaded the service. Socket binding never needed the upstream: the startup listing only pre-loads authorization and logs key counts, and the connection is only needed once a client asks for something. This change removes both dependencies, and makes the proxy recover on its own once the agent answers again.

• reconnectClient opens its first connection on first use instead of at construction. An agent that is not running yet, for example a password manager that starts after the proxy at login, no longer makes the runtime fail and idle: requests fail while it is missing, and the first request after it starts connects. A first dial that completes during shutdown is discarded.
• Run claims each group's startup refresh, binds the sockets, reports readiness, and then runs one shared upstream List in the background (or as soon as Run is canceled). Binding and readiness never wait for the upstream, and key counts are always logged after serving group.
• Clients that connect before the listing completes join the group's pending startup refresh and wait for its result instead of sending their own request. A slow startup result can never overwrite a fresher client refresh.
• The startup listing uses the normal List, with its reconnect and single retry after a transport failure.
• A request watchdog drops the shared upstream connection when a request has gone unanswered for 60 seconds. The agent answers requests in order, so with or without the proxy a hung agent or pending approval holds up every queued request; what the proxy lacked was recovery, because every request waited behind the one stuck connection forever. Now the waiting requests fail, as they would against the hung agent anyway, and the next request dials a fresh connection. A dropped request is not retried, and its error says the agent left a request unanswered. The startup listing is an ordinary request, so it is covered by the same mechanism. The connection dialed after a drop counts in the reconnects telemetry and logs upstream reconnected; only the very first connection logs upstream connected.
• Teardown interrupts upstream I/O, waits for the startup listing to finish, then closes clients, so no client is left waiting on an unfinished refresh. A listing interrupted by shutdown or reload is not logged as an upstream failure.
• Key counts move from serving group to a new group keys resolved line, logged when the background listing completes.
• runWithReload delegates to runWithServers, which takes the factory that builds each runtime's proxy server. Six reload and rollback tests from fix: unblock shutdown and reload when the upstream agent stalls #55 held a runtime in startup through a stalled listing; a hung upstream no longer delays readiness, so they now pass a test gate that holds the runtime in startup while it owns its group socket locks. Tests that used a missing agent to make startup fail now occupy the group socket instead.
• The server has an unexported per-instance hook for a client joining a refresh in flight, so the startup tests synchronize on a real join instead of timing.
• README: describes the new startup behavior, updates the log example, and corrects the recovery sentence deferred from fix: unblock shutdown and reload when the upstream agent stalls #55.

This differs from the approach proposed in #56. That issue kept "list, then bind" and a startup-only deadline, because a connection-wide deadline would interrupt other requests sharing the connection. Binding no longer waits for the listing, and since the agent answers in order, requests queued behind a stuck one cannot complete anyway, so one uniform watchdog replaces the startup deadline. A first version of this PR bounded the startup listing to 10 seconds with a synchronous replacement dial; review showed it cut off slow unlocks, cached a synthetic timeout, lost the startup retry, and could take 20 seconds. That code is gone: cache.go is unchanged from main, and the watchdog reuses the lazy dial instead of dialing a replacement synchronously.

Closes #56.

User impact

Group sockets are available immediately after startup or reload, whatever state the upstream agent is in, including when it is not running yet. Client requests fail while the agent is missing and wait for a slow, locked, or hung agent, as they would at any other time; they are interrupted when the service stops or reloads.

If the agent leaves a request unanswered for 60 seconds, that request and any queued behind it fail and the proxy reconnects on the next request. An unlock or approval that takes longer than that fails only the waiting request; once the agent is unlocked, the next request succeeds.

A missing upstream agent no longer makes a startup or reload fail, so it no longer causes the service to idle or a reload to roll back. Startup failures now come only from configuration and group sockets, and the idle message says "fix the problem" instead of "fix the upstream agent".

A runtime now becomes ready, and its configuration becomes the one restored after a failed reload, as soon as its sockets are bound, even while its upstream is still unanswered.

Log consumers that read the key count from serving group should use the new group keys resolved entry.

Validation

• Gopls workspace, file-context, symbol-reference, vulnerability baseline, and final diagnostics; no build or analysis errors.
• SSH_AGENT_PROXY_REQUIRE_OPENSSH=1 go test -race ./... -count=1.
• The proxy and app packages pass thirty race-enabled runs each across -cpu 1,2,4; twenty race-enabled repetitions of the startup, resolution, reload, rollback, shutdown, reconnect, cache, telemetry, and signing tests.
• New tests: sockets bind and readiness is reported while the startup listing is unanswered, and an early client waits for and shares that listing (one upstream List in total); shutdown during the listing returns cleanly, releases the waiting client, and logs no upstream-failure warning; a reload onto a hung upstream logs configuration reloaded and serves immediately; the proxy serves before the upstream agent starts and connects on the first request after it does; a first dial completing during shutdown is discarded; key counts are logged after serving group.
• Watchdog tests: an unanswered List or Sign drops the connection without retry and the next request reconnects (counted once in the reconnects telemetry); requests queued behind an unanswered one fail with it; an answered connection is left alone; and the server recovers when the agent never answers its first connection but answers new ones.
• Mutation checks in scratch copies: with the watchdog disabled, the recovery test hangs; with the post-drop reconnect left uncounted, all three reconnect assertions fail; a synchronous startup listing fails the proxy readiness and app reload tests; leaving startup refreshes unfinished on shutdown fails the shutdown test; launching a replacement before the held runtime stops fails the gated reload test (the held runtime still owns the socket lock); an eager initial dial fails the missing-agent test; listing before binding fails the log-order assertion.
• End to end with the built binary: against a fake agent that never replies, the socket is bound about 0.3 seconds after process start, an ssh-add -l client waits on the pending listing, a reload serves again immediately, and the agent received exactly one List per runtime; with no agent running, the socket is bound immediately, ssh-add -l fails in 0.02 seconds instead of hanging, and after the agent starts the next request connects and is served. with an agent that never answers its first connection but answers new ones, a client waiting on the stuck startup listing fails after 60 seconds and the next ssh-add -l is served over a new connection. SIGTERM exits with code 0 in every case.
• Native and Linux golangci-lint (0 issues, no suppressions), go vet ./... (also with GOOS=windows for the changed packages), formatting, and git diff --check.

No dependencies changed. The vulnerability baseline reports the known advisory for unused x/crypto/openpgp.

Checklist

• Tests cover changed behavior.
• Documentation is updated when behavior or design changed.
• No credentials, private configuration, or identifying key metadata are included.
• go test -race ./... passes locally, or the reason it was not run is documented.

🤖 Generated with Claude Code

[krisiasty]

Code review (high): 9 findings. Five affect correctness or behaviour and four are cleanup. They have not been verified separately.

The design comment on server.go:200 may make several of the others unnecessary: if socket binding no longer waits on the startup listing, the timeout isn't needed.

🤖 Generated with Claude Code

[krisiasty]

Code review (high), re-review of d16e42f. All 9 findings from the previous review are resolved: listWithin, listUsing, startupTimeout and the logCtx parameter are gone, and upstream.go and cache.go match main.

This review has 9 new findings: three correctness or behaviour issues introduced by the new design, three test-coverage problems, and three cleanups. They have not been verified separately.

🤖 Generated with Claude Code