Summary
runUserLogin's browser flow calls server.Shutdown(context.Background()) at cmd/andamio/user.go:232 (timeout path) and presumably ~line 237 (success path) — both with an unbounded context. The dev-login flow shipped in #101 correctly wraps this with a 2-second deadline via the new shutdownServer helper (cmd/andamio/dev.go:28-32), but user-login retains the original pattern. If the callback handler is stuck (deadlock, slow goroutine, pathological condition), Shutdown blocks indefinitely waiting for the handler to return, hanging the CLI on what should be a clean exit.
The asymmetry between the two browser flows is the bug — both should use the bounded variant. The dev-login change in #101 is the right pattern; user-login should adopt it.
Surfaced by
Round-2 ce-review of #101. Reliability reviewer R-001 (P2, confidence 0.85, marked pre_existing: true). The reviewer noted the dev-login fix is correct and the user-login defect predates this PR — but is now visibly asymmetric and worth aligning.
Reproduction
Hard to trigger in practice — would require a callback handler that's stuck not returning after responding to the HTTP request. The non-blocking-select pattern in user-login may make this effectively impossible in the current code, but the pattern is fragile: any future change that introduces a path where the handler can block (e.g., a synchronous downstream call inside the handler) immediately creates the hang.
Defensive coding consideration: http.Server.Shutdown with context.Background() waits forever for active handlers. The 2-second deadline in dev.go's shutdownServer is the right ceiling — long enough for a healthy handler to finish, short enough that a stuck handler doesn't strand the CLI.
Suggested fix
Move shutdownServer to a shared location (or duplicate the 4-line helper into user.go) and use it at both call sites in runUserLogin. Smallest surface; preserves user-login behavior under healthy conditions.
// cmd/andamio/user.go — replace BOTH server.Shutdown(context.Background())
// call sites with shutdownServer(server).
If a future PR characterizes the user-login browser flow with tests (currently zero coverage), a regression test for "handler stuck → Shutdown returns within 2 seconds" should land alongside.
Optional: move shutdownServer to a new browser_helpers.go (or similar) since both dev.go and user.go now consume the pattern. Not required — package-main scope already makes it shareable.
Acceptance criteria
Cross-references
Priority
P2 — pre-existing defect, never observed in production (as far as we know), but the asymmetry with the new dev-login code is a code-review trap. Worth fixing the next time user-login is touched.
Summary
runUserLogin's browser flow callsserver.Shutdown(context.Background())at cmd/andamio/user.go:232 (timeout path) and presumably ~line 237 (success path) — both with an unbounded context. The dev-login flow shipped in #101 correctly wraps this with a 2-second deadline via the newshutdownServerhelper (cmd/andamio/dev.go:28-32), but user-login retains the original pattern. If the callback handler is stuck (deadlock, slow goroutine, pathological condition),Shutdownblocks indefinitely waiting for the handler to return, hanging the CLI on what should be a clean exit.The asymmetry between the two browser flows is the bug — both should use the bounded variant. The dev-login change in #101 is the right pattern; user-login should adopt it.
Surfaced by
Round-2 ce-review of #101. Reliability reviewer R-001 (P2, confidence 0.85, marked
pre_existing: true). The reviewer noted the dev-login fix is correct and the user-login defect predates this PR — but is now visibly asymmetric and worth aligning.Reproduction
Hard to trigger in practice — would require a callback handler that's stuck not returning after responding to the HTTP request. The non-blocking-select pattern in user-login may make this effectively impossible in the current code, but the pattern is fragile: any future change that introduces a path where the handler can block (e.g., a synchronous downstream call inside the handler) immediately creates the hang.
Defensive coding consideration:
http.Server.Shutdownwithcontext.Background()waits forever for active handlers. The 2-second deadline in dev.go'sshutdownServeris the right ceiling — long enough for a healthy handler to finish, short enough that a stuck handler doesn't strand the CLI.Suggested fix
Move
shutdownServerto a shared location (or duplicate the 4-line helper into user.go) and use it at both call sites inrunUserLogin. Smallest surface; preserves user-login behavior under healthy conditions.If a future PR characterizes the user-login browser flow with tests (currently zero coverage), a regression test for "handler stuck → Shutdown returns within 2 seconds" should land alongside.
Optional: move
shutdownServerto a newbrowser_helpers.go(or similar) since bothdev.goanduser.gonow consume the pattern. Not required — package-main scope already makes it shareable.Acceptance criteria
server.Shutdowncall sites inrunUserLoginuseshutdownServer(the bounded variant)andamio user loginhappy path)[Unreleased]### Fixedentry (or### Internalif the project tracks non-user-visible fixes separately)Cross-references
shutdownServerto dev.go; user.go is the symmetric sitedefer listener.Close() + server.Shutdowndouble-close pattern in user login browser flow #103 — double-close pattern in user-login browser flow. Both should land together for a coherent user-login hardening.docs/plans/2026-05-22-001-feat-browser-based-dev-login-plan.md— user-login characterization is explicitly out of scope ("Deferred to Separate Tasks"); this issue is the natural home for itPriority
P2 — pre-existing defect, never observed in production (as far as we know), but the asymmetry with the new dev-login code is a code-review trap. Worth fixing the next time user-login is touched.