fix(backend): bind the HTTP listener before startup recovery so health checks don't crash-loop (#2944)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -180,7 +180,7 @@ Standalone agentctl is launched in its own process group so terminal Ctrl+C is h
|
||||
- **WS**: clients carry their identity; dispatched actions and subscriptions are scoped; workspace-carrying events route via `Hub.BroadcastToWorkspace`. A new `hub.Broadcast` (global) call site needs a `//ws:global` justification comment. Subscription actions (`*.subscribe`) `return` before reaching the dispatch backstop (`Client.authorizeAction`), so scoping a new subscribe topic means adding a hook to `SubscriptionAccessPolicy` (`internal/gateway/websocket/access.go`) *and* calling it explicitly inside that action's own handler — the backstop's `scopedActionRefs` cannot reach it. `run.subscribe`'s `Subscriptions.Run` hook (added for WO-02, resolving the run's workspace via `runs.GetRunWorkspaceID`) is the worked example.
|
||||
- **Task overview vs. session detail:** Keep cross-task state in the bounded, persisted `TaskStatusSummary` projection and `task.status_summary.updated`; keep unbounded transcript and execution data session-scoped. The [bounded task-status spec](../../docs/specs/platform/requirements/bounded-task-status-delivery.md) and [accepted ADR](../../docs/decisions/2026-08-01-separate-task-summary-session-stream-traffic.md) define the shared contract.
|
||||
- **Port-proxy subtree capability:** with auth enforced, `/port-proxy/:sessionId/:port/*` needs a credential on *every* request, but browser subresource fetches cannot always carry one (the `<link rel="manifest">` fetch never sends cookies; sandboxed iframes and `?token=`-opened previews drop them elsewhere). `PortProxyHandler` mints a short-lived (15 min, sliding), path-scoped, HMAC-signed capability after the document authenticates and propagates it as a cookie scoped to the proxy subtree plus a `kandev_cap` query parameter appended to rewritten asset URLs. `requireConnectionAuth` (`internal/gateway/websocket/access.go`) accepts either form only for the exact session:port it was minted for and restores the issuing identity, so `CheckSessionAccess` still gates on the real owner. Synthetic identities (auth disabled) skip minting — proxied bodies stay byte-identical to the pre-auth behavior.
|
||||
- **Self-authenticating webhooks** (automation and office channels) and `/health`, `/api/v1/features`, `/api/v1/app-state` stay public — the allowlist lives in `auth/httpmw/middleware.go` with a pinning test. Plugin webhooks are not allowlisted: `auth/httpmw` structurally defers only GET/POST relay paths because it cannot read manifests, and `internal/plugins.Controller.webhook` enforces each manifest's `access` policy.
|
||||
- **Self-authenticating webhooks** (automation and office channels) and `/health`, `/ready`, `/api/v1/features`, `/api/v1/app-state` stay public — the allowlist lives in `auth/httpmw/middleware.go` with a pinning test. Plugin webhooks are not allowlisted: `auth/httpmw` structurally defers only GET/POST relay paths because it cannot read manifests, and `internal/plugins.Controller.webhook` enforces each manifest's `access` policy.
|
||||
|
||||
## Execution Flow
|
||||
|
||||
|
||||
@@ -297,25 +297,35 @@ func waitForServiceStarted(stream *sprites.ServiceStream) error {
|
||||
}
|
||||
}
|
||||
|
||||
// waitForKandev polls the kandev /health endpoint inside the sprite via
|
||||
// CommandContext until kandev responds or the deadline is exceeded.
|
||||
// Using the internal address (localhost) avoids Sprites routing state that
|
||||
// may lag during a service restart triggered by enablePublicURL.
|
||||
// kandevReadyURL returns the readiness endpoint waitForKandev polls, broken
|
||||
// out so tests can pin the endpoint without going through the sprite exec/
|
||||
// websocket machinery. /health only proves the bootstrap listener has bound
|
||||
// the socket (it returns 200 immediately, before the real router is wired
|
||||
// in); /ready is what flips once the real router is swapped in and requests
|
||||
// stop getting the bootstrap handler's 503 "starting" response.
|
||||
func kandevReadyURL(port int) string {
|
||||
return fmt.Sprintf("http://localhost:%d/ready", port)
|
||||
}
|
||||
|
||||
// waitForKandev polls the kandev /ready endpoint inside the sprite via
|
||||
// CommandContext until kandev is ready to serve real traffic or the deadline
|
||||
// is exceeded. Using the internal address (localhost) avoids Sprites routing
|
||||
// state that may lag during a service restart triggered by enablePublicURL.
|
||||
func waitForKandev(ctx context.Context, sprite *sprites.Sprite, port int) error {
|
||||
const (
|
||||
timeout = 90 * time.Second
|
||||
retryWait = 3 * time.Second
|
||||
)
|
||||
healthURL := fmt.Sprintf("http://localhost:%d/health", port)
|
||||
readyURL := kandevReadyURL(port)
|
||||
deadline := time.Now().Add(timeout)
|
||||
|
||||
for time.Now().Before(deadline) {
|
||||
checkCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
|
||||
out, err := sprite.CommandContext(checkCtx, "curl", "-sf", healthURL).Output()
|
||||
out, err := sprite.CommandContext(checkCtx, "curl", "-sf", readyURL).Output()
|
||||
cancel()
|
||||
|
||||
if err == nil && len(out) > 0 {
|
||||
fmt.Fprintf(os.Stderr, " kandev is healthy\n")
|
||||
fmt.Fprintf(os.Stderr, " kandev is ready\n")
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -326,9 +336,9 @@ func waitForKandev(ctx context.Context, sprite *sprites.Sprite, port int) error
|
||||
}
|
||||
}
|
||||
|
||||
// Health check timed out — fetch logs to help diagnose.
|
||||
// Readiness check timed out — fetch logs to help diagnose.
|
||||
diag := fetchSpriteLogs(ctx, sprite)
|
||||
return fmt.Errorf("kandev did not become healthy within %v\n%s", timeout, diag)
|
||||
return fmt.Errorf("kandev did not become ready within %v\n%s", timeout, diag)
|
||||
}
|
||||
|
||||
// fetchSpriteLogs reads log files from the sprite for failure diagnostics.
|
||||
|
||||
@@ -176,3 +176,28 @@ func TestBuildExtractScript(t *testing.T) {
|
||||
t.Errorf("script should not launch the backend binary directly")
|
||||
}
|
||||
}
|
||||
|
||||
// TestKandevReadyURLPollsReadyNotHealth is the regression test for Review
|
||||
// round 3 finding R3-1: waitForKandev used to poll /health, which this
|
||||
// branch redefined as an unconditional-200 liveness probe served by the
|
||||
// bootstrap handler the instant the socket binds. That made preview-env CI
|
||||
// (.github/workflows/preview-env.yml) declare the deploy ready — and post
|
||||
// the PR comment with the URL — while every non-/health path, including the
|
||||
// whole app, was still returning the bootstrap handler's 503 "starting".
|
||||
//
|
||||
// Expected pre-fix failure: kandevReadyURL did not exist (waitForKandev
|
||||
// built the URL inline as ".../health"), so this fails to compile against
|
||||
// the pre-fix code, and once inlined the URL would end in /health, not
|
||||
// /ready.
|
||||
func TestKandevReadyURLPollsReadyNotHealth(t *testing.T) {
|
||||
got := kandevReadyURL(4173)
|
||||
want := "http://localhost:4173/ready"
|
||||
if got != want {
|
||||
t.Fatalf("kandevReadyURL(4173) = %q, want %q", got, want)
|
||||
}
|
||||
if strings.Contains(got, "/health") {
|
||||
t.Fatalf("kandevReadyURL must not poll /health — the bootstrap handler answers "+
|
||||
"200 there before the real router exists, so preview-env would declare "+
|
||||
"success while every other path still 503s: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -303,7 +303,11 @@ func BearerToken(r *http.Request) string {
|
||||
func isPublicPath(method, path string) bool {
|
||||
switch path {
|
||||
case "/health":
|
||||
// CLI + desktop readiness probes poll before any session can exist.
|
||||
// CLI + desktop liveness probes poll before any session can exist.
|
||||
return method == http.MethodGet
|
||||
case "/ready":
|
||||
// Kubernetes readinessProbe and the e2e fixture poll before any
|
||||
// session can exist, same as /health.
|
||||
return method == http.MethodGet
|
||||
case "/api/v1/features", "/api/v1/app-state":
|
||||
// SPA bootstrap reads; app-state returns the auth-aware boot payload
|
||||
|
||||
@@ -204,6 +204,7 @@ func TestEnabledModeAllowlistMatrix(t *testing.T) {
|
||||
blocked bool
|
||||
}{
|
||||
{name: "health", method: http.MethodGet, path: "/health"},
|
||||
{name: "ready", method: http.MethodGet, path: "/ready"},
|
||||
{name: "features", method: http.MethodGet, path: "/api/v1/features"},
|
||||
{name: "app-state", method: http.MethodGet, path: "/api/v1/app-state"},
|
||||
{name: "login", method: http.MethodPost, path: "/api/v1/auth/login"},
|
||||
|
||||
@@ -0,0 +1,263 @@
|
||||
package backendapp
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/http"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/config"
|
||||
"go.uber.org/goleak"
|
||||
)
|
||||
|
||||
// getJSON dials host:port+path over HTTP and decodes a JSON object body.
|
||||
// Keep-alives are disabled so no idle client connection lingers for goleak.
|
||||
func getJSON(host string, port int, path string) (int, map[string]any, error) {
|
||||
return requestJSON(host, port, http.MethodGet, path)
|
||||
}
|
||||
|
||||
func requestJSON(host string, port int, method string, path string) (int, map[string]any, error) {
|
||||
url := fmt.Sprintf("http://%s%s", net.JoinHostPort(host, fmt.Sprint(port)), path)
|
||||
client := &http.Client{
|
||||
Timeout: 500 * time.Millisecond,
|
||||
Transport: &http.Transport{DisableKeepAlives: true},
|
||||
}
|
||||
defer client.CloseIdleConnections()
|
||||
req, err := http.NewRequest(method, url, nil)
|
||||
if err != nil {
|
||||
return 0, nil, err
|
||||
}
|
||||
resp, err := client.Do(req)
|
||||
if err != nil {
|
||||
return 0, nil, err
|
||||
}
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
var body map[string]any
|
||||
if decodeErr := json.NewDecoder(resp.Body).Decode(&body); decodeErr != nil {
|
||||
return resp.StatusCode, nil, decodeErr
|
||||
}
|
||||
return resp.StatusCode, body, nil
|
||||
}
|
||||
|
||||
// TestBindBootstrapListenersServesLivenessBeforeSwap is the regression test
|
||||
// for docs/plans/startup-listener-before-recovery/task-01-bind-before-
|
||||
// startup.md: a caller must be able to bind the socket and get a 200 on the
|
||||
// launcher's liveness path (GET /health) before the real router exists.
|
||||
//
|
||||
// Expected pre-fix failure: bindBootstrapListeners does not exist yet, so
|
||||
// this fails to compile.
|
||||
func TestBindBootstrapListenersServesLivenessBeforeSwap(t *testing.T) {
|
||||
defer goleak.VerifyNone(t)
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Server.Host = "127.0.0.1"
|
||||
cfg.Server.Port = freePort(t)
|
||||
log := testLogger(t)
|
||||
|
||||
handler, server, listeners, err := bindBootstrapListeners(cfg, log, "1.2.3-test")
|
||||
if err != nil {
|
||||
t.Fatalf("bindBootstrapListeners: %v", err)
|
||||
}
|
||||
defer shutdown(t, listeners, server)
|
||||
|
||||
status, body, err := getJSON("127.0.0.1", cfg.Server.Port, "/health")
|
||||
if err != nil {
|
||||
t.Fatalf("GET /health before swap: %v", err)
|
||||
}
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("bootstrap /health status = %d, want %d (launcher liveness probe requires 200)", status, http.StatusOK)
|
||||
}
|
||||
if got := body[versionFieldKey]; got != "1.2.3-test" {
|
||||
t.Fatalf("bootstrap /health version = %v, want 1.2.3-test", got)
|
||||
}
|
||||
if got := body[statusKey]; got != "ok" {
|
||||
t.Fatalf("bootstrap /health status field = %v, want %q (must match healthHandler's contract)", got, "ok")
|
||||
}
|
||||
if got := body["mode"]; got != "websocket+http" {
|
||||
t.Fatalf("bootstrap /health mode = %v, want %q (must match healthHandler's contract)", got, "websocket+http")
|
||||
}
|
||||
|
||||
status, body, err = requestJSON("127.0.0.1", cfg.Server.Port, http.MethodPost, "/health")
|
||||
if err != nil {
|
||||
t.Fatalf("POST /health before swap: %v", err)
|
||||
}
|
||||
if status != http.StatusServiceUnavailable {
|
||||
t.Fatalf("bootstrap POST /health status = %d, want %d", status, http.StatusServiceUnavailable)
|
||||
}
|
||||
if body[statusKey] != startingStatus {
|
||||
t.Fatalf("bootstrap POST /health body = %v, want status=%q", body, startingStatus)
|
||||
}
|
||||
|
||||
// Every other path must answer deterministically, not hang or 404.
|
||||
status, body, err = getJSON("127.0.0.1", cfg.Server.Port, "/api/v1/whatever")
|
||||
if err != nil {
|
||||
t.Fatalf("GET non-health path before swap: %v", err)
|
||||
}
|
||||
if status != http.StatusServiceUnavailable {
|
||||
t.Fatalf("bootstrap non-health status = %d, want %d", status, http.StatusServiceUnavailable)
|
||||
}
|
||||
if body["status"] != startingStatus {
|
||||
t.Fatalf("bootstrap non-health body = %v, want status=%q", body, startingStatus)
|
||||
}
|
||||
|
||||
if handler == nil {
|
||||
t.Fatal("bindBootstrapListeners returned a nil handler switch")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBindBootstrapListenersEchoesDesktopHealthToken guards against the
|
||||
// bootstrap handler poisoning the launcher's health probe: probeHealth
|
||||
// (internal/launcher/health.go) treats a 200 with a missing/mismatched
|
||||
// X-Kandev-Desktop-Health-Token as a foreign process and permanently sticks
|
||||
// that verdict for the whole wait window.
|
||||
func TestBindBootstrapListenersEchoesDesktopHealthToken(t *testing.T) {
|
||||
defer goleak.VerifyNone(t)
|
||||
t.Setenv(desktopHealthTokenEnv, "bootstrap-token")
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Server.Host = "127.0.0.1"
|
||||
cfg.Server.Port = freePort(t)
|
||||
log := testLogger(t)
|
||||
|
||||
_, server, listeners, err := bindBootstrapListeners(cfg, log, "1.2.3-test")
|
||||
if err != nil {
|
||||
t.Fatalf("bindBootstrapListeners: %v", err)
|
||||
}
|
||||
defer shutdown(t, listeners, server)
|
||||
|
||||
url := fmt.Sprintf("http://%s/health", net.JoinHostPort("127.0.0.1", fmt.Sprint(cfg.Server.Port)))
|
||||
client := &http.Client{Timeout: 500 * time.Millisecond, Transport: &http.Transport{DisableKeepAlives: true}}
|
||||
defer client.CloseIdleConnections()
|
||||
resp, err := client.Get(url)
|
||||
if err != nil {
|
||||
t.Fatalf("GET /health: %v", err)
|
||||
}
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
if got := resp.Header.Get(desktopHealthTokenHeader); got != "bootstrap-token" {
|
||||
t.Fatalf("bootstrap /health token header = %q, want bootstrap-token", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandlerSwitchSwapsWithoutRebind is the regression test for the "no
|
||||
// window where the socket is closed and reopened" acceptance criterion: the
|
||||
// same listeners must serve the bootstrap handler and then the real handler
|
||||
// with no gap and no second bind.
|
||||
func TestHandlerSwitchSwapsWithoutRebind(t *testing.T) {
|
||||
defer goleak.VerifyNone(t)
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Server.Host = "127.0.0.1"
|
||||
cfg.Server.Port = freePort(t)
|
||||
log := testLogger(t)
|
||||
|
||||
handler, server, listeners, err := bindBootstrapListeners(cfg, log, "1.2.3-test")
|
||||
if err != nil {
|
||||
t.Fatalf("bindBootstrapListeners: %v", err)
|
||||
}
|
||||
defer shutdown(t, listeners, server)
|
||||
|
||||
boundBefore := listeners.boundAddrs()
|
||||
|
||||
status, _, err := getJSON("127.0.0.1", cfg.Server.Port, "/api/v1/whatever")
|
||||
if err != nil {
|
||||
t.Fatalf("GET before swap: %v", err)
|
||||
}
|
||||
if status != http.StatusServiceUnavailable {
|
||||
t.Fatalf("status before swap = %d, want %d", status, http.StatusServiceUnavailable)
|
||||
}
|
||||
|
||||
handler.Store(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
status, err = getStatus("127.0.0.1", cfg.Server.Port)
|
||||
if err != nil {
|
||||
t.Fatalf("GET after swap: %v", err)
|
||||
}
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("status after swap = %d, want %d", status, http.StatusOK)
|
||||
}
|
||||
|
||||
if got := listeners.boundAddrs(); len(got) != len(boundBefore) || got[0] != boundBefore[0] {
|
||||
t.Fatalf("bound addresses changed across swap: before=%v after=%v (want no rebind)", boundBefore, got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBindBootstrapListenersFailsWhenPortUnavailable pins today's bind-
|
||||
// failure behaviour: bindBootstrapListeners returns errServerBindFailed
|
||||
// (already logged by startHTTPServers) rather than a generic error, so the
|
||||
// caller can skip the misleading "Failed to start orchestrator" log.
|
||||
func TestBindBootstrapListenersFailsWhenPortUnavailable(t *testing.T) {
|
||||
defer goleak.VerifyNone(t)
|
||||
|
||||
blocker := listenOnFreePort(t)
|
||||
t.Cleanup(func() { _ = blocker.Close() })
|
||||
port := listenerPort(t, blocker)
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Server.Host = "127.0.0.1"
|
||||
cfg.Server.Port = port
|
||||
log := testLogger(t)
|
||||
|
||||
handler, server, listeners, err := bindBootstrapListeners(cfg, log, "1.2.3-test")
|
||||
if !errors.Is(err, errServerBindFailed) {
|
||||
t.Fatalf("bindBootstrapListeners error = %v, want %v", err, errServerBindFailed)
|
||||
}
|
||||
if handler != nil || server != nil || listeners != nil {
|
||||
t.Fatalf("bindBootstrapListeners returned non-nil results on bind failure: handler=%v server=%v listeners=%v",
|
||||
handler, server, listeners)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCloseBoundListenersReleasesPortOnStartupFailure is the regression test
|
||||
// for the listener-leak flagged in Build round 3 review: a step that fails
|
||||
// after bindBootstrapListeners already bound the socket (orchestrator start,
|
||||
// office services, storage composition, buildHTTPServer) must release the
|
||||
// port before returning, not just stop the background bind-retry loop.
|
||||
func TestCloseBoundListenersReleasesPortOnStartupFailure(t *testing.T) {
|
||||
defer goleak.VerifyNone(t)
|
||||
|
||||
cfg := &config.Config{}
|
||||
cfg.Server.Host = "127.0.0.1"
|
||||
cfg.Server.Port = freePort(t)
|
||||
log := testLogger(t)
|
||||
|
||||
_, server, listeners, err := bindBootstrapListeners(cfg, log, "1.2.3-test")
|
||||
if err != nil {
|
||||
t.Fatalf("bindBootstrapListeners: %v", err)
|
||||
}
|
||||
|
||||
closeBoundListeners(server, listeners, log)
|
||||
|
||||
// The port must become free: a later startup failure that leaks the
|
||||
// listener would leave a same-process retry (or even a fresh process
|
||||
// racing a restart) unable to bind here. server.Close() closing the
|
||||
// listener's fd and the OS releasing the address aren't guaranteed to be
|
||||
// observable in the same instant, so poll briefly rather than asserting
|
||||
// on the very next syscall.
|
||||
deadline := time.Now().Add(2 * time.Second)
|
||||
var lastErr error
|
||||
for time.Now().Before(deadline) {
|
||||
ln, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", fmt.Sprint(cfg.Server.Port)))
|
||||
if err == nil {
|
||||
if closeErr := ln.Close(); closeErr != nil {
|
||||
t.Fatalf("close probe listener: %v", closeErr)
|
||||
}
|
||||
return
|
||||
}
|
||||
lastErr = err
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
t.Fatalf("port still held after closeBoundListeners: %v", lastErr)
|
||||
}
|
||||
|
||||
// TestCloseBoundListenersNoopWhenListenersNil covers the pure-bind-failure
|
||||
// case: bindBootstrapListeners returns a nil listeners/server pair when the
|
||||
// bind itself failed, so closeBoundListeners must not dereference server.
|
||||
func TestCloseBoundListenersNoopWhenListenersNil(t *testing.T) {
|
||||
closeBoundListeners(nil, nil, testLogger(t))
|
||||
}
|
||||
@@ -14,7 +14,7 @@ import (
|
||||
)
|
||||
|
||||
// setReadyForTest flips the package-level readiness flag consulted by
|
||||
// healthHandler and restores its prior value on cleanup, so tests don't leak
|
||||
// readyHandler and restores its prior value on cleanup, so tests don't leak
|
||||
// state into each other or into TestMain-driven suites.
|
||||
func setReadyForTest(t *testing.T, value bool) {
|
||||
t.Helper()
|
||||
@@ -23,94 +23,121 @@ func setReadyForTest(t *testing.T, value bool) {
|
||||
t.Cleanup(func() { ready.Store(prev) })
|
||||
}
|
||||
|
||||
// TestHealthHandlerReadyBodyIncludesVersion covers AC-1..4: once ready, the
|
||||
// handler returns 200 with a version key equal to the configured build
|
||||
// version, alongside the unchanged status/service/mode fields, and no other
|
||||
// keys.
|
||||
func TestHealthHandlerReadyBodyIncludesVersion(t *testing.T) {
|
||||
// TestHealthHandlerBodyIncludesVersionRegardlessOfReadiness covers AC-19/20
|
||||
// of the 2026-08-22 liveness/readiness-split amendment
|
||||
// (docs/specs/health-endpoint-version/spec.md): /health is a pure liveness
|
||||
// probe, so its 200 body (status/service/mode/version, no other keys) is
|
||||
// identical whether or not the `ready` flag has flipped. The old ready/
|
||||
// starting two-shape split (superseded AC-1..7) now lives on readyHandler,
|
||||
// covered by TestReadyHandlerBodyShapesByReadiness below.
|
||||
func TestHealthHandlerBodyIncludesVersionRegardlessOfReadiness(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
setReadyForTest(t, true)
|
||||
|
||||
router := gin.New()
|
||||
router.GET("/health", healthHandler(routeParams{version: "1.2.3"}))
|
||||
for _, readyState := range []bool{true, false} {
|
||||
setReadyForTest(t, readyState)
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/health", nil))
|
||||
router := gin.New()
|
||||
router.GET("/health", healthHandler(routeParams{version: "1.2.3"}))
|
||||
|
||||
if recorder.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d", recorder.Code, http.StatusOK)
|
||||
}
|
||||
var body map[string]interface{}
|
||||
if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode body: %v", err)
|
||||
}
|
||||
if body[versionFieldKey] != "1.2.3" {
|
||||
t.Fatalf("version = %v, want 1.2.3", body[versionFieldKey])
|
||||
}
|
||||
if body["status"] != "ok" || body["service"] != kandevName || body["mode"] != "websocket+http" {
|
||||
t.Fatalf("unexpected ready body: %#v", body)
|
||||
}
|
||||
if len(body) != 4 {
|
||||
t.Fatalf("ready body keys = %#v, want exactly status/service/mode/version", body)
|
||||
recorder := httptest.NewRecorder()
|
||||
router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/health", nil))
|
||||
|
||||
if recorder.Code != http.StatusOK {
|
||||
t.Fatalf("ready=%v: status = %d, want %d", readyState, recorder.Code, http.StatusOK)
|
||||
}
|
||||
var body map[string]interface{}
|
||||
if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("ready=%v: decode body: %v", readyState, err)
|
||||
}
|
||||
if body[versionFieldKey] != "1.2.3" {
|
||||
t.Fatalf("ready=%v: version = %v, want 1.2.3", readyState, body[versionFieldKey])
|
||||
}
|
||||
if body["status"] != "ok" || body["service"] != kandevName || body["mode"] != "websocket+http" {
|
||||
t.Fatalf("ready=%v: unexpected body: %#v", readyState, body)
|
||||
}
|
||||
if len(body) != 4 {
|
||||
t.Fatalf("ready=%v: body keys = %#v, want exactly status/service/mode/version", readyState, body)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestHealthHandlerStartingBodyIncludesVersion covers AC-5..7: before ready,
|
||||
// the handler still returns the version alongside status/service, with no
|
||||
// other keys (mode is ready-path only).
|
||||
func TestHealthHandlerStartingBodyIncludesVersion(t *testing.T) {
|
||||
// TestReadyHandlerBodyShapesByReadiness covers AC-16..19: readyHandler now
|
||||
// owns the two-shape ready/starting split that healthHandler used to have —
|
||||
// 503 with status="starting" (no mode key) before ready, 200 with
|
||||
// status="ok" (no mode key) once ready — always carrying version either way.
|
||||
func TestReadyHandlerBodyShapesByReadiness(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
setReadyForTest(t, false)
|
||||
|
||||
router := gin.New()
|
||||
router.GET("/health", healthHandler(routeParams{version: "1.2.3"}))
|
||||
router.GET("/ready", readyHandler(routeParams{version: "1.2.3"}))
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/health", nil))
|
||||
startingRec := httptest.NewRecorder()
|
||||
router.ServeHTTP(startingRec, httptest.NewRequest(http.MethodGet, "/ready", nil))
|
||||
if startingRec.Code != http.StatusServiceUnavailable {
|
||||
t.Fatalf("starting status = %d, want %d", startingRec.Code, http.StatusServiceUnavailable)
|
||||
}
|
||||
var startingBody map[string]interface{}
|
||||
if err := json.Unmarshal(startingRec.Body.Bytes(), &startingBody); err != nil {
|
||||
t.Fatalf("decode starting body: %v", err)
|
||||
}
|
||||
if startingBody[versionFieldKey] != "1.2.3" {
|
||||
t.Fatalf("starting version = %v, want 1.2.3", startingBody[versionFieldKey])
|
||||
}
|
||||
if startingBody["status"] != startingStatus || startingBody["service"] != kandevName {
|
||||
t.Fatalf("unexpected starting body: %#v", startingBody)
|
||||
}
|
||||
if len(startingBody) != 3 {
|
||||
t.Fatalf("starting body keys = %#v, want exactly status/service/version", startingBody)
|
||||
}
|
||||
|
||||
if recorder.Code != http.StatusServiceUnavailable {
|
||||
t.Fatalf("status = %d, want %d", recorder.Code, http.StatusServiceUnavailable)
|
||||
setReadyForTest(t, true)
|
||||
readyRec := httptest.NewRecorder()
|
||||
router.ServeHTTP(readyRec, httptest.NewRequest(http.MethodGet, "/ready", nil))
|
||||
if readyRec.Code != http.StatusOK {
|
||||
t.Fatalf("ready status = %d, want %d", readyRec.Code, http.StatusOK)
|
||||
}
|
||||
var body map[string]interface{}
|
||||
if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode body: %v", err)
|
||||
var readyBody map[string]interface{}
|
||||
if err := json.Unmarshal(readyRec.Body.Bytes(), &readyBody); err != nil {
|
||||
t.Fatalf("decode ready body: %v", err)
|
||||
}
|
||||
if body[versionFieldKey] != "1.2.3" {
|
||||
t.Fatalf("version = %v, want 1.2.3", body[versionFieldKey])
|
||||
if readyBody[versionFieldKey] != "1.2.3" {
|
||||
t.Fatalf("ready version = %v, want 1.2.3", readyBody[versionFieldKey])
|
||||
}
|
||||
if body["status"] != startingStatus || body["service"] != kandevName {
|
||||
t.Fatalf("unexpected starting body: %#v", body)
|
||||
if readyBody["status"] != "ok" || readyBody["service"] != kandevName {
|
||||
t.Fatalf("unexpected ready body: %#v", readyBody)
|
||||
}
|
||||
if len(body) != 3 {
|
||||
t.Fatalf("starting body keys = %#v, want exactly status/service/version", body)
|
||||
if len(readyBody) != 3 {
|
||||
t.Fatalf("ready body keys = %#v, want exactly status/service/version", readyBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHealthHandlerDesktopTokenHeaderOnlyOnReadyPath guards a pre-existing
|
||||
// behavior the spec restates as a must-not-regress ("## What": the desktop
|
||||
// health-token header SHALL continue to be set on the 200 path only,
|
||||
// unchanged) but that had no direct test before this change extracted the
|
||||
// handler body into healthHandler. Confirms the refactor didn't move the
|
||||
// header write outside its original branch.
|
||||
func TestHealthHandlerDesktopTokenHeaderOnlyOnReadyPath(t *testing.T) {
|
||||
// TestHealthHandlerDesktopTokenHeaderAlwaysSetReadyEndpointNever covers AC-21:
|
||||
// the desktop health-token header is a /health-only concern, unaffected by
|
||||
// the liveness/readiness split — /health sets it on every request regardless
|
||||
// of readiness, and /ready never sets it at all.
|
||||
func TestHealthHandlerDesktopTokenHeaderAlwaysSetReadyEndpointNever(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
t.Setenv(desktopHealthTokenEnv, " desktop-token ")
|
||||
|
||||
router := gin.New()
|
||||
router.GET("/health", healthHandler(routeParams{version: "1.2.3"}))
|
||||
router.GET("/ready", readyHandler(routeParams{version: "1.2.3"}))
|
||||
|
||||
setReadyForTest(t, false)
|
||||
startingRec := httptest.NewRecorder()
|
||||
router.ServeHTTP(startingRec, httptest.NewRequest(http.MethodGet, "/health", nil))
|
||||
if got := startingRec.Header().Get(desktopHealthTokenHeader); got != "" {
|
||||
t.Fatalf("starting-path %s header = %q, want absent", desktopHealthTokenHeader, got)
|
||||
}
|
||||
for _, readyState := range []bool{false, true} {
|
||||
setReadyForTest(t, readyState)
|
||||
|
||||
setReadyForTest(t, true)
|
||||
readyRec := httptest.NewRecorder()
|
||||
router.ServeHTTP(readyRec, httptest.NewRequest(http.MethodGet, "/health", nil))
|
||||
if got := readyRec.Header().Get(desktopHealthTokenHeader); got != "desktop-token" {
|
||||
t.Fatalf("ready-path %s header = %q, want desktop-token", desktopHealthTokenHeader, got)
|
||||
healthRec := httptest.NewRecorder()
|
||||
router.ServeHTTP(healthRec, httptest.NewRequest(http.MethodGet, "/health", nil))
|
||||
if got := healthRec.Header().Get(desktopHealthTokenHeader); got != "desktop-token" {
|
||||
t.Fatalf("ready=%v: /health %s header = %q, want desktop-token", readyState, desktopHealthTokenHeader, got)
|
||||
}
|
||||
|
||||
readyRec := httptest.NewRecorder()
|
||||
router.ServeHTTP(readyRec, httptest.NewRequest(http.MethodGet, "/ready", nil))
|
||||
if got := readyRec.Header().Get(desktopHealthTokenHeader); got != "" {
|
||||
t.Fatalf("ready=%v: /ready %s header = %q, want absent", readyState, desktopHealthTokenHeader, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -265,3 +292,39 @@ func TestHealthHandlerAuthEnabledServesVersionWithoutCredential(t *testing.T) {
|
||||
t.Fatalf("status field = %v, want ok", body["status"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestReadyHandlerAuthEnabledServesWithoutCredential covers AC-22: /ready
|
||||
// must be on the unauthenticated allowlist (mirrored at the middleware layer
|
||||
// by TestEnabledModeAllowlistMatrix/ready), so it never returns 401/403 to
|
||||
// an unauthenticated k8s readinessProbe or e2e fixture poll even with the
|
||||
// auth feature flag on.
|
||||
func TestReadyHandlerAuthEnabledServesWithoutCredential(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
setReadyForTest(t, true)
|
||||
|
||||
svc := newSSOTestAuthService(t)
|
||||
if _, _, err := svc.Setup(context.Background(), "admin@example.com", "adminpass123", "Admin", "", ""); err != nil {
|
||||
t.Fatalf("Setup admin: %v", err)
|
||||
}
|
||||
|
||||
router := gin.New()
|
||||
router.Use(authhttpmw.Middleware(svc))
|
||||
router.GET("/ready", readyHandler(routeParams{version: "1.2.3"}))
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
router.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/ready", nil))
|
||||
|
||||
if recorder.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d (no auth challenge on /ready)", recorder.Code, http.StatusOK)
|
||||
}
|
||||
var body map[string]interface{}
|
||||
if err := json.Unmarshal(recorder.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode body: %v", err)
|
||||
}
|
||||
if body[versionFieldKey] != "1.2.3" {
|
||||
t.Fatalf("version = %v, want 1.2.3", body[versionFieldKey])
|
||||
}
|
||||
if body["status"] != "ok" {
|
||||
t.Fatalf("status field = %v, want ok", body["status"])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,6 +110,7 @@ const (
|
||||
branchDeletionsFieldKey = "branch_deletions"
|
||||
deletedFieldKey = "deleted"
|
||||
versionFieldKey = "version"
|
||||
serviceFieldKey = "service"
|
||||
kandevName = "kandev"
|
||||
startingStatus = "starting"
|
||||
)
|
||||
@@ -718,16 +719,27 @@ func registerRoutes(p routeParams) {
|
||||
authhttpapi.RegisterRoutes(p.router, p.authSvc, p.log)
|
||||
}
|
||||
|
||||
// /health is a readiness probe, not a liveness probe. It only
|
||||
// returns 200 after main has flipped the package-level `ready`
|
||||
// flag — which happens after route registration, agent-registry
|
||||
// seeding, the HTTP listener accepting connections, and (when
|
||||
// KANDEV_E2E_MOCK is set) the testharness routes being mounted.
|
||||
// Before that, return 503 so callers (including the e2e fixture's
|
||||
// waitForHealth) keep polling instead of racing ahead and hitting
|
||||
// 404s on routes that aren't wired yet.
|
||||
// /health is a liveness probe: it answers 200 unconditionally, matching
|
||||
// the bootstrap handler's /health contract (internal/backendapp/
|
||||
// httpserver.go) so the launcher's healthcheck never depends on startup
|
||||
// progress. By the time this handler is reachable at all, ready is
|
||||
// already true (see the comment on startGatewayAndServe's Store calls),
|
||||
// but it does not gate on that flag — liveness must not depend on
|
||||
// readiness, or the crash loop docs/specs/startup-listener-before-
|
||||
// recovery/spec.md exists to fix comes back.
|
||||
p.router.GET("/health", healthHandler(p))
|
||||
|
||||
// /ready is a readiness probe. It returns 200 only after main has
|
||||
// flipped the package-level `ready` flag — which happens after route
|
||||
// registration, agent-registry seeding, the HTTP listener accepting
|
||||
// connections, and (when KANDEV_E2E_MOCK is set) the testharness routes
|
||||
// being mounted. Before that, return 503 so callers (including the e2e
|
||||
// fixture's readiness wait and Kubernetes' readinessProbe) keep polling
|
||||
// instead of racing ahead and hitting 404s on routes that aren't wired
|
||||
// yet. See docs/specs/health-endpoint-version/spec.md for the exact
|
||||
// contract this endpoint now owns.
|
||||
p.router.GET("/ready", readyHandler(p))
|
||||
|
||||
// /api/v1/features is a public, unauthenticated read of the runtime
|
||||
// feature-flag map. The frontend SSR-fetches it once per page render to
|
||||
// decide whether to mount Office (and any future flagged feature). The
|
||||
@@ -763,7 +775,7 @@ func registerRoutes(p routeParams) {
|
||||
} else {
|
||||
p.router.NoRoute(func(c *gin.Context) {
|
||||
path := c.Request.URL.Path
|
||||
if strings.HasPrefix(path, "/api/") || path == "/ws" || path == "/health" {
|
||||
if strings.HasPrefix(path, "/api/") || path == "/ws" || path == "/health" || path == "/ready" {
|
||||
c.AbortWithStatus(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
@@ -787,42 +799,62 @@ func registerRoutes(p routeParams) {
|
||||
}
|
||||
}
|
||||
|
||||
// healthHandler serves GET /health. The response always carries the
|
||||
// process's running version — in both the ready and not-ready bodies — so an
|
||||
// operator can identify the build of a backend that is stuck starting, and so
|
||||
// a monitor never needs a credential to read it (see docs/specs/
|
||||
// health-endpoint-version/spec.md).
|
||||
// healthHandler serves GET /health, the liveness probe. It always answers
|
||||
// 200 with the process's running version — regardless of the `ready` flag —
|
||||
// mirroring the bootstrap handler's /health contract (httpserver.go) so the
|
||||
// launcher's healthcheck never depends on startup progress. See docs/specs/
|
||||
// health-endpoint-version/spec.md for the exact response contract.
|
||||
func healthHandler(p routeParams) gin.HandlerFunc {
|
||||
// p.version is normally the package-level Version wired in by run()'s
|
||||
// registerRoutes call. Falling back here keeps the never-empty guarantee
|
||||
// (AC-11) true regardless of that call-site wiring, since standing up
|
||||
// run()'s full DI graph in a test just to exercise that one field
|
||||
// assignment is out of proportion to this change.
|
||||
version := p.version
|
||||
if version == "" {
|
||||
version = Version
|
||||
}
|
||||
version := resolveVersion(p)
|
||||
return func(c *gin.Context) {
|
||||
if !ready.Load() {
|
||||
c.JSON(http.StatusServiceUnavailable, gin.H{
|
||||
"status": startingStatus,
|
||||
"service": kandevName,
|
||||
versionFieldKey: version,
|
||||
})
|
||||
return
|
||||
}
|
||||
if token := desktopHealthToken(); token != "" {
|
||||
c.Header(desktopHealthTokenHeader, token)
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"status": "ok",
|
||||
"service": kandevName,
|
||||
statusKey: "ok",
|
||||
serviceFieldKey: kandevName,
|
||||
"mode": "websocket+http",
|
||||
versionFieldKey: version,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// readyHandler serves GET /ready, the readiness probe. The response always
|
||||
// carries the process's running version — in both the ready and not-ready
|
||||
// bodies — so an operator can identify the build of a backend that is stuck
|
||||
// starting, and so a monitor never needs a credential to read it (see
|
||||
// docs/specs/health-endpoint-version/spec.md).
|
||||
func readyHandler(p routeParams) gin.HandlerFunc {
|
||||
version := resolveVersion(p)
|
||||
return func(c *gin.Context) {
|
||||
if !ready.Load() {
|
||||
c.JSON(http.StatusServiceUnavailable, gin.H{
|
||||
statusKey: startingStatus,
|
||||
serviceFieldKey: kandevName,
|
||||
versionFieldKey: version,
|
||||
})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
statusKey: "ok",
|
||||
serviceFieldKey: kandevName,
|
||||
versionFieldKey: version,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// resolveVersion returns p.version, falling back to the package-level
|
||||
// Version. p.version is normally wired in by run()'s registerRoutes call;
|
||||
// the fallback keeps the never-empty guarantee (AC-11) true regardless of
|
||||
// that call-site wiring, since standing up run()'s full DI graph in a test
|
||||
// just to exercise that one field assignment is out of proportion.
|
||||
func resolveVersion(p routeParams) string {
|
||||
if p.version != "" {
|
||||
return p.version
|
||||
}
|
||||
return Version
|
||||
}
|
||||
|
||||
func desktopHealthToken() string {
|
||||
return strings.TrimSpace(os.Getenv(desktopHealthTokenEnv))
|
||||
}
|
||||
|
||||
@@ -2,9 +2,12 @@ package backendapp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net"
|
||||
"net/http"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/config"
|
||||
@@ -12,6 +15,107 @@ import (
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
// errServerBindFailed signals that the HTTP listener could not be bound.
|
||||
// startGatewayAndServe checks for it with errors.Is so it can skip the
|
||||
// "Failed to start orchestrator" log for what is actually a bind failure —
|
||||
// bindBootstrapListeners (via startHTTPServers) already logs the specific
|
||||
// cause at the appropriate level.
|
||||
var errServerBindFailed = errors.New("server bind failed")
|
||||
|
||||
// handlerSwitch is an http.Handler whose effective implementation can be
|
||||
// swapped at runtime without closing or rebinding the underlying listener.
|
||||
// Startup constructs it holding the bootstrap handler and later Store()s the
|
||||
// fully wired Gin router once buildHTTPServer returns, so exactly one
|
||||
// *http.Server/*serverListeners pair flows from bind through shutdown.
|
||||
type handlerSwitch struct {
|
||||
current atomic.Pointer[http.Handler]
|
||||
}
|
||||
|
||||
// newHandlerSwitch returns a handlerSwitch already serving initial.
|
||||
func newHandlerSwitch(initial http.Handler) *handlerSwitch {
|
||||
hs := &handlerSwitch{}
|
||||
hs.Store(initial)
|
||||
return hs
|
||||
}
|
||||
|
||||
// Store replaces the handler that serves subsequent requests.
|
||||
func (hs *handlerSwitch) Store(h http.Handler) {
|
||||
hs.current.Store(&h)
|
||||
}
|
||||
|
||||
func (hs *handlerSwitch) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
(*hs.current.Load()).ServeHTTP(w, r)
|
||||
}
|
||||
|
||||
// newBootstrapHandler answers every request while the real router is still
|
||||
// being built. GET /health always returns 200 (plus the desktop health
|
||||
// token header, if configured) so the launcher's liveness probe
|
||||
// (internal/launcher/health.go) succeeds regardless of startup progress —
|
||||
// making liveness depend on readiness here would bring back the crash loop
|
||||
// this handler exists to fix. Every other path returns a deterministic 503
|
||||
// with a parseable "starting" body instead of hanging, resetting, or 404ing.
|
||||
// GET /health's body is identical to healthHandler's in helpers.go (status
|
||||
// "ok", service, mode, version) since /health is a pure liveness probe and
|
||||
// its shape must not depend on startup progress.
|
||||
func newBootstrapHandler(version string) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodGet || r.URL.Path != "/health" {
|
||||
body := map[string]any{
|
||||
statusKey: startingStatus,
|
||||
serviceFieldKey: kandevName,
|
||||
versionFieldKey: version,
|
||||
}
|
||||
writeBootstrapJSON(w, http.StatusServiceUnavailable, body)
|
||||
return
|
||||
}
|
||||
if token := desktopHealthToken(); token != "" {
|
||||
w.Header().Set(desktopHealthTokenHeader, token)
|
||||
}
|
||||
writeBootstrapJSON(w, http.StatusOK, map[string]any{
|
||||
statusKey: "ok",
|
||||
serviceFieldKey: kandevName,
|
||||
"mode": "websocket+http",
|
||||
versionFieldKey: version,
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
func writeBootstrapJSON(w http.ResponseWriter, status int, body map[string]any) {
|
||||
w.Header().Set("Content-Type", "application/json; charset=utf-8")
|
||||
w.WriteHeader(status)
|
||||
_ = json.NewEncoder(w).Encode(body)
|
||||
}
|
||||
|
||||
// bindBootstrapListeners binds the configured HTTP addresses immediately,
|
||||
// before any startup work runs, and serves them with the bootstrap handler.
|
||||
// The returned handlerSwitch is later Store()d with the real router; the
|
||||
// returned *http.Server and *serverListeners are the same instances that
|
||||
// must flow unmodified into awaitShutdown, so shutdown needs no changes.
|
||||
func bindBootstrapListeners(cfg *config.Config, log *logger.Logger, version string) (*handlerSwitch, *http.Server, *serverListeners, error) {
|
||||
handler := newHandlerSwitch(newBootstrapHandler(version))
|
||||
server := &http.Server{
|
||||
Handler: handler,
|
||||
ReadTimeout: cfg.Server.ReadTimeoutDuration(),
|
||||
WriteTimeout: cfg.Server.WriteTimeoutDuration(),
|
||||
}
|
||||
|
||||
port := resolvedHTTPPort(cfg)
|
||||
hosts, err := cfg.Server.ResolvedBinds()
|
||||
if err != nil {
|
||||
log.Error("Invalid server bind configuration", zap.Error(err))
|
||||
return nil, nil, nil, errServerBindFailed
|
||||
}
|
||||
listeners, ok := startHTTPServers(server, hosts, port, log)
|
||||
if !ok {
|
||||
return nil, nil, nil, errServerBindFailed
|
||||
}
|
||||
|
||||
log.Info("HTTP listener bound; serving liveness probe while startup continues",
|
||||
zap.Int("port", port), zap.Strings("hosts", hosts))
|
||||
|
||||
return handler, server, listeners, nil
|
||||
}
|
||||
|
||||
// serverBindRetryInterval is how often the background retry loop re-attempts a
|
||||
// bind for an address that failed the initial bind (e.g. a tailnet IP that only
|
||||
// exists once tailscaled comes up). It is a var so tests can shorten it.
|
||||
@@ -162,21 +266,3 @@ func (sl *serverListeners) Stop() {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// probeAddr returns an address the readiness probe can dial. It prefers a bound
|
||||
// address that resolves to loopback (always reachable from the box without
|
||||
// depending on a routable interface), falling back to the first bound address.
|
||||
func (sl *serverListeners) probeAddr() string {
|
||||
sl.mu.Lock()
|
||||
defer sl.mu.Unlock()
|
||||
if len(sl.bound) == 0 {
|
||||
return ""
|
||||
}
|
||||
for _, a := range sl.bound {
|
||||
probe := serverProbeAddr(a)
|
||||
if host, _, err := net.SplitHostPort(probe); err == nil && config.IsLoopbackHost(host) {
|
||||
return probe
|
||||
}
|
||||
}
|
||||
return serverProbeAddr(sl.bound[0])
|
||||
}
|
||||
|
||||
@@ -260,25 +260,3 @@ func TestServerListenersStopClosesListenersAndDrains(t *testing.T) {
|
||||
t.Fatal("expected connection failure on 127.0.0.2 after shutdown")
|
||||
}
|
||||
}
|
||||
|
||||
func TestProbeAddrPrefersLoopback(t *testing.T) {
|
||||
sl := &serverListeners{bound: []string{"100.64.0.1:8080", "127.0.0.1:8080"}}
|
||||
if got := sl.probeAddr(); got != "127.0.0.1:8080" {
|
||||
t.Fatalf("probeAddr() = %q, want 127.0.0.1:8080", got)
|
||||
}
|
||||
|
||||
wildcard := &serverListeners{bound: []string{"0.0.0.0:8080"}}
|
||||
if got := wildcard.probeAddr(); got != "127.0.0.1:8080" {
|
||||
t.Fatalf("probeAddr() wildcard = %q, want 127.0.0.1:8080", got)
|
||||
}
|
||||
|
||||
wildcard6 := &serverListeners{bound: []string{"[::]:8080"}}
|
||||
if got := wildcard6.probeAddr(); got != "[::1]:8080" {
|
||||
t.Fatalf("probeAddr() ipv6 wildcard = %q, want [::1]:8080", got)
|
||||
}
|
||||
|
||||
tailnetOnly := &serverListeners{bound: []string{"100.64.0.1:8080"}}
|
||||
if got := tailnetOnly.probeAddr(); got != "100.64.0.1:8080" {
|
||||
t.Fatalf("probeAddr() tailnet-only = %q, want 100.64.0.1:8080", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -176,12 +176,18 @@ func parseBackendFlags(args []string) (backendFlags, func(), error) {
|
||||
return out, flags.Usage, flags.Parse(args)
|
||||
}
|
||||
|
||||
// ready is the readiness flag consulted by the GET /health handler.
|
||||
// Until it flips true, /health returns 503 — so callers polling for
|
||||
// readiness (Playwright fixtures, container orchestrators, manual
|
||||
// curl loops) keep waiting instead of racing ahead. main() flips it
|
||||
// after every route is mounted, the agent registry is seeded, and
|
||||
// the HTTP listener is accepting connections.
|
||||
// ready is the readiness flag consulted by the GET /ready handler. Until it
|
||||
// flips true, /ready returns 503 — so callers polling for readiness
|
||||
// (Playwright fixtures, container orchestrators, manual curl loops) keep
|
||||
// waiting instead of racing ahead of route registration. GET /health is a
|
||||
// liveness probe and is unaffected by this flag: it answers 200 as soon as
|
||||
// the listener is bound (see bindBootstrapListeners), because the launcher's
|
||||
// healthcheck must succeed regardless of startup progress — making it depend
|
||||
// on readiness would bring back the crash loop docs/specs/startup-listener-
|
||||
// before-recovery/spec.md exists to fix. startGatewayAndServe flips this flag
|
||||
// before swapping the bootstrap handler for the fully wired router (in that
|
||||
// order — see the comment at the call site), so a request racing the swap
|
||||
// never observes the real router with ready still false.
|
||||
var ready atomic.Bool
|
||||
|
||||
// Run contains all startup logic and returns 0 on success or 1 on fatal error.
|
||||
@@ -741,14 +747,21 @@ func startAgentInfrastructure(
|
||||
agentSettingsController, lifecycleMgr, agentRegistry, orchestratorSvc, msgCreator, repoCloner, agentctlBinaryPath, addRuntimeCleanup, runCleanups, cancelContext, restoreCleanups)
|
||||
}
|
||||
|
||||
// startOrchestratorAndAutomationConsumers establishes the event-consumer
|
||||
// chain in dependency order. The GitHub poller performs an immediate sweep on
|
||||
// start, so both downstream consumers must be ready before it is started.
|
||||
// startOrchestratorAndAutomationConsumers establishes the startup chain in
|
||||
// dependency order: the HTTP listener must be bound (so the launcher's
|
||||
// liveness probe can succeed) before the orchestrator runs its startup
|
||||
// recovery sweeps, which can run long. The GitHub poller performs an
|
||||
// immediate sweep on start, so both downstream consumers must be ready
|
||||
// before it is started.
|
||||
func startOrchestratorAndAutomationConsumers(
|
||||
bindListeners func() error,
|
||||
startOrchestrator func() error,
|
||||
startAutomation func(),
|
||||
startGitHubPoller func(),
|
||||
) error {
|
||||
if err := bindListeners(); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := startOrchestrator(); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -757,6 +770,23 @@ func startOrchestratorAndAutomationConsumers(
|
||||
return nil
|
||||
}
|
||||
|
||||
// closeBoundListeners releases the HTTP listener(s) bound by bindListeners
|
||||
// when a later startup step fails and startGatewayAndServe returns early.
|
||||
// listeners.Stop only halts the background bind-retry loop; the bound TCP
|
||||
// sockets are released by the shared http.Server (see serverListeners's doc
|
||||
// comment), so an early return that skips this leaves the port held by this
|
||||
// process even though the caller sees a start failure. server is non-nil
|
||||
// whenever listeners is, since bindListeners assigns both together.
|
||||
func closeBoundListeners(server *http.Server, listeners *serverListeners, log *logger.Logger) {
|
||||
if listeners == nil {
|
||||
return
|
||||
}
|
||||
listeners.Stop()
|
||||
if err := server.Close(); err != nil {
|
||||
log.Warn("failed to close HTTP listeners after startup failure", zap.Error(err))
|
||||
}
|
||||
}
|
||||
|
||||
// startGatewayAndServe sets up the WebSocket gateway, HTTP routes, starts the server,
|
||||
// and blocks until a shutdown signal.
|
||||
//
|
||||
@@ -873,7 +903,22 @@ func startGatewayAndServe(
|
||||
services.Plugins.SetUtilityAgent(pluginsUtilityAgentAdapter{svc: services.Utility, userSvc: services.User}, pluginsHostUtilityAdapter{mgr: hostUtilityMgr})
|
||||
}
|
||||
|
||||
var (
|
||||
handler *handlerSwitch
|
||||
server *http.Server
|
||||
listeners *serverListeners
|
||||
)
|
||||
bindListeners := func() error {
|
||||
h, s, l, err := bindBootstrapListeners(cfg, log, Version)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
handler, server, listeners = h, s, l
|
||||
return nil
|
||||
}
|
||||
|
||||
if err := startOrchestratorAndAutomationConsumers(
|
||||
bindListeners,
|
||||
func() error { return orchestratorSvc.Start(ctx) },
|
||||
func() {
|
||||
if services.Automation == nil {
|
||||
@@ -894,7 +939,10 @@ func startGatewayAndServe(
|
||||
log.Info("GitHub poller started")
|
||||
},
|
||||
); err != nil {
|
||||
log.Error("Failed to start orchestrator", zap.Error(err))
|
||||
if !errors.Is(err, errServerBindFailed) {
|
||||
log.Error("Failed to start orchestrator", zap.Error(err))
|
||||
}
|
||||
closeBoundListeners(server, listeners, log)
|
||||
return false
|
||||
}
|
||||
log.Info("Orchestrator initialized")
|
||||
@@ -921,6 +969,7 @@ func startGatewayAndServe(
|
||||
// ============================================
|
||||
runProcessorSvc, ok := initOfficeServices(ctx, cfg, log, repos, services, orchestratorSvc, eventBus, agentctlBinaryPath, addCleanup, lifecycleMgr, agentRegistry)
|
||||
if !ok {
|
||||
closeBoundListeners(server, listeners, log)
|
||||
return false
|
||||
}
|
||||
scheduling := startSchedulingRuntime(
|
||||
@@ -983,6 +1032,7 @@ func startGatewayAndServe(
|
||||
)
|
||||
if err != nil {
|
||||
log.Error("Failed to initialize storage maintenance", zap.Error(err))
|
||||
closeBoundListeners(server, listeners, log)
|
||||
return false
|
||||
}
|
||||
hostUtilityMgr.SetTemporaryArtifactRegistry(storageComposition.tempArtifacts)
|
||||
@@ -1046,23 +1096,18 @@ func startGatewayAndServe(
|
||||
// ============================================
|
||||
// HTTP SERVER
|
||||
// ============================================
|
||||
server, err := buildHTTPServer(cfg, log, gateway, repos, services, agentSettingsController,
|
||||
// The listener was already bound (and is serving the bootstrap handler)
|
||||
// by bindListeners above, before orchestratorSvc.Start ran its startup
|
||||
// recovery sweeps. Build the real router now and swap it in on the same,
|
||||
// already-bound handler and listeners — no second bind, no window where
|
||||
// the socket is closed and reopened.
|
||||
builtServer, err := buildHTTPServer(cfg, log, gateway, repos, services, agentSettingsController,
|
||||
lifecycleMgr, eventBus, orchestratorSvc, notificationCtrl, msgCreator, agentRegistry, hostUtilityMgr,
|
||||
addCleanup, repoCloner, systemSvc, storageComposition.workspaceRestorer,
|
||||
storageComposition.tempArtifacts, dbPool, agentRuntimeAvailability)
|
||||
if err != nil {
|
||||
log.Error("Failed to build HTTP server", zap.Error(err))
|
||||
return false
|
||||
}
|
||||
|
||||
port := resolvedHTTPPort(cfg)
|
||||
hosts, err := cfg.Server.ResolvedBinds()
|
||||
if err != nil {
|
||||
log.Error("Invalid server bind configuration", zap.Error(err))
|
||||
return false
|
||||
}
|
||||
listeners, ok := startHTTPServers(server, hosts, port, log)
|
||||
if !ok {
|
||||
closeBoundListeners(server, listeners, log)
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -1072,17 +1117,31 @@ func startGatewayAndServe(
|
||||
zap.String("http", "/api/v1"),
|
||||
)
|
||||
|
||||
// Flip the readiness flag once the HTTP listener is actually
|
||||
// accepting connections, not just "spawned". Serve runs in a goroutine
|
||||
// after we bind the socket above; probe a reachable local listener with a
|
||||
// short retry loop — once a single connect succeeds, the kernel queue is up
|
||||
// and any subsequent /health call will land on a wired route.
|
||||
go waitListenerThenMarkReady(listeners.probeAddr(), log)
|
||||
// Flip readiness before swapping in the fully wired router — see
|
||||
// publishReadiness for why the order matters and
|
||||
// TestPublishReadinessFlipsReadyBeforeSwappingHandler for the regression
|
||||
// test pinning it.
|
||||
publishReadiness(func() { ready.Store(true) }, func() { handler.Store(builtServer.Handler) })
|
||||
|
||||
awaitShutdown(server, listeners, scheduling, orchestratorSvc, lifecycleMgr, runCleanups, log)
|
||||
return true
|
||||
}
|
||||
|
||||
// publishReadiness flips readiness and then swaps in the fully wired router,
|
||||
// in that order — never the reverse. A request landing between the two must
|
||||
// never observe the real router while ready is still false: readyHandler
|
||||
// gates on ready.Load(), so the reverse order would 503 a client on a router
|
||||
// that is otherwise already fully up, recreating the exact flap
|
||||
// docs/specs/startup-listener-before-recovery/spec.md exists to prevent.
|
||||
// Flipping ready first means any request in that window still hits the
|
||||
// bootstrap handler, whose /health branch is unconditionally 200 and whose
|
||||
// every other path (including /ready) is a deterministic 503 "starting" —
|
||||
// never a stale ready=false read through the real router.
|
||||
func publishReadiness(markReady func(), swapHandler func()) {
|
||||
markReady()
|
||||
swapHandler()
|
||||
}
|
||||
|
||||
// serverListenAddr formats the host and port into a listen address, binding
|
||||
// all interfaces when host is empty.
|
||||
func serverListenAddr(host string, port int) string {
|
||||
@@ -1093,58 +1152,6 @@ func serverListenAddr(host string, port int) string {
|
||||
return net.JoinHostPort(host, fmt.Sprint(port))
|
||||
}
|
||||
|
||||
// serverProbeAddr rewrites a listen address into a loopback address suitable
|
||||
// for local probing, mapping wildcard hosts to 127.0.0.1 / ::1.
|
||||
func serverProbeAddr(listenAddr string) string {
|
||||
host, port, err := net.SplitHostPort(listenAddr)
|
||||
if err != nil {
|
||||
return listenAddr
|
||||
}
|
||||
switch host {
|
||||
case "", "0.0.0.0":
|
||||
host = "127.0.0.1"
|
||||
case "::":
|
||||
// Preserve the address family: an IPv6-only wildcard listener isn't
|
||||
// reachable via 127.0.0.1, so probe the IPv6 loopback instead.
|
||||
host = "::1"
|
||||
}
|
||||
return net.JoinHostPort(host, port)
|
||||
}
|
||||
|
||||
// waitListenerThenMarkReady probes the local HTTP listener until a
|
||||
// connect succeeds, then flips the package-level `ready` flag so the
|
||||
// /health handler stops returning 503. Runs in its own goroutine so
|
||||
// the caller can proceed into awaitShutdown.
|
||||
//
|
||||
// The probe budget is generous (30s) — under heavy parallel-suite
|
||||
// load the OS scheduler can delay the listen goroutine for a couple
|
||||
// of seconds. If the budget expires the flag still flips, so the
|
||||
// backend doesn't permanently advertise "not ready" if probing fails
|
||||
// for some unrelated reason (e.g. an iptables hiccup on a dev box).
|
||||
func waitListenerThenMarkReady(listenAddr string, log *logger.Logger) {
|
||||
addr := serverProbeAddr(listenAddr)
|
||||
deadline := time.Now().Add(30 * time.Second)
|
||||
ticker := time.NewTicker(50 * time.Millisecond)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
conn, err := net.DialTimeout("tcp", addr, 500*time.Millisecond)
|
||||
if err == nil {
|
||||
_ = conn.Close()
|
||||
// A successful TCP dial proves the listener is bound.
|
||||
ready.Store(true)
|
||||
log.Info("backend ready", zap.String("addr", addr))
|
||||
return
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
log.Warn("backend readiness probe never connected; flipping ready anyway",
|
||||
zap.String("addr", addr))
|
||||
ready.Store(true)
|
||||
return
|
||||
}
|
||||
<-ticker.C
|
||||
}
|
||||
}
|
||||
|
||||
// initOfficeServices constructs the run processor service for every backend and
|
||||
// adds Office-only services, reconciliation, and subscribers when the feature
|
||||
// is enabled. Global run and cron scheduling starts separately so this feature
|
||||
|
||||
@@ -39,45 +39,56 @@ func TestDesktopHealthTokenTrimsEnv(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHealthHandlerEchoesConfiguredToken(t *testing.T) {
|
||||
// TestHealthHandlerAlwaysOkRegardlessOfReadiness is the regression test for
|
||||
// R1-1: /health is a liveness probe and must answer 200 (with the desktop
|
||||
// health token echoed, when configured) whether or not the `ready` flag has
|
||||
// flipped. Gating it on readiness would bring back the crash loop docs/specs/
|
||||
// startup-listener-before-recovery/spec.md exists to fix.
|
||||
func TestHealthHandlerAlwaysOkRegardlessOfReadiness(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
t.Setenv(desktopHealthTokenEnv, "route-health-token")
|
||||
ready.Store(true)
|
||||
t.Cleanup(func() { ready.Store(false) })
|
||||
|
||||
router := gin.New()
|
||||
router.GET("/health", healthHandler(routeParams{}))
|
||||
response := httptest.NewRecorder()
|
||||
request := httptest.NewRequest(http.MethodGet, "/health", nil)
|
||||
router.ServeHTTP(response, request)
|
||||
for _, readyState := range []bool{false, true} {
|
||||
ready.Store(readyState)
|
||||
t.Cleanup(func() { ready.Store(false) })
|
||||
|
||||
if response.Code != http.StatusOK {
|
||||
t.Fatalf("health status = %d, want %d", response.Code, http.StatusOK)
|
||||
}
|
||||
if got := response.Header().Get(desktopHealthTokenHeader); got != "route-health-token" {
|
||||
t.Fatalf("health token header = %q, want route-health-token", got)
|
||||
router := gin.New()
|
||||
router.GET("/health", healthHandler(routeParams{}))
|
||||
response := httptest.NewRecorder()
|
||||
request := httptest.NewRequest(http.MethodGet, "/health", nil)
|
||||
router.ServeHTTP(response, request)
|
||||
|
||||
if response.Code != http.StatusOK {
|
||||
t.Fatalf("ready=%v: health status = %d, want %d", readyState, response.Code, http.StatusOK)
|
||||
}
|
||||
if got := response.Header().Get(desktopHealthTokenHeader); got != "route-health-token" {
|
||||
t.Fatalf("ready=%v: health token header = %q, want route-health-token", readyState, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestServerProbeAddr(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
listenAddr string
|
||||
want string
|
||||
}{
|
||||
{name: "port-only address probes loopback", listenAddr: ":38429", want: "127.0.0.1:38429"},
|
||||
{name: "wildcard ipv4 probes loopback", listenAddr: "0.0.0.0:38429", want: "127.0.0.1:38429"},
|
||||
{name: "wildcard ipv6 probes ipv6 loopback", listenAddr: "[::]:38429", want: "[::1]:38429"},
|
||||
{name: "loopback address probes itself", listenAddr: "127.0.0.1:38429", want: "127.0.0.1:38429"},
|
||||
{name: "ipv6 loopback probes itself", listenAddr: "[::1]:38429", want: "[::1]:38429"},
|
||||
// TestReadyHandlerGatesOnReadyFlag is the regression test for the task-02
|
||||
// requirement that readiness (unlike liveness) reports not-ready during a
|
||||
// blocked startup and ready once startup completes.
|
||||
func TestReadyHandlerGatesOnReadyFlag(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
ready.Store(false)
|
||||
t.Cleanup(func() { ready.Store(false) })
|
||||
|
||||
router := gin.New()
|
||||
router.GET("/ready", readyHandler(routeParams{}))
|
||||
|
||||
response := httptest.NewRecorder()
|
||||
router.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/ready", nil))
|
||||
if response.Code != http.StatusServiceUnavailable {
|
||||
t.Fatalf("not-ready status = %d, want %d", response.Code, http.StatusServiceUnavailable)
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := serverProbeAddr(tt.listenAddr); got != tt.want {
|
||||
t.Fatalf("serverProbeAddr(%q) = %q, want %q", tt.listenAddr, got, tt.want)
|
||||
}
|
||||
})
|
||||
ready.Store(true)
|
||||
response = httptest.NewRecorder()
|
||||
router.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/ready", nil))
|
||||
if response.Code != http.StatusOK {
|
||||
t.Fatalf("ready status = %d, want %d", response.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ func TestStartOrchestratorAndAutomationConsumersOrder(t *testing.T) {
|
||||
var order []string
|
||||
|
||||
err := startOrchestratorAndAutomationConsumers(
|
||||
func() error { order = append(order, "bind"); return nil },
|
||||
func() error {
|
||||
order = append(order, "orchestrator")
|
||||
return nil
|
||||
@@ -21,7 +22,7 @@ func TestStartOrchestratorAndAutomationConsumersOrder(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("start sequence returned error: %v", err)
|
||||
}
|
||||
want := []string{"orchestrator", "automation", "github"}
|
||||
want := []string{"bind", "orchestrator", "automation", "github"}
|
||||
if !reflect.DeepEqual(order, want) {
|
||||
t.Fatalf("start order = %v, want %v", order, want)
|
||||
}
|
||||
@@ -32,6 +33,7 @@ func TestStartOrchestratorAndAutomationConsumersStopsAfterOrchestratorFailure(t
|
||||
var automationStarted, githubStarted bool
|
||||
|
||||
err := startOrchestratorAndAutomationConsumers(
|
||||
func() error { return nil },
|
||||
func() error { return startErr },
|
||||
func() { automationStarted = true },
|
||||
func() { githubStarted = true },
|
||||
@@ -44,3 +46,49 @@ func TestStartOrchestratorAndAutomationConsumersStopsAfterOrchestratorFailure(t
|
||||
t.Fatalf("downstream consumers started after orchestrator failure: automation=%v github=%v", automationStarted, githubStarted)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStartOrchestratorAndAutomationConsumersStopsAfterBindFailure pins the
|
||||
// bind-before-startup ordering invariant (docs/plans/startup-listener-before-
|
||||
// recovery/task-03-ordering-guard.md): a bind failure must short-circuit
|
||||
// before the orchestrator, automation, or GitHub poller ever start, exactly
|
||||
// like an orchestrator-start failure does today.
|
||||
func TestStartOrchestratorAndAutomationConsumersStopsAfterBindFailure(t *testing.T) {
|
||||
bindErr := errors.New("bind unavailable")
|
||||
var orchestratorStarted, automationStarted, githubStarted bool
|
||||
|
||||
err := startOrchestratorAndAutomationConsumers(
|
||||
func() error { return bindErr },
|
||||
func() error { orchestratorStarted = true; return nil },
|
||||
func() { automationStarted = true },
|
||||
func() { githubStarted = true },
|
||||
)
|
||||
|
||||
if !errors.Is(err, bindErr) {
|
||||
t.Fatalf("start sequence error = %v, want %v", err, bindErr)
|
||||
}
|
||||
if orchestratorStarted || automationStarted || githubStarted {
|
||||
t.Fatalf("downstream steps started after bind failure: orchestrator=%v automation=%v github=%v",
|
||||
orchestratorStarted, automationStarted, githubStarted)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPublishReadinessFlipsReadyBeforeSwappingHandler is the regression test
|
||||
// for R1-2 (docs/specs/health-endpoint-version/spec.md's 2026-08-22
|
||||
// amendment): startGatewayAndServe must flip the readiness flag before
|
||||
// swapping in the fully wired router, never the reverse, or a request racing
|
||||
// the swap could observe the real router with ready still false and get a
|
||||
// spurious 503 — recreating the exact flap docs/specs/startup-listener-
|
||||
// before-recovery/spec.md exists to prevent.
|
||||
func TestPublishReadinessFlipsReadyBeforeSwappingHandler(t *testing.T) {
|
||||
var order []string
|
||||
|
||||
publishReadiness(
|
||||
func() { order = append(order, "ready") },
|
||||
func() { order = append(order, "handler") },
|
||||
)
|
||||
|
||||
want := []string{"ready", "handler"}
|
||||
if !reflect.DeepEqual(order, want) {
|
||||
t.Fatalf("publishReadiness order = %v, want %v", order, want)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,6 +71,11 @@ func runDev(ctx context.Context, opts Options) int {
|
||||
if readyURL != "" {
|
||||
cfg.ports.BackendURL = readyURL
|
||||
}
|
||||
if err := waitForReadyFn(ctx, cfg.ports.BackendURL, backend); err != nil {
|
||||
supervisor.shutdown("backend readiness failure")
|
||||
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
|
||||
return 1
|
||||
}
|
||||
fmt.Printf("[kandev] backend ready at %s\n", cfg.ports.BackendURL)
|
||||
|
||||
webURL := fmt.Sprintf("http://localhost:%d", cfg.ports.WebPort)
|
||||
|
||||
@@ -28,6 +28,7 @@ func TestRunDevLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
oldWaitForURL := waitForURLFn
|
||||
oldOpenBrowser := openBrowserFn
|
||||
oldStartProcess := startProcessFn
|
||||
@@ -36,6 +37,7 @@ func TestRunDevLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
|
||||
attachSignalsFn = oldAttachSignals
|
||||
launchBackendFn = oldLaunchBackend
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
waitForURLFn = oldWaitForURL
|
||||
openBrowserFn = oldOpenBrowser
|
||||
startProcessFn = oldStartProcess
|
||||
@@ -66,6 +68,10 @@ func TestRunDevLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
|
||||
readyURL = "http://127.0.0.2:" + itoa(backendCfg.Ports.BackendPort)
|
||||
return readyURL, nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, url string, _ childState) error {
|
||||
events = append(events, "wait-ready:"+url)
|
||||
return nil
|
||||
}
|
||||
waitForURLFn = func(_ context.Context, url string, _ childState, _ time.Duration) error {
|
||||
events = append(events, "wait-url:"+url)
|
||||
return nil
|
||||
@@ -92,6 +98,7 @@ func TestRunDevLaunchesBackendThenWebAndOpensBrowser(t *testing.T) {
|
||||
"attach-signals",
|
||||
"launch-backend",
|
||||
"wait-health:http://localhost:" + itoa(backendCfg.Ports.BackendPort),
|
||||
"wait-ready:" + readyURL,
|
||||
"start-web:web",
|
||||
"wait-url:http://localhost:" + itoa(backendCfg.Ports.WebPort),
|
||||
"open:" + readyURL,
|
||||
@@ -166,6 +173,7 @@ func TestRunDevUsesConfiguredHealthTimeoutForBackendAndWeb(t *testing.T) {
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
oldWaitForURL := waitForURLFn
|
||||
oldOpenBrowser := openBrowserFn
|
||||
oldStartProcess := startProcessFn
|
||||
@@ -174,6 +182,7 @@ func TestRunDevUsesConfiguredHealthTimeoutForBackendAndWeb(t *testing.T) {
|
||||
attachSignalsFn = oldAttachSignals
|
||||
launchBackendFn = oldLaunchBackend
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
waitForURLFn = oldWaitForURL
|
||||
openBrowserFn = oldOpenBrowser
|
||||
startProcessFn = oldStartProcess
|
||||
@@ -191,6 +200,9 @@ func TestRunDevUsesConfiguredHealthTimeoutForBackendAndWeb(t *testing.T) {
|
||||
backendTimeout = timeout
|
||||
return "", nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
return nil
|
||||
}
|
||||
waitForURLFn = func(_ context.Context, _ string, _ childState, timeout time.Duration) error {
|
||||
webTimeout = timeout
|
||||
return nil
|
||||
@@ -218,6 +230,7 @@ func TestRunDevHeadlessSkipsBrowser(t *testing.T) {
|
||||
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
oldWaitForURL := waitForURLFn
|
||||
oldOpenBrowser := openBrowserFn
|
||||
oldStartProcess := startProcessFn
|
||||
@@ -225,6 +238,7 @@ func TestRunDevHeadlessSkipsBrowser(t *testing.T) {
|
||||
t.Cleanup(func() {
|
||||
launchBackendFn = oldLaunchBackend
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
waitForURLFn = oldWaitForURL
|
||||
openBrowserFn = oldOpenBrowser
|
||||
startProcessFn = oldStartProcess
|
||||
@@ -241,6 +255,9 @@ func TestRunDevHeadlessSkipsBrowser(t *testing.T) {
|
||||
waitForHealthFn = func(_ context.Context, _ backendEndpointSet, _ childState, _ time.Duration, _ string, _ func()) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
return nil
|
||||
}
|
||||
waitForURLFn = func(_ context.Context, _ string, _ childState, _ time.Duration) error {
|
||||
return nil
|
||||
}
|
||||
@@ -345,6 +362,71 @@ func TestRunDevShutsDownTreeOnBackendHealthFailure(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunDevShutsDownTreeOnBackendReadinessFailure is the R2-1 regression
|
||||
// test for dev mode: it must not launch the web child or open the browser
|
||||
// onto a backend that answered /health but never became ready.
|
||||
func TestRunDevShutsDownTreeOnBackendReadinessFailure(t *testing.T) {
|
||||
repo := makeRepoTree(t)
|
||||
t.Chdir(repo)
|
||||
t.Setenv("KANDEV_TASK_ID", "")
|
||||
t.Setenv("KANDEV_DATABASE_PATH", "")
|
||||
t.Setenv("KANDEV_HOME_DIR", "")
|
||||
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
oldWaitForURL := waitForURLFn
|
||||
oldOpenBrowser := openBrowserFn
|
||||
oldStartProcess := startProcessFn
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldStatusOutput := launcherStatusOutput
|
||||
t.Cleanup(func() {
|
||||
launchBackendFn = oldLaunchBackend
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
waitForURLFn = oldWaitForURL
|
||||
openBrowserFn = oldOpenBrowser
|
||||
startProcessFn = oldStartProcess
|
||||
attachSignalsFn = oldAttachSignals
|
||||
launcherStatusOutput = oldStatusOutput
|
||||
})
|
||||
|
||||
var output strings.Builder
|
||||
launcherStatusOutput = &output
|
||||
|
||||
browserOpened := false
|
||||
attachSignalsFn = func(_ *processSupervisor) {}
|
||||
launchBackendFn = func(_ backendLaunchConfig) (*restartableBackend, func(), error) {
|
||||
return &restartableBackend{exitCh: make(chan int, 1)}, func() {}, nil
|
||||
}
|
||||
waitForHealthFn = func(_ context.Context, _ backendEndpointSet, _ childState, _ time.Duration, _ string, _ func()) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
return errTestHealth
|
||||
}
|
||||
waitForURLFn = func(_ context.Context, _ string, _ childState, _ time.Duration) error {
|
||||
t.Fatal("waitForURL called after backend readiness failure")
|
||||
return nil
|
||||
}
|
||||
openBrowserFn = func(_ string) { browserOpened = true }
|
||||
startProcessFn = func(_ string, _ []string, _ string, _ []string, _ bool, _ string, _ *processSupervisor) (*managedProcess, func(), error) {
|
||||
t.Fatal("web child launched after backend readiness failure")
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
code := runDev(context.Background(), Options{Command: CommandDev})
|
||||
if code != 1 {
|
||||
t.Fatalf("runDev() = %d, want 1", code)
|
||||
}
|
||||
if !strings.Contains(output.String(), "graceful shutdown started") {
|
||||
t.Fatalf("supervisor shutdown not triggered on readiness failure:\n%s", output.String())
|
||||
}
|
||||
if browserOpened {
|
||||
t.Fatal("browser opened despite readiness failure")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunDevShutsDownTreeWhenWebChildExits(t *testing.T) {
|
||||
repo := makeRepoTree(t)
|
||||
t.Chdir(repo)
|
||||
@@ -354,6 +436,7 @@ func TestRunDevShutsDownTreeWhenWebChildExits(t *testing.T) {
|
||||
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
oldWaitForURL := waitForURLFn
|
||||
oldOpenBrowser := openBrowserFn
|
||||
oldStartProcess := startProcessFn
|
||||
@@ -362,6 +445,7 @@ func TestRunDevShutsDownTreeWhenWebChildExits(t *testing.T) {
|
||||
t.Cleanup(func() {
|
||||
launchBackendFn = oldLaunchBackend
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
waitForURLFn = oldWaitForURL
|
||||
openBrowserFn = oldOpenBrowser
|
||||
startProcessFn = oldStartProcess
|
||||
@@ -381,6 +465,9 @@ func TestRunDevShutsDownTreeWhenWebChildExits(t *testing.T) {
|
||||
waitForHealthFn = func(_ context.Context, _ backendEndpointSet, _ childState, _ time.Duration, _ string, _ func()) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
return nil
|
||||
}
|
||||
waitForURLFn = func(_ context.Context, _ string, _ childState, _ time.Duration) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -350,6 +350,57 @@ func safeProbeError(err error) string {
|
||||
return "network error"
|
||||
}
|
||||
|
||||
// waitForReady polls GET /ready until the backend reports readiness or the
|
||||
// process exits. Unlike waitForHealth, it has no timeout of its own and never
|
||||
// triggers a kill: /health (and its healthTimeoutReleaseMS/healthTimeoutDevMS
|
||||
// budget) already made the keep-or-kill decision by the time this runs. This
|
||||
// only decides when the bootstrap handler has handed off to the real router,
|
||||
// so the launcher can print "backend ready"/open a browser onto real content
|
||||
// instead of the bootstrap stub's 503 while startup recovery is still
|
||||
// in flight. Do not fold this into waitForHealth's timeout — that would
|
||||
// recreate the crash loop docs/specs/startup-listener-before-recovery/spec.md
|
||||
// exists to fix.
|
||||
func waitForReady(ctx context.Context, baseURL string, proc childState) error {
|
||||
readyURL := baseURL + "/ready"
|
||||
for ctx.Err() == nil {
|
||||
if exited, code := proc.Exited(); exited {
|
||||
return fmt.Errorf("backend exited (code %d) before it reported ready", code)
|
||||
}
|
||||
if probeReady(ctx, readyURL) {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return fmt.Errorf("backend readiness wait canceled at %s: %w", readyURL, err)
|
||||
}
|
||||
if exited, code := proc.Exited(); exited {
|
||||
return fmt.Errorf("backend exited (code %d) before it reported ready", code)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
case <-time.After(healthPollInterval):
|
||||
}
|
||||
}
|
||||
return fmt.Errorf("backend readiness wait canceled at %s: %w", readyURL, ctx.Err())
|
||||
}
|
||||
|
||||
// probeReady reports whether a single readiness request returned 2xx. Unlike
|
||||
// probeHealth, it does not check the desktop health token: readyHandler never
|
||||
// sets X-Kandev-Desktop-Health-Token (that header is /health-only, see
|
||||
// docs/specs/health-endpoint-version/spec.md AC-21).
|
||||
func probeReady(ctx context.Context, readyURL string) bool {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, readyURL, nil)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
resp, err := healthProbeClient.Do(req)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
return resp.StatusCode >= 200 && resp.StatusCode < 300
|
||||
}
|
||||
|
||||
func healthPort(baseURL string) string {
|
||||
parsed, err := url.Parse(baseURL)
|
||||
if err == nil && parsed.Port() != "" {
|
||||
|
||||
@@ -20,6 +20,15 @@ type fakeChild struct {
|
||||
|
||||
func (f fakeChild) Exited() (bool, int) { return f.exited, f.code }
|
||||
|
||||
type toggledChild struct {
|
||||
exited atomic.Bool
|
||||
code atomic.Int32
|
||||
}
|
||||
|
||||
func (c *toggledChild) Exited() (bool, int) {
|
||||
return c.exited.Load(), int(c.code.Load())
|
||||
}
|
||||
|
||||
func singleHealthTarget(url string) backendEndpointSet {
|
||||
return backendEndpointSet{
|
||||
bindHosts: []string{"localhost"},
|
||||
@@ -426,6 +435,161 @@ func TestWaitForHealthCallsOnFailureWhenContextCanceled(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForReadyReturnsNilOnReadyResponse(t *testing.T) {
|
||||
var calls atomic.Int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/ready" {
|
||||
t.Errorf("path = %q, want /ready", r.URL.Path)
|
||||
}
|
||||
// The first probe still reports 503 (bootstrap handler / not-ready-yet),
|
||||
// exercising the poll loop, before flipping to ready.
|
||||
if calls.Add(1) == 1 {
|
||||
w.WriteHeader(http.StatusServiceUnavailable)
|
||||
_, _ = w.Write([]byte(`{"status":"starting"}`))
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(`{"status":"ok"}`))
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
if err := waitForReady(context.Background(), srv.URL, fakeChild{}); err != nil {
|
||||
t.Fatalf("waitForReady() = %v, want nil", err)
|
||||
}
|
||||
if got := calls.Load(); got < 2 {
|
||||
t.Fatalf("probe calls = %d, want at least 2", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForReadyRejectsChildExitDuringSuccessfulProbe(t *testing.T) {
|
||||
requestStarted := make(chan struct{})
|
||||
responseRelease := make(chan struct{})
|
||||
var child toggledChild
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/ready" {
|
||||
t.Errorf("path = %q, want /ready", r.URL.Path)
|
||||
}
|
||||
close(requestStarted)
|
||||
<-responseRelease
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
var releaseOnce sync.Once
|
||||
t.Cleanup(func() {
|
||||
releaseOnce.Do(func() { close(responseRelease) })
|
||||
srv.Close()
|
||||
})
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
done <- waitForReady(ctx, srv.URL, &child)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-requestStarted:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("waitForReady did not issue a readiness request")
|
||||
}
|
||||
child.code.Store(7)
|
||||
child.exited.Store(true)
|
||||
releaseOnce.Do(func() { close(responseRelease) })
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
if err == nil {
|
||||
t.Fatal("waitForReady() = nil, want exit error when child exits during probe")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "code 7") {
|
||||
t.Fatalf("error = %v, want the backend exit code", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("waitForReady did not return after the readiness probe completed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForReadyIgnoresMissingDesktopHealthToken(t *testing.T) {
|
||||
// readyHandler never sets X-Kandev-Desktop-Health-Token (that header is
|
||||
// /health-only); probeReady must accept a bare 2xx regardless.
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
if err := waitForReady(context.Background(), srv.URL, fakeChild{}); err != nil {
|
||||
t.Fatalf("waitForReady() = %v, want nil", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForReadyKeepsPollingPastWhatWouldBeAHealthTimeout(t *testing.T) {
|
||||
// waitForReady has no timeout of its own: unlike waitForHealth's
|
||||
// healthTimeoutReleaseMS/healthTimeoutDevMS budget, a long readiness wait
|
||||
// (startup recovery still running) must not be treated as a failure.
|
||||
var calls atomic.Int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
calls.Add(1)
|
||||
w.WriteHeader(http.StatusServiceUnavailable)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 700*time.Millisecond)
|
||||
defer cancel()
|
||||
err := waitForReady(ctx, srv.URL, fakeChild{})
|
||||
if err == nil {
|
||||
t.Fatal("waitForReady() = nil, want the injected context deadline to end the wait")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "canceled") {
|
||||
t.Fatalf("error = %v, want a cancellation error", err)
|
||||
}
|
||||
if got := calls.Load(); got < 2 {
|
||||
t.Fatalf("probe calls = %d, want at least 2 (proves it kept polling past a single failure)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForReadyReturnsWhenContextCanceled(t *testing.T) {
|
||||
release := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
|
||||
select {
|
||||
case <-release:
|
||||
case <-r.Context().Done():
|
||||
}
|
||||
}))
|
||||
t.Cleanup(func() {
|
||||
close(release)
|
||||
srv.Close()
|
||||
})
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
done <- waitForReady(ctx, srv.URL, fakeChild{})
|
||||
}()
|
||||
cancel()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
if err == nil {
|
||||
t.Fatal("waitForReady() = nil, want cancellation error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "canceled") {
|
||||
t.Fatalf("error = %v, want a cancellation error", err)
|
||||
}
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("waitForReady ignored context cancellation")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForReadyFailsFastWhenBackendExited(t *testing.T) {
|
||||
err := waitForReady(context.Background(), "http://127.0.0.1:1", fakeChild{exited: true, code: 3})
|
||||
if err == nil {
|
||||
t.Fatal("waitForReady() = nil, want exit error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "code 3") {
|
||||
t.Fatalf("error = %v, want the backend exit code", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForHealthFailsFastWhenBackendExited(t *testing.T) {
|
||||
var failures int
|
||||
_, err := waitForHealth(
|
||||
|
||||
@@ -17,6 +17,7 @@ var (
|
||||
newSupervisorFn = newSupervisor
|
||||
launchBackendFn = launchRestartableBackend
|
||||
waitForHealthFn = waitForHealth
|
||||
waitForReadyFn = waitForReady
|
||||
attachSignalsFn = func(supervisor *processSupervisor) {
|
||||
supervisor.attachSignals()
|
||||
}
|
||||
@@ -173,6 +174,12 @@ func runManagedApp(ctx context.Context, cfg managedAppConfig) int {
|
||||
if readyURL != "" {
|
||||
cfg.Ports.BackendURL = readyURL
|
||||
}
|
||||
shutdownDebugf("runManagedApp backend healthy, waiting for readiness")
|
||||
if err := waitForReadyFn(ctx, cfg.Ports.BackendURL, backend); err != nil {
|
||||
supervisor.shutdown("backend readiness failure")
|
||||
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
|
||||
return 1
|
||||
}
|
||||
fmt.Printf("[kandev] backend ready at %s\n", cfg.Ports.BackendURL)
|
||||
|
||||
if !shouldOpenBrowser(cfg.Opts, cfg.Startup) {
|
||||
|
||||
@@ -2,6 +2,7 @@ package launcher
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
@@ -200,12 +201,14 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldStartParentWatch := startParentWatchFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
t.Cleanup(func() {
|
||||
newSupervisorFn = oldNewSupervisor
|
||||
launchBackendFn = oldLaunchBackend
|
||||
attachSignalsFn = oldAttachSignals
|
||||
startParentWatchFn = oldStartParentWatch
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
})
|
||||
|
||||
var events []string
|
||||
@@ -242,6 +245,10 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
events = append(events, "wait-health")
|
||||
return "", nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
events = append(events, "wait-ready")
|
||||
return nil
|
||||
}
|
||||
t.Setenv("KANDEV_HOME_DIR", t.TempDir())
|
||||
|
||||
code := runManagedApp(context.Background(), managedAppConfig{
|
||||
@@ -258,7 +265,7 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
if code != 0 {
|
||||
t.Fatalf("runManagedApp() = %d, want 0", code)
|
||||
}
|
||||
want := []string{"new-supervisor", "attach-signals", "start-parent-watch", "launch-backend", "wait-health"}
|
||||
want := []string{"new-supervisor", "attach-signals", "start-parent-watch", "launch-backend", "wait-health", "wait-ready"}
|
||||
if !reflect.DeepEqual(events, want) {
|
||||
t.Fatalf("events = %v, want %v", events, want)
|
||||
}
|
||||
@@ -273,16 +280,85 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunManagedAppShutsDownOnReadinessFailure is the regression test for
|
||||
// R2-1: a launcher that only waited on /health would report "backend ready"
|
||||
// and open a browser onto the bootstrap handler's 503 the moment the socket
|
||||
// bound, before startup recovery finished. waitForReadyFn must run after
|
||||
// waitForHealthFn succeeds, and its failure must abort before either the
|
||||
// ready print or a browser open, exactly like a health failure does.
|
||||
func TestRunManagedAppShutsDownOnReadinessFailure(t *testing.T) {
|
||||
oldNewSupervisor := newSupervisorFn
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldStartParentWatch := startParentWatchFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
oldStatusOutput := launcherStatusOutput
|
||||
t.Cleanup(func() {
|
||||
newSupervisorFn = oldNewSupervisor
|
||||
launchBackendFn = oldLaunchBackend
|
||||
attachSignalsFn = oldAttachSignals
|
||||
startParentWatchFn = oldStartParentWatch
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
launcherStatusOutput = oldStatusOutput
|
||||
})
|
||||
|
||||
var output strings.Builder
|
||||
launcherStatusOutput = &output
|
||||
|
||||
newSupervisorFn = newSupervisor
|
||||
attachSignalsFn = func(_ *processSupervisor) {}
|
||||
startParentWatchFn = func(_ *processSupervisor) *parentWatchdog {
|
||||
return newParentWatchdog(0, nil, nil)
|
||||
}
|
||||
launchBackendFn = func(_ backendLaunchConfig) (*restartableBackend, func(), error) {
|
||||
return &restartableBackend{exitCh: make(chan int, 1)}, func() {}, nil
|
||||
}
|
||||
waitForHealthFn = func(_ context.Context, _ backendEndpointSet, _ childState, _ time.Duration, _ string, _ func()) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
readyWaited := false
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
readyWaited = true
|
||||
return errors.New("test readiness failure")
|
||||
}
|
||||
t.Setenv("KANDEV_HOME_DIR", t.TempDir())
|
||||
|
||||
code := runManagedApp(context.Background(), managedAppConfig{
|
||||
Header: "test",
|
||||
Mode: "start",
|
||||
Backend: "kandev",
|
||||
BackendCWD: t.TempDir(),
|
||||
Ports: portConfig{
|
||||
BackendPort: 48123,
|
||||
BackendURL: "http://localhost:48123",
|
||||
},
|
||||
Opts: Options{Headless: true},
|
||||
})
|
||||
if code != 1 {
|
||||
t.Fatalf("runManagedApp() = %d, want 1", code)
|
||||
}
|
||||
if !readyWaited {
|
||||
t.Fatal("waitForReadyFn was never called")
|
||||
}
|
||||
if !strings.Contains(output.String(), "graceful shutdown started") {
|
||||
t.Fatalf("supervisor shutdown not triggered on readiness failure:\n%s", output.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunManagedAppPreservesDesktopOwnedHealthToken(t *testing.T) {
|
||||
oldNewSupervisor := newSupervisorFn
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
oldWaitForReady := waitForReadyFn
|
||||
t.Cleanup(func() {
|
||||
newSupervisorFn = oldNewSupervisor
|
||||
launchBackendFn = oldLaunchBackend
|
||||
attachSignalsFn = oldAttachSignals
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
waitForReadyFn = oldWaitForReady
|
||||
})
|
||||
|
||||
const desktopToken = "desktop-owned-token"
|
||||
@@ -300,6 +376,9 @@ func TestRunManagedAppPreservesDesktopOwnedHealthToken(t *testing.T) {
|
||||
waitedHealthToken = expectedToken
|
||||
return "", nil
|
||||
}
|
||||
waitForReadyFn = func(_ context.Context, _ string, _ childState) error {
|
||||
return nil
|
||||
}
|
||||
t.Setenv("KANDEV_HOME_DIR", t.TempDir())
|
||||
t.Setenv("KANDEV_DESKTOP_NATIVE_NOTIFICATIONS", "true")
|
||||
t.Setenv("KANDEV_DESKTOP_HEALTH_TOKEN", desktopToken)
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"go.uber.org/zap"
|
||||
@@ -787,6 +788,14 @@ func (s *Service) restoreTaskLifecycleToken(ctx context.Context, taskID, key, ev
|
||||
}
|
||||
}
|
||||
|
||||
// lifecycleSweepStuckWarningInterval controls how long
|
||||
// reconcileTaskLifecycleTokens waits before logging a "still recovering"
|
||||
// warning while the sweep is still in flight. It is a var so tests can
|
||||
// shorten it. This is a log-only signal: it never cancels the sweep, because
|
||||
// the resume calls it drives run under context.WithoutCancel
|
||||
// (task_operations.go), so cancellation would not work anyway.
|
||||
var lifecycleSweepStuckWarningInterval = 60 * time.Second
|
||||
|
||||
// reconcileTaskLifecycleTokens scans durable lifecycle markers at startup.
|
||||
// A small fixed worker pool and bounded per-task attempts prevent a corrupted
|
||||
// or repeatedly failing row from creating an unbounded goroutine storm.
|
||||
@@ -839,6 +848,19 @@ func (s *Service) reconcileTaskLifecycleTokens(ctx context.Context) {
|
||||
if len(jobs) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
start := time.Now()
|
||||
taskCount := len(jobs)
|
||||
s.logger.Info("startup lifecycle sweep starting", zap.Int("task_count", taskCount))
|
||||
|
||||
var processed atomic.Int64
|
||||
sweepDone := make(chan struct{})
|
||||
warnDone := make(chan struct{})
|
||||
go func() {
|
||||
defer close(warnDone)
|
||||
s.warnIfLifecycleSweepStuck(taskCount, &processed, start, sweepDone)
|
||||
}()
|
||||
|
||||
jobIDs := make(chan string)
|
||||
workerCount := 4
|
||||
if len(jobs) < workerCount {
|
||||
@@ -851,6 +873,7 @@ func (s *Service) reconcileTaskLifecycleTokens(ctx context.Context) {
|
||||
defer wg.Done()
|
||||
for taskID := range jobIDs {
|
||||
s.recoverTaskLifecycleToken(ctx, taskID)
|
||||
processed.Add(1)
|
||||
}
|
||||
}()
|
||||
}
|
||||
@@ -859,6 +882,30 @@ func (s *Service) reconcileTaskLifecycleTokens(ctx context.Context) {
|
||||
}
|
||||
close(jobIDs)
|
||||
wg.Wait()
|
||||
close(sweepDone)
|
||||
<-warnDone
|
||||
|
||||
s.logger.Info("startup lifecycle sweep finished",
|
||||
zap.Int("task_count", taskCount),
|
||||
zap.Duration("elapsed", time.Since(start)))
|
||||
}
|
||||
|
||||
// warnIfLifecycleSweepStuck logs a warning if the startup lifecycle sweep is
|
||||
// still running after lifecycleSweepStuckWarningInterval, so a slow sweep is
|
||||
// diagnosable from logs instead of requiring a goroutine dump. It is
|
||||
// log-only: it never cancels the sweep, and it always exits (either the
|
||||
// timer fires once, or done closes first).
|
||||
func (s *Service) warnIfLifecycleSweepStuck(taskCount int, processed *atomic.Int64, start time.Time, done <-chan struct{}) {
|
||||
timer := time.NewTimer(lifecycleSweepStuckWarningInterval)
|
||||
defer timer.Stop()
|
||||
select {
|
||||
case <-done:
|
||||
case <-timer.C:
|
||||
s.logger.Warn("startup lifecycle sweep still running",
|
||||
zap.Int("task_count", taskCount),
|
||||
zap.Int64("processed", processed.Load()),
|
||||
zap.Duration("elapsed", time.Since(start)))
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) recoverTaskLifecycleToken(ctx context.Context, taskID string) {
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
package orchestrator
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"go.uber.org/zap"
|
||||
"go.uber.org/zap/zapcore"
|
||||
"go.uber.org/zap/zaptest/observer"
|
||||
|
||||
commonlogger "github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
// blockingLifecycleRepo blocks GetTask until release is closed, letting a
|
||||
// test hold reconcileTaskLifecycleTokens open long enough to observe the
|
||||
// stuck-sweep warning.
|
||||
type blockingLifecycleRepo struct {
|
||||
sessionExecutorStore
|
||||
release <-chan struct{}
|
||||
}
|
||||
|
||||
func (r *blockingLifecycleRepo) GetTask(ctx context.Context, id string) (*models.Task, error) {
|
||||
<-r.release
|
||||
return r.sessionExecutorStore.GetTask(ctx, id)
|
||||
}
|
||||
|
||||
func (r *blockingLifecycleRepo) ListTasksWithMetadataKey(ctx context.Context, key string) ([]*models.Task, error) {
|
||||
return r.sessionExecutorStore.(interface {
|
||||
ListTasksWithMetadataKey(context.Context, string) ([]*models.Task, error)
|
||||
}).ListTasksWithMetadataKey(ctx, key)
|
||||
}
|
||||
|
||||
func observedTestLogger(t *testing.T) (*commonlogger.Logger, *observer.ObservedLogs) {
|
||||
t.Helper()
|
||||
core, logs := observer.New(zapcore.DebugLevel)
|
||||
log, err := commonlogger.NewFromZap(zap.New(core))
|
||||
if err != nil {
|
||||
t.Fatalf("create observed logger: %v", err)
|
||||
}
|
||||
return log, logs
|
||||
}
|
||||
|
||||
// observedTestLoggerWatching is observedTestLogger plus a channel that closes
|
||||
// the first time a log entry with the given message is emitted, so a test can
|
||||
// synchronize on the log itself instead of polling with time.Sleep.
|
||||
func observedTestLoggerWatching(t *testing.T, message string) (*commonlogger.Logger, *observer.ObservedLogs, <-chan struct{}) {
|
||||
t.Helper()
|
||||
core, logs := observer.New(zapcore.DebugLevel)
|
||||
|
||||
seen := make(chan struct{})
|
||||
var once sync.Once
|
||||
hooked := zapcore.RegisterHooks(core, func(entry zapcore.Entry) error {
|
||||
if entry.Message == message {
|
||||
once.Do(func() { close(seen) })
|
||||
}
|
||||
return nil
|
||||
})
|
||||
|
||||
log, err := commonlogger.NewFromZap(zap.New(hooked))
|
||||
if err != nil {
|
||||
t.Fatalf("create observed logger: %v", err)
|
||||
}
|
||||
return log, logs, seen
|
||||
}
|
||||
|
||||
// TestReconcileTaskLifecycleTokensLogsStartAndFinish is the regression test
|
||||
// for docs/plans/startup-listener-before-recovery/task-03-ordering-guard.md:
|
||||
// a long startup sweep must be diagnosable from logs (task count, elapsed
|
||||
// time) rather than requiring a goroutine dump.
|
||||
//
|
||||
// Expected pre-fix failure: reconcileTaskLifecycleTokens logs neither
|
||||
// message today, so both FilterMessage results are empty.
|
||||
func TestReconcileTaskLifecycleTokensLogsStartAndFinish(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "sweep-log-task", "sweep-log-session", "step1")
|
||||
if err := repo.SetTaskMetadataKey(ctx, "sweep-log-task", models.MetaKeyQueuePromotionPending, true); err != nil {
|
||||
t.Fatalf("seed promotion token: %v", err)
|
||||
}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
log, logs := observedTestLogger(t)
|
||||
svc.logger = log
|
||||
|
||||
svc.reconcileTaskLifecycleTokens(ctx)
|
||||
|
||||
starts := logs.FilterMessage("startup lifecycle sweep starting").All()
|
||||
if len(starts) != 1 {
|
||||
t.Fatalf("start logs = %#v, want exactly one", starts)
|
||||
}
|
||||
if got := fmt.Sprint(starts[0].ContextMap()["task_count"]); got != "1" {
|
||||
t.Fatalf("start log task_count = %v, want 1", starts[0].ContextMap()["task_count"])
|
||||
}
|
||||
|
||||
finishes := logs.FilterMessage("startup lifecycle sweep finished").All()
|
||||
if len(finishes) != 1 {
|
||||
t.Fatalf("finish logs = %#v, want exactly one", finishes)
|
||||
}
|
||||
if got := fmt.Sprint(finishes[0].ContextMap()["task_count"]); got != "1" {
|
||||
t.Fatalf("finish log task_count = %v, want 1", finishes[0].ContextMap()["task_count"])
|
||||
}
|
||||
if _, ok := finishes[0].ContextMap()["elapsed"]; !ok {
|
||||
t.Fatalf("finish log missing elapsed field: %#v", finishes[0].ContextMap())
|
||||
}
|
||||
}
|
||||
|
||||
// TestReconcileTaskLifecycleTokensWarnsWhenStuck is the regression test for
|
||||
// the "still recovering N tasks" log-only signal: a sweep still running
|
||||
// after lifecycleSweepStuckWarningInterval must warn instead of staying
|
||||
// silent, and must never cancel the in-flight recovery (context.WithoutCancel
|
||||
// in task_operations.go means cancellation would not work anyway).
|
||||
//
|
||||
// Expected pre-fix failure: lifecycleSweepStuckWarningInterval does not
|
||||
// exist yet, so this fails to compile.
|
||||
func TestReconcileTaskLifecycleTokensWarnsWhenStuck(t *testing.T) {
|
||||
prevInterval := lifecycleSweepStuckWarningInterval
|
||||
lifecycleSweepStuckWarningInterval = 20 * time.Millisecond
|
||||
t.Cleanup(func() { lifecycleSweepStuckWarningInterval = prevInterval })
|
||||
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "stuck-task", "stuck-session", "step1")
|
||||
if err := repo.SetTaskMetadataKey(ctx, "stuck-task", models.MetaKeyQueuePromotionPending, true); err != nil {
|
||||
t.Fatalf("seed promotion token: %v", err)
|
||||
}
|
||||
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
|
||||
log, _, warned := observedTestLoggerWatching(t, "startup lifecycle sweep still running")
|
||||
svc.logger = log
|
||||
|
||||
release := make(chan struct{})
|
||||
svc.repo = &blockingLifecycleRepo{sessionExecutorStore: repo, release: release}
|
||||
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
svc.reconcileTaskLifecycleTokens(ctx)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-warned:
|
||||
case <-time.After(2 * time.Second):
|
||||
close(release)
|
||||
t.Fatal("stuck-sweep warning never logged")
|
||||
}
|
||||
|
||||
close(release)
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("sweep did not finish after release")
|
||||
}
|
||||
}
|
||||
@@ -29,7 +29,7 @@ Use `scripts/release/prepare-desktop-runtime.sh` and `scripts/release/verify-des
|
||||
|
||||
## Architecture
|
||||
|
||||
- Frontend code in `src/` is only the startup/error surface. The real product UI is still served by the Go backend after `/health` succeeds.
|
||||
- Frontend code in `src/` is only the startup/error surface. The real product UI is still served by the Go backend after `/ready` succeeds.
|
||||
- Rust code owns backend process spawning and cleanup. Do not expose broad shell or filesystem permissions to frontend JavaScript.
|
||||
- Native menus emit versioned `kandev-desktop-v1-*` events for SPA-owned context and navigation.
|
||||
Updater, notification, and external-link operations use narrow generated Tauri commands scoped
|
||||
|
||||
@@ -11,11 +11,17 @@ const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const desktopRoot = resolve(__dirname, "..");
|
||||
const repoRoot = resolve(desktopRoot, "../..");
|
||||
|
||||
// Keep above HEALTH_TIMEOUT in apps/desktop/src-tauri/src/backend.rs (60s): the Rust side
|
||||
// retries GET /health every 250ms until that deadline, so this must give it enough room to
|
||||
// finish, or a launcher Rust would still consider "starting" gets killed here first.
|
||||
// desktop-launch-smoke.test.mjs asserts the relationship so the two stay in sync.
|
||||
// The Rust side (apps/desktop/src-tauri/src/backend.rs) does a two-stage wait before it
|
||||
// navigates the webview: wait_for_backend polls GET /health every 250ms against a bounded
|
||||
// HEALTH_TIMEOUT (60s), then wait_for_ready polls GET /ready every 250ms with NO timeout —
|
||||
// it only gives up on child exit or shutdown. HEALTH_REQUESTED_TIMEOUT_MS must stay above
|
||||
// HEALTH_TIMEOUT so the fake runtime doesn't get killed here before the Rust side even
|
||||
// finishes the first stage; desktop-launch-smoke.test.mjs asserts that relationship so the
|
||||
// two stay in sync. READY_REQUESTED_TIMEOUT_MS only bounds this test — the fake runtime
|
||||
// answers /ready immediately once it's listening, so the real wait_for_ready being unbounded
|
||||
// doesn't matter here.
|
||||
export const HEALTH_REQUESTED_TIMEOUT_MS = 90_000;
|
||||
export const READY_REQUESTED_TIMEOUT_MS = 60_000;
|
||||
export const ROOT_REQUESTED_TIMEOUT_MS = 60_000;
|
||||
|
||||
// Only run the CLI behavior when this file is executed directly (`node desktop-launch-smoke.mjs`
|
||||
@@ -81,6 +87,12 @@ async function runSmoke() {
|
||||
failIfExited,
|
||||
describeChild,
|
||||
);
|
||||
await waitForFile(
|
||||
join(stateDir, "ready-requested"),
|
||||
READY_REQUESTED_TIMEOUT_MS,
|
||||
failIfExited,
|
||||
describeChild,
|
||||
);
|
||||
await waitForFile(
|
||||
join(stateDir, "root-requested"),
|
||||
ROOT_REQUESTED_TIMEOUT_MS,
|
||||
@@ -91,7 +103,9 @@ async function runSmoke() {
|
||||
await stopProcess(child);
|
||||
}
|
||||
|
||||
console.log("Desktop smoke passed: WebView requested / after backend health succeeded.");
|
||||
console.log(
|
||||
"Desktop smoke passed: WebView requested / after backend health and readiness succeeded.",
|
||||
);
|
||||
}
|
||||
|
||||
async function writeFakeRuntime(runtimeDir, stateDir) {
|
||||
@@ -153,6 +167,13 @@ async function runFakeRuntime(stateDir, args) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (req.url === "/ready") {
|
||||
await writeFile(join(stateDir, "ready-requested"), "1");
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end('{"status":"ok"}');
|
||||
return;
|
||||
}
|
||||
|
||||
if (req.url === "/") {
|
||||
await writeFile(join(stateDir, "root-requested"), "1");
|
||||
res.writeHead(200, { "content-type": "text/html" });
|
||||
|
||||
@@ -287,6 +287,7 @@ fn launch_and_wait(app: &AppHandle, state: &BackendState) -> Result<String, Stri
|
||||
return Err("Desktop startup cancelled".to_string());
|
||||
}
|
||||
wait_for_backend(port, state, HEALTH_TIMEOUT, &health_token)?;
|
||||
wait_for_ready(port, state)?;
|
||||
Ok(format!("http://{LOOPBACK_HOST}:{port}/"))
|
||||
}
|
||||
|
||||
@@ -530,6 +531,42 @@ fn wait_for_backend(
|
||||
}
|
||||
}
|
||||
|
||||
// wait_for_ready polls GET /ready after wait_for_backend's /health check
|
||||
// already passed. /health flips to 200 as soon as the listener binds, before
|
||||
// startup recovery finishes, so treating a healthy response as "safe to
|
||||
// navigate the webview" reopens the crash-loop-adjacent bug this backend
|
||||
// change fixed: the window would load the bootstrap handler's raw 503 body.
|
||||
// /ready is the readiness signal instead. This wait has no timeout and never
|
||||
// aborts the launch on its own: HEALTH_TIMEOUT above already made the
|
||||
// keep-or-kill decision, and readiness can legitimately take longer while
|
||||
// startup recovery sweeps run. It only returns early if the backend process
|
||||
// exits or shutdown starts. Mirrors wait_for_backend's settle-then-recheck
|
||||
// after a successful probe: a bare "GET /ready succeeded" is not proof the
|
||||
// backend that answered is still the one we launched, since another process
|
||||
// could rebind the same loopback port in the gap between exit and probe.
|
||||
fn wait_for_ready(port: u16, state: &BackendState) -> Result<(), String> {
|
||||
loop {
|
||||
if state.is_shutdown_started() {
|
||||
return Err("Desktop startup cancelled".to_string());
|
||||
}
|
||||
if let Some(message) = state.child_exit_message()? {
|
||||
return Err(format!(
|
||||
"Kandev launcher exited before /ready reported ready ({message})"
|
||||
));
|
||||
}
|
||||
if ready_ready(port) {
|
||||
thread::sleep(HEALTH_READY_SETTLE);
|
||||
if let Some(message) = state.child_exit_message()? {
|
||||
return Err(format!(
|
||||
"Kandev launcher exited before /ready reported ready ({message})"
|
||||
));
|
||||
}
|
||||
return Ok(());
|
||||
}
|
||||
thread::sleep(Duration::from_millis(250));
|
||||
}
|
||||
}
|
||||
|
||||
fn launcher_exit_message(status: &str, output: Option<String>) -> String {
|
||||
let mut message = status.to_string();
|
||||
append_recent_output(&mut message, output);
|
||||
@@ -577,6 +614,37 @@ fn request_health(port: u16, expected_health_token: &str) -> Result<bool, String
|
||||
))
|
||||
}
|
||||
|
||||
fn ready_ready(port: u16) -> bool {
|
||||
request_ready(port).unwrap_or(false)
|
||||
}
|
||||
|
||||
// request_ready hits /ready rather than /health. /ready never sets
|
||||
// DESKTOP_HEALTH_TOKEN_HEADER (that header is /health-only — see
|
||||
// docs/specs/health-endpoint-version/spec.md AC-21), so unlike
|
||||
// request_health this does not check for one.
|
||||
fn request_ready(port: u16) -> Result<bool, String> {
|
||||
let addr = (LOOPBACK_HOST, port)
|
||||
.to_socket_addrs()
|
||||
.map_err(|err| err.to_string())?
|
||||
.next()
|
||||
.ok_or_else(|| format!("Could not resolve {LOOPBACK_HOST}:{port}"))?;
|
||||
let mut stream = TcpStream::connect_timeout(&addr, Duration::from_millis(250))
|
||||
.map_err(|err| err.to_string())?;
|
||||
stream
|
||||
.set_read_timeout(Some(Duration::from_millis(250)))
|
||||
.map_err(|err| err.to_string())?;
|
||||
stream
|
||||
.write_all(
|
||||
format!(
|
||||
"GET /ready HTTP/1.1\r\nHost: {LOOPBACK_HOST}:{port}\r\nConnection: close\r\n\r\n"
|
||||
)
|
||||
.as_bytes(),
|
||||
)
|
||||
.map_err(|err| err.to_string())?;
|
||||
let response = read_http_response_head(&mut stream)?;
|
||||
Ok(response.starts_with(b"HTTP/1.1 200") || response.starts_with(b"HTTP/1.0 200"))
|
||||
}
|
||||
|
||||
fn read_http_response_head<R: Read>(reader: &mut R) -> Result<Vec<u8>, String> {
|
||||
const MAX_HEADER_BYTES: usize = 8 * 1024;
|
||||
let mut response = Vec::with_capacity(512);
|
||||
@@ -1122,6 +1190,47 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_ready_accepts_response_without_health_token_header() {
|
||||
// readyHandler never sets X-Kandev-Desktop-Health-Token (that header
|
||||
// is /health-only); request_ready must accept a bare 2xx.
|
||||
let listener = TcpListener::bind((LOOPBACK_HOST, 0)).expect("bind ready listener");
|
||||
let port = listener.local_addr().expect("listener addr").port();
|
||||
thread::spawn(move || {
|
||||
if let Ok((mut stream, _)) = listener.accept() {
|
||||
let mut buf = [0_u8; 512];
|
||||
let _ = stream.read(&mut buf);
|
||||
let _ = stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n");
|
||||
}
|
||||
});
|
||||
|
||||
let ready = request_ready(port).expect("request_ready");
|
||||
assert!(
|
||||
ready,
|
||||
"request_ready must treat a bare 2xx as ready with no token check"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_ready_rejects_non_2xx_response() {
|
||||
let listener = TcpListener::bind((LOOPBACK_HOST, 0)).expect("bind ready listener");
|
||||
let port = listener.local_addr().expect("listener addr").port();
|
||||
thread::spawn(move || {
|
||||
if let Ok((mut stream, _)) = listener.accept() {
|
||||
let mut buf = [0_u8; 512];
|
||||
let _ = stream.read(&mut buf);
|
||||
let _ = stream
|
||||
.write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\n\r\n");
|
||||
}
|
||||
});
|
||||
|
||||
let ready = request_ready(port).expect("request_ready");
|
||||
assert!(
|
||||
!ready,
|
||||
"request_ready must reject a non-2xx response (bootstrap-not-ready-yet)"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn stop_terminates_tracked_child() {
|
||||
@@ -1145,6 +1254,48 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn wait_for_ready_rechecks_child_exit_after_success() {
|
||||
// The child exits well before the /ready response arrives, so a
|
||||
// wait_for_ready that trusted a bare "GET /ready succeeded" without
|
||||
// rechecking would return Ok for a backend that is already gone.
|
||||
let listener = TcpListener::bind((LOOPBACK_HOST, 0)).expect("bind ready listener");
|
||||
let port = listener.local_addr().expect("listener addr").port();
|
||||
thread::spawn(move || {
|
||||
if let Ok((mut stream, _)) = listener.accept() {
|
||||
let mut buf = [0_u8; 512];
|
||||
let _ = stream.read(&mut buf);
|
||||
thread::sleep(Duration::from_millis(300));
|
||||
let _ = stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n");
|
||||
}
|
||||
});
|
||||
|
||||
let state = BackendState::default();
|
||||
let child = Command::new("sh")
|
||||
.arg("-c")
|
||||
.arg("sleep 0.1")
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.expect("spawn short-lived child");
|
||||
assert!(state.set_child(child));
|
||||
|
||||
let result = wait_for_ready(port, &state);
|
||||
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"wait_for_ready must not return Ok once the child has exited"
|
||||
);
|
||||
assert!(
|
||||
result
|
||||
.unwrap_err()
|
||||
.contains("exited before /ready reported ready"),
|
||||
"wait_for_ready error should name the exit-before-ready reason"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn set_child_terminates_child_when_shutdown_already_started() {
|
||||
|
||||
@@ -475,7 +475,10 @@ export const backendFixture = base.extend<object, { backend: BackendContext }>({
|
||||
// --- Spawn backend ---
|
||||
backendProc = spawnBackendProcess(scopedEnv.apply(baselineEnv), debug, backendPort);
|
||||
registerProcess(backendProc);
|
||||
await waitForHealth(`${baseUrl}/health`, HEALTH_TIMEOUT_MS, backendProc);
|
||||
// /ready (not /health) — /health flips green as soon as the listener
|
||||
// is bound, before routes are wired; tests that immediately issue API
|
||||
// requests need the readiness contract instead.
|
||||
await waitForHealth(`${baseUrl}/ready`, HEALTH_TIMEOUT_MS, backendProc);
|
||||
const frontendUrl = baseUrl;
|
||||
|
||||
/**
|
||||
@@ -499,14 +502,15 @@ export const backendFixture = base.extend<object, { backend: BackendContext }>({
|
||||
await waitForPortFree(backendPort);
|
||||
backendProc = spawnBackendProcess(nextEnv, debug, backendPort);
|
||||
registerProcess(backendProc);
|
||||
// Pass the process so waitForHealth fails fast if it exits (e.g. port still in use)
|
||||
await waitForHealth(`${baseUrl}/health`, HEALTH_TIMEOUT_MS, backendProc);
|
||||
// Pass the process so waitForHealth fails fast if it exits (e.g. port still in use).
|
||||
// /ready, not /health — see the comment on the initial spawn above.
|
||||
await waitForHealth(`${baseUrl}/ready`, HEALTH_TIMEOUT_MS, backendProc);
|
||||
};
|
||||
|
||||
let recovery: Promise<void> | null = null;
|
||||
const ensureReady = async () => {
|
||||
try {
|
||||
await waitForHealth(`${baseUrl}/health`, 5_000);
|
||||
await waitForHealth(`${baseUrl}/ready`, 5_000);
|
||||
return;
|
||||
} catch {
|
||||
// A worker can outlive a backend process that a prior test left
|
||||
|
||||
+7
-3
@@ -281,17 +281,21 @@ The volume at `/data` carries over the database, worktrees, npm globals, and `$H
|
||||
|
||||
## Health Check
|
||||
|
||||
The backend exposes a `/health` endpoint:
|
||||
The backend exposes a `/health` endpoint (liveness: the process is up and its listener is
|
||||
bound, even mid-startup) and a `/ready` endpoint (readiness: startup finished and it can
|
||||
serve real traffic):
|
||||
|
||||
```bash
|
||||
curl http://localhost:38429/health
|
||||
curl http://localhost:38429/ready
|
||||
```
|
||||
|
||||
For Docker health checks in compose:
|
||||
Compose's single `healthcheck:` concept maps to "can serve real traffic," so it should
|
||||
point at `/ready`, not `/health`:
|
||||
|
||||
```yaml
|
||||
healthcheck:
|
||||
test: ["CMD", "curl", "-f", "http://localhost:38429/health"]
|
||||
test: ["CMD", "curl", "-f", "http://localhost:38429/ready"]
|
||||
interval: 30s
|
||||
timeout: 5s
|
||||
retries: 3
|
||||
|
||||
+4
-4
@@ -203,12 +203,12 @@ spec:
|
||||
|
||||
## Health Checks
|
||||
|
||||
The deployment includes both probes on the `/health` endpoint:
|
||||
The deployment's liveness probe calls `/health`; the readiness probe calls `/ready`:
|
||||
|
||||
- **Liveness probe**: Restarts the pod if the backend becomes unresponsive (30s interval, 3 failures)
|
||||
- **Readiness probe**: Removes the pod from service during startup or issues (10s interval, 3 failures)
|
||||
- **Liveness probe**: Restarts the pod if the backend becomes unresponsive (30s interval, 3 failures). `/health` returns 200 as soon as the TCP listener accepts connections, before startup finishes.
|
||||
- **Readiness probe**: Removes the pod from service during startup or issues (10s interval, 3 failures). `/ready` returns 503 until the real router is wired in and the pod can serve real traffic, then 200.
|
||||
|
||||
The CLI launcher also performs an internal health check — it waits for the backend to be healthy before starting the web server.
|
||||
The CLI launcher performs the same two-stage check: it waits for `/health` (the backend process is alive) and then for `/ready` (startup finished, the backend can serve real requests) before starting the web server.
|
||||
|
||||
## Scaling
|
||||
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
---
|
||||
spec: docs/specs/startup-listener-before-recovery/spec.md
|
||||
created: 2026-08-22
|
||||
revised: 2026-08-23
|
||||
status: done
|
||||
---
|
||||
|
||||
# Fix Plan: Bind and Answer Liveness Before Startup Recovery
|
||||
|
||||
## Confirmed root cause
|
||||
|
||||
`backendapp.startGatewayAndServe` starts the orchestrator at `main.go:877` and
|
||||
reaches the first `net.Listen` only at `main.go:1064` -> `httpserver.go:76`.
|
||||
`/health` is a route on the server built after that point (`helpers.go:716`),
|
||||
so no socket exists while `Service.Start` runs.
|
||||
|
||||
`Start` runs `reconcileTaskLifecycleTokens` synchronously (`service.go:2255`)
|
||||
with four workers (`event_handlers_workflow.go:843`) and waits on a
|
||||
`sync.WaitGroup`. Each task can reach `attemptColdResume`, which waits
|
||||
`constants.AgentLaunchTimeout` = **15 minutes** (`timeouts.go:44`) for session
|
||||
readiness (`task_operations.go:2500`), then 30s for prompt readiness, and
|
||||
retries once (`task_operations.go:2235`). Worst case is roughly 31 minutes per
|
||||
task, four at a time, against a 45s health window
|
||||
(`internal/launcher/constants.go`).
|
||||
|
||||
Corroborated empirically: a boot given a full 10-minute window still never
|
||||
bound, producing only 2 ACP session creations in 10 minutes across 32 pending
|
||||
tokens. Workers were parked in the session-ready wait, not cycling.
|
||||
|
||||
## Approach
|
||||
|
||||
**Bind the socket first; keep recovery exactly where it is.**
|
||||
|
||||
An earlier revision of this plan proposed extracting the sweeps and running
|
||||
them after the listener, under a deadline. That approach is rejected:
|
||||
|
||||
- `attemptColdResume` and `reapAndReloadSession` use `context.WithoutCancel`
|
||||
(`task_operations.go:2308`, `task_operations.go:2255`) specifically so a
|
||||
resume survives caller cancellation. An external deadline would not stop an
|
||||
in-flight resume, so the promised bound was fictional.
|
||||
- The sweeps currently run *before* `watcher.Start` and `scheduler.Start`
|
||||
(`service.go:2251` vs `service.go:2262`). Moving them after the listener
|
||||
would race live watcher events, scheduler activity, GitHub polling, and API
|
||||
mutations. The per-task mutex covers only the manual-move lifecycle body; it
|
||||
does not serialise queued WIP reconciliation or dependency launches.
|
||||
|
||||
So the fix changes **when the socket is bound**, not when recovery runs:
|
||||
|
||||
```
|
||||
bind listeners early, serve a bootstrap handler
|
||||
-> liveness endpoint returns 200 from this moment
|
||||
-> every other route returns an explicit "starting" status
|
||||
orchestrator.Start(ctx) unchanged position, unchanged ordering
|
||||
buildHTTPServer(...) unchanged
|
||||
swap the bootstrap handler for the real router
|
||||
mark ready
|
||||
```
|
||||
|
||||
## Correction to a claim in the earlier revision
|
||||
|
||||
`workflowStore.ReconcileQueuedTasks` was listed as a third synchronous
|
||||
launching sweep. That overclaims: it promotes and publishes `task.moved`
|
||||
(`workflow_store.go:472`, `workflow_store.go:650`), but the watcher that
|
||||
handles that event does not start until later in `Start` (`service.go:2262`),
|
||||
so it does not itself synchronously reach the auto-start path. Its promotion is
|
||||
instead picked up by the following lifecycle-token sweep through the durable
|
||||
promotion marker. Two sweeps launch synchronously:
|
||||
`reconcileTaskLifecycleTokens` and `reconcileDependencyLaunchesOnStartup`
|
||||
(`event_handlers_dependencies.go:172`). This does not change the fix, which no
|
||||
longer depends on classifying the sweeps.
|
||||
|
||||
## Implementation waves and parallel candidates
|
||||
|
||||
- [x] [Task 01: Bind listeners and serve liveness before startup work](task-01-bind-before-startup.md) (`done`)
|
||||
- [x] [Task 02: Gate application routes behind an explicit starting state](task-02-gate-routes-while-starting.md) (`done`)
|
||||
- [x] [Task 03: Guard the ordering against regression](task-03-ordering-guard.md) (`done`)
|
||||
|
||||
No task is `parallel-safe`; all three touch the same startup sequence.
|
||||
|
||||
## Validation results
|
||||
|
||||
Implemented and validated across 5 Build/Verify/Review rounds; landed in PR
|
||||
[#2944](https://github.com/kdlbs/kandev/pull/2944)
|
||||
(`feature/backend-never-binds-k15` -> `main`). `/health` is now bound and
|
||||
answering liveness immediately at listen time; `/ready` gates readiness
|
||||
separately per `docs/specs/health-endpoint-version/spec.md`. Full command
|
||||
receipts and round-by-round findings are recorded in the task's Kandev plan.
|
||||
|
||||
## Regression test that must fail first
|
||||
|
||||
`apps/backend/internal/backendapp/binds_before_startup_test.go` (new): run
|
||||
startup with a collaborator that blocks inside `orchestrator.Start`, and assert
|
||||
that the configured port accepts a TCP connection and answers the liveness
|
||||
probe with 200 while `Start` is still blocked.
|
||||
|
||||
Before Task 01 this fails: no socket exists. After Task 01 it passes.
|
||||
|
||||
## Validation commands
|
||||
|
||||
```bash
|
||||
make -C apps/backend test
|
||||
make -C apps/backend lint
|
||||
cd apps/backend && go test ./internal/backendapp/... -run 'Bind|Ready|Start|Health' -count=1
|
||||
```
|
||||
|
||||
## Risks and guardrails
|
||||
|
||||
- **Do not move, reorder, or parallelise the startup sweeps.** Their position
|
||||
before `watcher.Start` is load-bearing. This plan deliberately leaves
|
||||
`Service.Start` untouched.
|
||||
- **Do not rely on cancelling recovery.** `context.WithoutCancel` makes that
|
||||
ineffective; anything that claims to bound it must change the resume contract
|
||||
first, which is out of scope.
|
||||
- **Shutdown ordering is not what it looks like.** `awaitShutdown` stops the
|
||||
orchestrator and lifecycle manager first (`helpers.go:1740`), then runs
|
||||
cleanups (`helpers.go:1755`). Do not register the bootstrap listener's
|
||||
teardown as a cleanup that must run before those stops; hand the early
|
||||
listeners to the same shutdown path that owns the real ones so a single
|
||||
socket is never closed twice or leaked.
|
||||
- The liveness endpoint must not become a readiness endpoint. If it starts
|
||||
reporting startup progress in its status code, the launcher will kill the
|
||||
process again and this fix is undone.
|
||||
- Binding before the router exists means an early request can arrive before any
|
||||
route is registered. The bootstrap handler must answer every path
|
||||
deterministically, never fall through to a nil handler.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- The time a large board spends in "starting". Bulk recovery inheriting the
|
||||
interactive 15-minute `AgentLaunchTimeout` per task is a real defect, but it
|
||||
needs its own spec and a change to the `context.WithoutCancel` resume
|
||||
contract. Recorded in the spec's follow-up section.
|
||||
- WIP promotion, feeder pull, and manual-move lifecycle semantics.
|
||||
- Launcher health-window configuration.
|
||||
@@ -0,0 +1,83 @@
|
||||
---
|
||||
id: "01-bind-before-startup"
|
||||
title: "Bind listeners and serve liveness before startup work"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/startup-listener-before-recovery/spec.md"
|
||||
parallelism: sequential
|
||||
---
|
||||
|
||||
# Task 01: Bind Listeners and Serve Liveness Before Startup Work
|
||||
|
||||
## Intent
|
||||
|
||||
Move socket binding ahead of orchestrator startup so the process can answer a
|
||||
liveness probe while startup work is still running. `Service.Start` and the
|
||||
startup sweeps are not touched.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Listeners are bound before `orchestratorSvc.Start(ctx)` is invoked
|
||||
(`main.go:877`), using the same host and port resolution as today
|
||||
(`serverListenAddr`, `startHTTPServers`).
|
||||
- While startup work runs, the bound socket is served by a bootstrap handler
|
||||
that answers the launcher's liveness path with 200. The launcher probes
|
||||
`/health` (`internal/launcher/health.go`); confirm the exact path and method
|
||||
before choosing what the bootstrap handler answers.
|
||||
- The bootstrap handler answers **every** path deterministically. No path may
|
||||
fall through to a nil handler or hang.
|
||||
- Once `buildHTTPServer` returns, the real router replaces the bootstrap
|
||||
handler on the already-bound listeners, with no window in which the socket is
|
||||
closed and reopened and no second bind of the same port.
|
||||
- The existing readiness flag (`ready`, consulted by the `/health` handler at
|
||||
`main.go:179`) keeps its current meaning for the real router. Task 02 owns
|
||||
making "starting" visible; this task must not make liveness depend on
|
||||
startup progress.
|
||||
- Shutdown closes each listener exactly once. Follow the existing ownership in
|
||||
`awaitShutdown` (`helpers.go:1740`, `helpers.go:1755`) rather than adding a
|
||||
cleanup that races the orchestrator stop.
|
||||
- Bind failure still aborts startup with the same behaviour and logging as
|
||||
today.
|
||||
|
||||
## Suggested mechanism
|
||||
|
||||
Serve an `http.Server` whose `Handler` is a small delegate holding an
|
||||
`atomic.Pointer[http.Handler]`. It starts as the bootstrap handler and is
|
||||
swapped to the Gin engine once built. This avoids closing and rebinding the
|
||||
socket, and keeps a single shutdown owner.
|
||||
|
||||
## Regression test (write first, must fail)
|
||||
|
||||
`apps/backend/internal/backendapp/binds_before_startup_test.go`
|
||||
|
||||
- Drive startup with a substituted orchestrator start function that blocks on a
|
||||
channel the test controls, following the existing
|
||||
`startOrchestratorAndAutomationConsumers` seam (`main.go:877`) already used
|
||||
by `startup_order_test.go`.
|
||||
- Assert a TCP connection to the configured port succeeds and the liveness path
|
||||
returns 200 while the start function is still blocked.
|
||||
- Release the block and assert the real router then serves a normal API route
|
||||
on the same port.
|
||||
|
||||
Expected pre-fix failure: the connection is refused; nothing is bound.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/backendapp/main.go`
|
||||
- `apps/backend/internal/backendapp/httpserver.go`
|
||||
- `apps/backend/internal/backendapp/binds_before_startup_test.go` (new)
|
||||
|
||||
## Validation
|
||||
|
||||
```bash
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp/... -run 'Bind|Start|Health|Ready' -count=1
|
||||
make -C . lint
|
||||
```
|
||||
|
||||
## Notes
|
||||
|
||||
Do not modify `apps/backend/internal/orchestrator/service.go` in this task. The
|
||||
sweeps stay where they are; only the bind moves.
|
||||
@@ -0,0 +1,67 @@
|
||||
---
|
||||
id: "02-gate-routes-while-starting"
|
||||
title: "Gate application routes behind an explicit starting state"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-bind-before-startup"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/startup-listener-before-recovery/spec.md"
|
||||
parallelism: sequential
|
||||
---
|
||||
|
||||
# Task 02: Gate Application Routes Behind an Explicit Starting State
|
||||
|
||||
## Intent
|
||||
|
||||
Task 01 makes the socket answer while the application is still starting. A
|
||||
caller must be able to tell that apart from a started backend, and must get a
|
||||
deterministic answer rather than a hang.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Non-liveness requests arriving before startup completes receive an explicit
|
||||
starting response with a stable status code (503 is the conventional choice)
|
||||
and a machine-readable body, not a hang, a reset, or a 404.
|
||||
- Readiness is exposed distinctly from liveness so an operator can tell "bound,
|
||||
still starting" from "fully started". Prefer extending the existing `/health`
|
||||
payload with a field over adding a new endpoint.
|
||||
- **The liveness status code does not change while starting.** If the path the
|
||||
launcher probes starts returning non-200 during startup, the crash loop
|
||||
returns and Task 01 is undone. Add an explicit test asserting the launcher's
|
||||
probe path returns 200 during the starting state.
|
||||
- The starting state clears exactly when the real router takes over and the
|
||||
existing `ready` flag flips; there is no third state that can persist.
|
||||
- No new user-facing web copy. If any is added later it goes through `t()` per
|
||||
the i18n rules.
|
||||
|
||||
## Regression test (write first, must fail)
|
||||
|
||||
Extend `apps/backend/internal/backendapp/binds_before_startup_test.go` or add
|
||||
`apps/backend/internal/backendapp/starting_state_test.go`:
|
||||
|
||||
- While startup is blocked, assert an API route returns the starting status and
|
||||
a parseable body.
|
||||
- In the same blocked state, assert the launcher's liveness path returns 200.
|
||||
- After startup completes, assert the same API route serves normally and
|
||||
readiness reports ready.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/backendapp/main.go`
|
||||
- `apps/backend/internal/backendapp/helpers.go` (the `/health` payload,
|
||||
around `helpers.go:716`)
|
||||
- the corresponding `*_test.go` files
|
||||
|
||||
## Validation
|
||||
|
||||
```bash
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp/... -count=1
|
||||
make -C . lint
|
||||
```
|
||||
|
||||
## Notes
|
||||
|
||||
Check `docs/specs/health-endpoint-version/spec.md` before changing the
|
||||
`/health` response shape; that spec owns it and may need the same amendment
|
||||
treatment the WIP-limit spec received.
|
||||
@@ -0,0 +1,63 @@
|
||||
---
|
||||
id: "03-ordering-guard"
|
||||
title: "Guard the bind-before-startup ordering against regression"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on: ["01-bind-before-startup", "02-gate-routes-while-starting"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/startup-listener-before-recovery/spec.md"
|
||||
parallelism: sequential
|
||||
---
|
||||
|
||||
# Task 03: Guard the Ordering Against Regression
|
||||
|
||||
## Intent
|
||||
|
||||
The defect is an ordering invariant, and ordering invariants rot silently. The
|
||||
existing `startup_order_test.go` pins orchestrator-before-automation-before-
|
||||
GitHub but says nothing about the bind. Pin the new invariant, and make the
|
||||
cost of a slow startup visible instead of mysterious.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- `apps/backend/internal/backendapp/startup_order_test.go` is extended to
|
||||
assert that binding happens before orchestrator startup, in the same style as
|
||||
the existing ordering assertions.
|
||||
- Startup logs make a long start diagnosable without a goroutine dump: log when
|
||||
the socket is bound, and when startup reconciliation begins and ends, with
|
||||
elapsed time and the number of tasks the lifecycle sweep will process.
|
||||
- A sweep still running after a generous interval logs at `warn` with the count
|
||||
outstanding, so an operator sees "still recovering N tasks" rather than
|
||||
silence. This is a log-only signal; it must not cancel anything, because
|
||||
`context.WithoutCancel` means cancellation would not work
|
||||
(`task_operations.go:2308`).
|
||||
- The spec's follow-up section is carried into a tracked issue or a new spec
|
||||
stub: bulk startup recovery inheriting the interactive 15-minute
|
||||
`AgentLaunchTimeout` per task (`timeouts.go:44`) is a real defect this plan
|
||||
deliberately does not fix.
|
||||
|
||||
## Regression test (write first, must fail)
|
||||
|
||||
Extend `apps/backend/internal/backendapp/startup_order_test.go` with an
|
||||
assertion that the bind step is recorded before the orchestrator start step.
|
||||
It fails against the pre-Task-01 ordering.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/backendapp/startup_order_test.go`
|
||||
- `apps/backend/internal/backendapp/main.go`
|
||||
- `apps/backend/internal/orchestrator/event_handlers_workflow.go` (sweep
|
||||
start/end logging only; no behaviour change)
|
||||
|
||||
## Validation
|
||||
|
||||
```bash
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp/... ./internal/orchestrator/... -count=1
|
||||
make -C . lint
|
||||
```
|
||||
|
||||
## Notes
|
||||
|
||||
Keep the sweep logging cheap. It runs before the watcher starts, so it cannot
|
||||
publish events; plain structured logs are the right level here.
|
||||
@@ -63,7 +63,7 @@ External MCP clients (Claude Code, Cursor connecting to `/mcp`) must be configur
|
||||
|
||||
## Endpoints that stay public
|
||||
|
||||
`/health` (readiness probes), the login/setup/invite pages, `GET /api/v1/features`, and self-authenticating webhook receivers (automation webhooks with `X-Webhook-Secret`, office channel HMAC webhooks). Everything else requires a session or token.
|
||||
`/health` (liveness probes) and `/ready` (readiness probes), the login/setup/invite pages, `GET /api/v1/features`, and self-authenticating webhook receivers (automation webhooks with `X-Webhook-Secret`, office channel HMAC webhooks). Everything else requires a session or token.
|
||||
|
||||
Plugin webhooks (`/api/plugins/{id}/webhooks/{key}`) are **not** public by default: a plugin's manifest must explicitly declare `webhooks[].public: true` for that specific webhook to accept anonymous requests. Unflagged webhooks require a session or PAT like any other endpoint. See [Plugin manifest reference](plugins-manifest.md).
|
||||
|
||||
|
||||
+3
-2
@@ -128,8 +128,9 @@ On a normal start, the launcher:
|
||||
3. selects backend and `agentctl` ports;
|
||||
4. starts and supervises the backend;
|
||||
5. derives one or more `/health` targets from the effective server binds;
|
||||
6. waits up to 45 seconds for any target to return the launcher's health token; and
|
||||
7. opens the reachable access URL in the default browser.
|
||||
6. waits up to 45 seconds for any target to return the launcher's health token (the backend is alive and its socket is bound);
|
||||
7. waits for `/ready` (the backend has finished startup recovery and can serve real requests); this wait is unbounded by design, since recovery can legitimately take much longer than 45 seconds; and
|
||||
8. opens the reachable access URL in the default browser.
|
||||
|
||||
The launcher remains in the foreground. Press `Ctrl+C` or terminate it to stop the backend and its managed children cleanly. A force-kill can leave worktree processes or containers running; inspect them before deleting data.
|
||||
|
||||
|
||||
@@ -231,7 +231,7 @@ Put `KANDEV_DB_PASSWORD` in a permission-restricted deployment secret, not shell
|
||||
|
||||
## Reverse proxy and network policy
|
||||
|
||||
The backend serves SPA, API, `/ws`, `/mcp`, and `/health` on one origin. Proxy the entire root path and preserve WebSocket upgrades. Caddy does this automatically:
|
||||
The backend serves SPA, API, `/ws`, `/mcp`, `/health`, and `/ready` on one origin. Proxy the entire root path and preserve WebSocket upgrades. Caddy does this automatically:
|
||||
|
||||
```text
|
||||
kandev.example.com {
|
||||
@@ -263,17 +263,17 @@ Remote Docker profiles are not a workaround: that executor runtime is currently
|
||||
|
||||
## Health and observability
|
||||
|
||||
`GET /health` returns 503 during startup and 200 after routes are registered and the listener is accepting connections:
|
||||
`GET /health` returns 200 as soon as the listener is accepting connections, even mid-startup; `GET /ready` returns 503 during startup and 200 after routes are registered:
|
||||
|
||||
```bash
|
||||
curl --fail http://localhost:38429/health
|
||||
curl --fail http://localhost:38429/ready
|
||||
```
|
||||
|
||||
It is a startup/readiness signal, not a deep database, Git, Docker, provider, or agent check. A Compose health check can still use it:
|
||||
Neither is a deep database, Git, Docker, provider, or agent check. A Compose health check should use `/ready`, since Compose's single healthcheck concept (`docker ps` status, `depends_on: condition: service_healthy`) maps to "can serve real traffic," not just "process is alive":
|
||||
|
||||
```yaml
|
||||
healthcheck:
|
||||
test: ["CMD", "curl", "-f", "http://localhost:38429/health"]
|
||||
test: ["CMD", "curl", "-f", "http://localhost:38429/ready"]
|
||||
interval: 30s
|
||||
timeout: 5s
|
||||
retries: 3
|
||||
@@ -306,7 +306,7 @@ Delete `kandev-data` only after verifying that its database, workspaces, and cre
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
- **UI unreachable:** check `docker ps`, published address/port, host firewall, and `docker logs kandev`; then call `/health`.
|
||||
- **UI unreachable:** check `docker ps`, published address/port, host firewall, and `docker logs kandev`; then call `/ready`.
|
||||
- **Permission denied under `/data`:** inspect mount ownership and root-squash behavior. The runtime user is UID 1000 after entrypoint setup.
|
||||
- **CLI login works only as root:** repeat it with `docker exec --user kandev`; repair ownership of the affected files before restarting.
|
||||
- **Image pull fails:** authenticate to GHCR if your network policy requires it and verify the tag/platform with `docker buildx imagetools inspect`.
|
||||
|
||||
+2
-2
@@ -194,7 +194,7 @@ The universal image already runs as `kandev`; use `kubectl exec -it deployment/k
|
||||
|
||||
The example requests 250 millicores and 512 MiB, with limits of 2 CPU and 2 GiB. Those are placeholders, not capacity recommendations. Local/Worktree agents share the pod limit with the control plane and can exceed it during builds. Measure workload memory, CPU, ephemeral storage, PVC growth, and process counts; then set requests/limits accordingly.
|
||||
|
||||
Both example probes call `/health`. That endpoint returns 503 during startup and 200 once routes are wired and the TCP listener accepts connections. It is a readiness signal, not a deep check of database, repository, Docker, provider, or agent health. The supplied liveness probe therefore tests the same shallow condition.
|
||||
The example liveness probe calls `/health`; the example readiness probe calls `/ready`. `/health` returns 200 as soon as the TCP listener accepts connections, before startup finishes: it confirms the process is alive, not that it can serve real traffic, so gating liveness on it never restarts a pod that is merely still starting up. `/ready` returns 503 until routes are wired, the agent registry is seeded, and (in e2e builds) the mock-harness routes are mounted, then 200. Neither is a deep check of database, repository, Docker, provider, or agent health.
|
||||
|
||||
Long migrations or slow storage may need a startup probe to prevent premature liveness restarts:
|
||||
|
||||
@@ -207,7 +207,7 @@ startupProbe:
|
||||
failureThreshold: 60
|
||||
```
|
||||
|
||||
Tune from observed startup time. Keep readiness on `/health`; use separate external monitoring for dependencies and real workflows.
|
||||
Tune from observed startup time. Keep liveness on `/health` and readiness on `/ready`; use separate external monitoring for dependencies and real workflows.
|
||||
|
||||
</details>
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ Kandev does not currently provide a user-login boundary for the web application,
|
||||
## Quick path
|
||||
|
||||
1. Choose one process owner for the database and Kandev home.
|
||||
2. Check `/health` for readiness and **System > Status** for diagnostics.
|
||||
2. Check `/ready` for readiness and **System > Status** for diagnostics.
|
||||
3. Back up the database and `master.key` before upgrades, resets, or recovery work.
|
||||
4. Preserve Git branches and external provider state separately from database backups.
|
||||
|
||||
@@ -58,19 +58,25 @@ if the new backend can provide it and a working task still exists.
|
||||
|
||||
## Health and readiness
|
||||
|
||||
Use the top-level readiness endpoint for supervisors and probes:
|
||||
Use the top-level liveness endpoint for process supervisors:
|
||||
|
||||
```bash
|
||||
curl -fsS http://127.0.0.1:38429/health
|
||||
```
|
||||
|
||||
After startup it returns HTTP 200 with:
|
||||
It returns HTTP 200 as soon as the listener accepts connections, with:
|
||||
|
||||
```json
|
||||
{"status":"ok","service":"kandev","mode":"websocket+http","version":"1.2.3"}
|
||||
```
|
||||
|
||||
It returns HTTP 503 with `status: "starting"` (plus the same `version`) until routes, the agent registry, and the listener are ready. The supplied Kubernetes probes use this endpoint. `/health` is unauthenticated even when auth is enabled, so it's also the credential-free way for monitoring to read the running version; there is no need to authenticate to **System > About** just to check what build is deployed.
|
||||
`/health` never returns a non-2xx status while the process is alive, even mid-startup: it confirms the process is up, not that it can serve real traffic. Use the readiness endpoint instead when you need to know the backend can actually serve requests:
|
||||
|
||||
```bash
|
||||
curl -fsS http://127.0.0.1:38429/ready
|
||||
```
|
||||
|
||||
It returns HTTP 503 with `status: "starting"` (plus the same `version`) until routes, the agent registry, and the listener are ready, then HTTP 200. The supplied Kubernetes liveness probe uses `/health`; the readiness probe uses `/ready`. Both are unauthenticated even when auth is enabled, so they're also a credential-free way for monitoring to read the running version; there is no need to authenticate to **System > About** just to check what build is deployed.
|
||||
|
||||
For application diagnostics, open **Settings > System > Status** or request:
|
||||
|
||||
@@ -313,7 +319,7 @@ The UI flow applies to the configured SQLite path:
|
||||
2. Open **Settings > System > Backups**, choose **Restore**, type `RESTORE`, and confirm.
|
||||
3. Kandev stops scheduling, active executions, and database-backed workers. It copies the selected snapshot to `<configured-database-path>.new`, validates the SQLite checkpoint result, and closes the pool. It then quarantines the configured database and its `-wal`/`-shm` sidecars before installing the staged file. If installation fails, Kandev restores the quarantined files.
|
||||
4. Click **Restart Kandev** in the success dialog. If automatic restart is unavailable, quit and relaunch Kandev manually. The backend must restart before database-backed work resumes.
|
||||
5. Check `/health`, **System > Status**, database schema version, secrets, and representative tasks.
|
||||
5. Check `/ready`, **System > Status**, database schema version, secrets, and representative tasks.
|
||||
|
||||
Restore does not roll back worktrees or remote/provider state. A database may therefore refer to files, containers, pull requests, or credentials from a different point in time. Reconcile them before restarting automation.
|
||||
|
||||
@@ -433,7 +439,7 @@ Before any update, finish or stop active sessions, create and export a database
|
||||
- Transient npx: start the desired release with `npx -y kandev@latest`; this does not update a persistent package.
|
||||
- Docker/Kubernetes: replace the image and recreate the workload. Do not treat an in-container package install as a durable update.
|
||||
|
||||
After restart, verify `/health`, **System > About**, **System > Status**, the database page, and one non-destructive agent session. Kandev does not perform automatic binary or database rollback. If rollback is necessary, restore a compatible pre-upgrade database and matching application release together.
|
||||
After restart, verify `/ready`, **System > About**, **System > Status**, the database page, and one non-destructive agent session. Kandev does not perform automatic binary or database rollback. If rollback is necessary, restore a compatible pre-upgrade database and matching application release together.
|
||||
|
||||
## Resource metrics
|
||||
|
||||
@@ -474,8 +480,9 @@ drawer mirrors it as the saved left sequence followed by the saved right sequenc
|
||||
|
||||
| Symptom | Check | Action |
|
||||
| --- | --- | --- |
|
||||
| `/health` stays at 503 or cannot connect | Process-manager and launcher output | Confirm port ownership, database reachability, writable Kandev home, and required executables; then restart once |
|
||||
| Status page says unhealthy while `/health` is 200 | `/api/v1/system/health` issue IDs | Fix Git, GitHub, agent discovery, or Linux inotify warning; readiness and application diagnostics have different meanings |
|
||||
| `/health` cannot connect | Process-manager and launcher output | Confirm port ownership, database reachability, writable Kandev home, and required executables; then restart once |
|
||||
| `/ready` stays at 503 while `/health` is 200 | Backend startup logs | Process is alive but still wiring routes, seeding the agent registry, or mounting test-harness routes; give it more time before restarting |
|
||||
| Status page says unhealthy while `/ready` is 200 | `/api/v1/system/health` issue IDs | Fix Git, GitHub, agent discovery, or Linux inotify warning; readiness and application diagnostics have different meanings |
|
||||
| Backups page reports a 15-second create timeout | Reload the backup list and inspect the `backup-create` job/log | Large `VACUUM INTO` jobs can still finish; avoid double-clicking and ensure free disk |
|
||||
| Backup/maintenance fails on PostgreSQL | Active driver on Database page | Use `pg_dump`, provider snapshots, and PostgreSQL maintenance; System backup/vacuum/reset is SQLite-only |
|
||||
| Restored data looks stale | Whether the backend was restarted immediately | Quit/restart; do not keep using the old open database connections |
|
||||
|
||||
@@ -246,7 +246,7 @@ Review the warning from the OS-package phase. Install a compiler toolchain, `pkg
|
||||
Use the backend port from the latest startup banner, not the preferred port or the Vite port. Confirm locally inside the VM:
|
||||
|
||||
```bash
|
||||
curl --fail http://127.0.0.1:38429/health
|
||||
curl --fail http://127.0.0.1:38429/ready
|
||||
```
|
||||
|
||||
Substitute the logged port. Then check the provider's port visibility, proxy path/WebSocket support, firewall, and process logs. Do not solve the problem by making an unauthenticated development port public.
|
||||
|
||||
@@ -52,13 +52,13 @@ kandev service status
|
||||
kandev service logs
|
||||
```
|
||||
|
||||
The installer writes the managed unit or plist and starts the process. It does **not** poll `/health`; use `status` and `logs` first. The logs print the actual listener URL because the launcher selects a free fallback port when `38429` is unavailable. Copy that URL and append `/health`; for example, if the log reports port `43127`:
|
||||
The installer writes the managed unit or plist and starts the process. It does **not** poll `/health`; use `status` and `logs` first. The logs print the actual listener URL because the launcher selects a free fallback port when `38429` is unavailable. Copy that URL and append `/ready`; for example, if the log reports port `43127`:
|
||||
|
||||
```bash
|
||||
curl --fail http://127.0.0.1:43127/health
|
||||
curl --fail http://127.0.0.1:43127/ready
|
||||
```
|
||||
|
||||
`/health` reports backend readiness after routes and the agent registry are initialized and the HTTP listener is accepting connections. It is not a deep health check of the database, message bus, executors, or remote providers.
|
||||
`/ready` reports backend readiness after routes and the agent registry are initialized and the HTTP listener is accepting connections. It is not a deep health check of the database, message bus, executors, or remote providers.
|
||||
|
||||
### System service
|
||||
|
||||
|
||||
@@ -35,5 +35,6 @@ The former legacy size exceptions for migrated sources were removed. All canonic
|
||||
The following specification was added after this migration and remains in the legacy layout until it moves into an owning system:
|
||||
|
||||
- [Task Cost & Token Ledger](task-cost-ledger/spec.md) (draft)
|
||||
- [Startup listener before recovery](startup-listener-before-recovery/spec.md) (draft)
|
||||
|
||||
## Authoring rule
|
||||
|
||||
@@ -57,10 +57,13 @@ This increment preserves the shipped desktop contract:
|
||||
Vite/React SPA without requiring Node.js or a separate web server at runtime;
|
||||
- desktop and CLI/browser launches share the existing Kandev data directory, database, worktrees,
|
||||
executor settings, integrations, and agent configuration;
|
||||
- the desktop shell owns the backend process tree, waits for authenticated health readiness, and
|
||||
cleans up on startup failure, quit, update restart, or WebView failure;
|
||||
- the desktop shell owns the backend process tree, waits for a two-stage readiness signal (a
|
||||
token-verified `/health` proving process ownership, then an unauthenticated `/ready` proving
|
||||
application readiness), and cleans up on startup failure, quit, update restart, or WebView
|
||||
failure;
|
||||
- the backend binds to an app-selected loopback port, and the WebView navigates only after a
|
||||
per-launch health token proves that the responding process is the backend the shell started;
|
||||
per-launch health token first proves that the responding process is the backend the shell
|
||||
started, and `/ready` then reports the backend has finished startup;
|
||||
- GUI launches retain the predictable process environment and common user binary locations needed
|
||||
to discover configured agent CLIs;
|
||||
- a second launch focuses the existing instance instead of starting another backend; and
|
||||
@@ -75,13 +78,18 @@ dependency. It selects an available loopback port, starts:
|
||||
kandev --headless --port <available-loopback-port>
|
||||
```
|
||||
|
||||
with `KANDEV_SERVER_HOST=127.0.0.1`, and waits for `GET /health`. Each launch supplies a generated
|
||||
`KANDEV_DESKTOP_HEALTH_TOKEN`; readiness requires the same value in the successful response's
|
||||
`X-Kandev-Desktop-Health-Token` header before the WebView navigates to the backend origin. The
|
||||
desktop shell also sets `KANDEV_DESKTOP_NATIVE_NOTIFICATIONS=true` to identify the launch as
|
||||
desktop-owned. The nested native launcher must forward the shell's exact non-empty health token to
|
||||
the backend and use it for its own readiness poll rather than replacing it with another token. A
|
||||
second instance focuses the first and does not start another backend.
|
||||
with `KANDEV_SERVER_HOST=127.0.0.1`, and waits for readiness in two stages. First it polls
|
||||
`GET /health`, which flips to 200 as soon as the listener binds, before startup recovery
|
||||
finishes; each launch supplies a generated `KANDEV_DESKTOP_HEALTH_TOKEN`, and this stage
|
||||
requires the same value in the successful response's `X-Kandev-Desktop-Health-Token` header,
|
||||
proving the responding process is the backend the shell started. Once `/health` passes, the
|
||||
shell then polls unauthenticated `GET /ready` (which carries no health token), and only
|
||||
navigates the WebView to the backend origin once `/ready` reports the backend has finished
|
||||
startup. The desktop shell also sets `KANDEV_DESKTOP_NATIVE_NOTIFICATIONS=true` to identify the
|
||||
launch as desktop-owned. The nested native launcher must forward the shell's exact non-empty
|
||||
health token to the backend and use it for its own `/health` readiness poll rather than
|
||||
replacing it with another token. A second instance focuses the first and does not start another
|
||||
backend.
|
||||
|
||||
The shell preserves the existing GUI-launch `PATH` normalization and the existing Kandev data
|
||||
directory, SQLite database, worktrees, executor settings, integrations, and agent configuration.
|
||||
@@ -199,8 +207,10 @@ X-Kandev-Desktop-Health-Token: <per-launch-token>
|
||||
```
|
||||
|
||||
Desktop launches set `KANDEV_SERVER_HOST=127.0.0.1`. The health response must echo the per-launch
|
||||
token before the WebView navigates to the loopback SPA. Existing backend HTTP, WebSocket, and boot
|
||||
payload contracts remain the product API; native integration does not replace them.
|
||||
token before the shell trusts the responding process; the WebView then navigates to the loopback
|
||||
SPA only once the subsequent unauthenticated `GET /ready` reports the backend has finished
|
||||
startup. Existing backend HTTP, WebSocket, and boot payload contracts remain the product API;
|
||||
native integration does not replace them.
|
||||
|
||||
## State machines
|
||||
|
||||
|
||||
@@ -155,13 +155,18 @@ Numbered, pass/fail, EARS-form. Each is directly testable against the handler.
|
||||
|
||||
**Starting-path payload**
|
||||
|
||||
5. WHILE the readiness flag is unset, a `GET /health` request SHALL receive HTTP 503.
|
||||
6. WHILE the readiness flag is unset, a `GET /health` request SHALL receive a JSON body
|
||||
5. **SUPERSEDED, see [Amendment](#amendment-2026-08-22--liveness-readiness-split).**
|
||||
~~WHILE the readiness flag is unset, a `GET /health` request SHALL receive HTTP 503.~~
|
||||
`/health` now always answers 200; the not-ready 503 contract moved to `GET /ready`
|
||||
(AC-16).
|
||||
6. **SUPERSEDED, see [Amendment](#amendment-2026-08-22--liveness-readiness-split).**
|
||||
~~WHILE the readiness flag is unset, a `GET /health` request SHALL receive a JSON body
|
||||
containing a `version` key whose value equals the process's configured build version
|
||||
string.
|
||||
7. WHILE the readiness flag is unset, a `GET /health` response body SHALL contain exactly
|
||||
string.~~ Moved to `GET /ready` (AC-17).
|
||||
7. **SUPERSEDED, see [Amendment](#amendment-2026-08-22--liveness-readiness-split).**
|
||||
~~WHILE the readiness flag is unset, a `GET /health` response body SHALL contain exactly
|
||||
the keys `status`, `service`, and `version`, with `status` equal to `"starting"` and
|
||||
`service` equal to `"kandev"`.
|
||||
`service` equal to `"kandev"`.~~ Moved to `GET /ready` (AC-18).
|
||||
|
||||
**Value correctness**
|
||||
|
||||
@@ -179,9 +184,12 @@ Numbered, pass/fail, EARS-form. Each is directly testable against the handler.
|
||||
12. WHEN authentication is enabled AND a `GET /health` request arrives with no credential,
|
||||
the backend SHALL respond with the full body including `version`, and SHALL NOT
|
||||
respond 401 or 403.
|
||||
13. The change SHALL NOT alter the response status code, headers, or the `status` field
|
||||
13. **SUPERSEDED, see [Amendment](#amendment-2026-08-22--liveness-readiness-split).**
|
||||
~~The change SHALL NOT alter the response status code, headers, or the `status` field
|
||||
value for any readiness state, so that an existing prober asserting only HTTP status
|
||||
or `status` continues to pass unmodified.
|
||||
or `status` continues to pass unmodified.~~ `/health`'s status code for the not-ready
|
||||
state changed (503 → 200) by the amendment below; the original no-status-code-change
|
||||
guarantee now applies to `GET /ready` instead (AC-19, AC-21).
|
||||
14. WHEN authentication is enabled AND a `GET /api/v1/system/info` request arrives with no
|
||||
credential, the backend SHALL continue to reject it, confirming this change does not
|
||||
widen the allowlist.
|
||||
@@ -241,7 +249,7 @@ Every in-tree `/health` consumer was checked against an additive body field.
|
||||
| Desktop smoke test | `apps/desktop/e2e/desktop-launch-smoke.mjs` | URL match on a fake runtime | None |
|
||||
| Playwright fixture | `apps/web/e2e/fixtures/backend.ts:461` | Waits for HTTP readiness | None |
|
||||
| Homebrew service | `scripts/release/kandev.rb` | Polls the URL | None |
|
||||
| Docker docs example | `docs/docker.md` | `curl -f`, status only | None |
|
||||
| Docker docs example | `docs/docker.md` | `curl -f`, status only | None at the time; later repointed to `/ready` by the Amendment below |
|
||||
| Public operations doc | `docs/public/operations.md:70` | **Pins the exact JSON body** | **Must be updated — AC-15** |
|
||||
|
||||
No consumer deserializes the body into a fixed struct that would reject an unknown field,
|
||||
@@ -307,11 +315,123 @@ Resolved with the requester on 2026-08-07, before this spec was frozen.
|
||||
(AC-15). It is the only doc that reproduces the payload verbatim.
|
||||
- `docs/docker.md`, `docs/k8s.md`, `docs/run-as-a-service.md`, `docs/public/cli.md`, and
|
||||
`docs/public/authentication.md` reference `/health` but do not reproduce its body, so
|
||||
they need no change.
|
||||
they need no change for this base spec. (Superseded for `docs/docker.md` and
|
||||
`docs/k8s.md` by the Amendment below: the liveness/readiness split gave them a second
|
||||
endpoint to document, which is a structural change independent of the body-shape
|
||||
question this bullet answers. `docs/run-as-a-service.md` still needs no change from
|
||||
either spec; it has its own pre-existing `/health` wording inconsistency that predates
|
||||
both this spec and the Amendment and is tracked separately, not fixed here.)
|
||||
- Consider noting in `docs/public/operations.md` that `/health` is now the
|
||||
credential-free way to read the running version, since that is the operator-visible
|
||||
benefit.
|
||||
|
||||
## Amendment (2026-08-22) — Liveness/Readiness Split
|
||||
|
||||
Made during the `backend-never-binds` DEFECT fix
|
||||
(`docs/specs/startup-listener-before-recovery/spec.md`), by nova28, after Review
|
||||
flagged that fix's bootstrap handler as violating this spec's frozen AC-5/6/7/13.
|
||||
|
||||
**Why.** `startup-listener-before-recovery` binds the HTTP listener and serves a
|
||||
bootstrap handler *before* orchestrator startup recovery sweeps run, so the
|
||||
launcher's liveness probe (`internal/launcher/health.go`) never times out
|
||||
waiting for a socket that startup recovery is blocking. That bootstrap handler
|
||||
answers `GET /health` 200 unconditionally, because the launcher's healthcheck
|
||||
must succeed regardless of startup progress — gating it on readiness is exactly
|
||||
the crash-loop bug that fix exists to close. But this spec's AC-5/6/7/13 require
|
||||
`/health` to answer 503 while the readiness flag is unset, and the real router
|
||||
(`healthHandler`, `internal/backendapp/helpers.go`) enforced that. The two
|
||||
requirements are mutually exclusive on one endpoint: `/health` cannot be both
|
||||
"always 200 so liveness never flaps" and "503 until ready so readiness probers
|
||||
wait." Reordering `ready.Store`/`handler.Store` (the fix's own R1-2 correction)
|
||||
only fixes *when* the swap between the bootstrap and real handler is safe; it
|
||||
does not resolve this contract conflict, because the real handler's `!ready`
|
||||
branch is what conflicts with the bootstrap handler's unconditional 200.
|
||||
|
||||
**Resolution.** Split the concerns onto two endpoints, matching standard
|
||||
liveness/readiness terminology (Kubernetes' own naming for the same
|
||||
distinction):
|
||||
|
||||
- **`GET /health` becomes a pure liveness probe.** It answers 200 as soon as
|
||||
the listener is bound — before, during, and after startup — and never gates
|
||||
on the `ready` flag. This is now true in **both** the bootstrap handler
|
||||
(`internal/backendapp/httpserver.go`) and the real router's `healthHandler`
|
||||
(`internal/backendapp/helpers.go`), so the endpoint's behavior is identical
|
||||
across the handler swap. All of this spec's ready-path ACs (AC-1..4, AC-8..12,
|
||||
AC-14, AC-15) continue to apply to `/health`'s response, since it still always
|
||||
returns the shape those ACs describe (`status: "ok"`, `service`, `mode`,
|
||||
`version`) — the only change is that shape is now served for *every* request,
|
||||
not conditionally.
|
||||
- **`GET /ready` is a new endpoint that inherits `/health`'s original readiness
|
||||
contract.** It is registered only on the real router (`helpers.go`), not the
|
||||
bootstrap handler — while the bootstrap handler is in effect, any non-`/health`
|
||||
path (including `/ready`) already falls through to its existing 503
|
||||
`"starting"` branch (`internal/backendapp/httpserver.go`), so `/ready`
|
||||
correctly reports not-ready during that window with zero bootstrap-handler
|
||||
code change. Once the real router is swapped in, `/ready` gates on the same
|
||||
`ready` flag `/health` used to.
|
||||
|
||||
**New acceptance criteria** (supersede AC-5, AC-6, AC-7, AC-13 for `/health`;
|
||||
apply to `/ready`):
|
||||
|
||||
16. WHILE the readiness flag is unset, a `GET /ready` request SHALL receive
|
||||
HTTP 503.
|
||||
17. WHILE the readiness flag is unset, a `GET /ready` request SHALL receive a
|
||||
JSON body containing a `version` key whose value equals the process's
|
||||
configured build version string.
|
||||
18. WHILE the readiness flag is unset, a `GET /ready` response body SHALL
|
||||
contain exactly the keys `status`, `service`, and `version`, with `status`
|
||||
equal to `"starting"` and `service` equal to `"kandev"`.
|
||||
19. WHEN the readiness flag is set, a `GET /ready` request SHALL receive HTTP
|
||||
200 and a JSON body containing exactly the keys `status`, `service`, and
|
||||
`version`, with `status` equal to `"ok"` and `service` equal to `"kandev"`.
|
||||
`/ready`'s 200 body deliberately omits `mode` — it is not a duplicate of
|
||||
`/health`, it exists solely to answer "can this backend serve real
|
||||
traffic yet."
|
||||
20. A `GET /health` request SHALL receive HTTP 200 regardless of the readiness
|
||||
flag's state, once the listener is accepting connections. This formalizes
|
||||
the bootstrap-handler behavior `startup-listener-before-recovery` depends
|
||||
on and is the reason AC-5/6/7 are superseded rather than merely relocated
|
||||
unchanged.
|
||||
21. The desktop health-token response header (AC in the original "## What"
|
||||
section) SHALL be set only on `GET /health`, never on `GET /ready` — the
|
||||
token contract belongs to the desktop launcher's liveness probe
|
||||
(`internal/launcher/health.go`), which does not consult `/ready`.
|
||||
22. `GET /ready` SHALL be on the unauthenticated allowlist
|
||||
(`internal/auth/httpmw/middleware.go`), identically to `/health`, so
|
||||
Kubernetes' `readinessProbe` and the e2e fixture's readiness wait do not
|
||||
need a credential.
|
||||
|
||||
**Updated Consumer Compatibility Audit** (rows changed from the original
|
||||
table):
|
||||
|
||||
| Consumer | File | How it reads readiness | Impact |
|
||||
|---|---|---|---|
|
||||
| K8s readinessProbe | `k8s/deployment.yaml` | HTTP status only, now against `/ready` | Repointed |
|
||||
| K8s livenessProbe | `k8s/deployment.yaml` | HTTP status only, stays on `/health` | None |
|
||||
| Playwright fixture | `apps/web/e2e/fixtures/backend.ts` | Waits for HTTP readiness, now against `/ready` | Repointed |
|
||||
| Go launcher | `internal/launcher/health.go` | Status code against `/health`; body discarded | None — still liveness, unaffected |
|
||||
| CLI, Desktop shell, Homebrew service | (unchanged files) | Status/socket against `/health` | None — liveness consumers, correctly unaffected |
|
||||
| Docker Compose healthcheck example | `docs/docker.md` | Was `curl -f .../health`, status only | Repointed to `/ready` (Review round 3, R3-1 finding) — Compose's single healthcheck concept means "can serve real traffic," which is `/ready`'s contract, not `/health`'s |
|
||||
| K8s docs prose | `docs/k8s.md` | Described "both probes on `/health`" | Corrected to match `k8s/deployment.yaml`: liveness stays `/health`, readiness is `/ready` (Review round 3, R3-3 finding) |
|
||||
| Playwright fixture docs | `docs/test_e2e_web.md` | Described waiting for `/health` | Corrected to `/ready`, matching `apps/web/e2e/fixtures/backend.ts`'s actual wait (Review round 3, R3-3 finding) |
|
||||
|
||||
**Updated Out of Scope note.** The original spec's "Renaming, moving, or
|
||||
changing the auth posture of any endpoint" bullet refers to `/health` itself,
|
||||
which is unchanged in name, route, or auth posture by this amendment. `/ready`
|
||||
is a new endpoint, not a rename — adding it does not conflict with that bullet.
|
||||
|
||||
**Updated Files Reference** (in addition to the original table):
|
||||
|
||||
| File | Change |
|
||||
|---|---|
|
||||
| `apps/backend/internal/backendapp/helpers.go` | `healthHandler` no longer gates on `ready`; new `readyHandler` gains the original 503/200 gate |
|
||||
| `apps/backend/internal/backendapp/httpserver.go` | No change required — the bootstrap handler's existing "any non-`/health` path" 503 branch already covers `/ready` |
|
||||
| `apps/backend/internal/auth/httpmw/middleware.go` | Added `/ready` to `isPublicPath`, mirroring `/health` |
|
||||
| `k8s/deployment.yaml` | `readinessProbe.httpGet.path` changed to `/ready`; `livenessProbe` stays `/health` |
|
||||
| `apps/web/e2e/fixtures/backend.ts` | Readiness waits repointed to `/ready` |
|
||||
| `docs/public/{operations,k8s,docker,run-as-a-service,authentication,remote-cloud-environment}.md` | Updated to describe the liveness/readiness split |
|
||||
| `docs/docker.md`, `docs/k8s.md`, `docs/test_e2e_web.md` | Updated (Review round 3 / Build round 4, not the initial amendment pass) to repoint their `/health`-only readiness references to `/ready` |
|
||||
|
||||
## Rollback
|
||||
|
||||
Revert the handler change and the doc change. No migration, no persisted state, no config
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
---
|
||||
status: draft
|
||||
created: 2026-08-22
|
||||
revised: 2026-08-22
|
||||
type: repair
|
||||
---
|
||||
|
||||
# The Backend Must Bind and Answer Liveness Before Startup Recovery Runs
|
||||
|
||||
## Problem
|
||||
|
||||
The backend does not bind its HTTP socket until orchestrator startup
|
||||
reconciliation has finished, and that reconciliation can wait on agent
|
||||
processes for hours. Under the launcher's health window the process is killed
|
||||
before it ever listens, restarted, and the cycle repeats without converging.
|
||||
|
||||
`backendapp.startGatewayAndServe` calls `orchestrator.Service.Start` at
|
||||
`main.go:877` and reaches the first `net.Listen` only via `main.go:1064` ->
|
||||
`httpserver.go:76`. `/health` is a route on the server built after that point
|
||||
(`helpers.go:716`); no earlier path serves it. So while `Start` runs there is
|
||||
no listener at all, which is exactly what `lsof` showed.
|
||||
|
||||
`Start` runs `reconcileTaskLifecycleTokens` synchronously
|
||||
(`service.go:2255`), fanning four workers over every task with a pending
|
||||
lifecycle token and waiting on a `sync.WaitGroup`. The per-task work reaches
|
||||
`autoStartStepPrompt` -> `PromptTask` -> `ensureSessionRunning` ->
|
||||
`attemptColdResume`.
|
||||
|
||||
**The per-task cost is far larger than the prompt-ready timeout suggests.**
|
||||
`attemptColdResume` first waits for session readiness via `waitForSessionReady`
|
||||
(`task_operations.go:2498`), whose budget is `constants.AgentLaunchTimeout` =
|
||||
**15 minutes** (`timeouts.go:44`, 10m setup script + 5m allowance). Only after
|
||||
that does the 30s `waitForAgentPromptReady` apply, and the attempt is retried
|
||||
once. Worst case is roughly 31 minutes per task, four at a time.
|
||||
|
||||
Measured on the affected instance: 424 tasks, 32 carrying
|
||||
`manual_move_lifecycle` metadata, on a 1.0 GB database. A boot given a full
|
||||
10-minute health window still never bound, and produced only 2 successful ACP
|
||||
session creations in those 10 minutes. That is the signature of workers parked
|
||||
in the 15-minute session-ready wait, not of workers cycling through tasks.
|
||||
|
||||
Three consequences, in severity order:
|
||||
|
||||
1. **Socket liveness is coupled to agent readiness.** The process cannot answer
|
||||
a health probe, serve the UI, or expose the API until agents that may never
|
||||
become ready are waited out.
|
||||
2. **The failure is self-sustaining.** `healthTimeoutReleaseMS` is 45s
|
||||
(`internal/launcher/constants.go`). The launcher declares a backend health
|
||||
failure and stops the process; a supervisor restarts it; the same tokens are
|
||||
still pending because none were cleared, and the runtimes orphaned by the
|
||||
kill are added to the next boot's work. Ten consecutive boots were observed,
|
||||
none of which bound a socket.
|
||||
3. **The wait cannot be bounded from outside.** `attemptColdResume` and
|
||||
`reapAndReloadSession` deliberately use `context.WithoutCancel`
|
||||
(`task_operations.go:2308`, `task_operations.go:2255`) so a resume survives
|
||||
its caller's cancellation. A deadline handed to the sweep therefore does not
|
||||
stop an in-flight resume. Any fix that relies on cancelling the sweep to
|
||||
protect startup is ineffective.
|
||||
|
||||
This is startup ordering, not a defect in the recovery logic. The same frames
|
||||
are reached whenever a lifecycle token is pending.
|
||||
|
||||
## Broken behavior
|
||||
|
||||
- No socket is bound, and `/health` cannot be answered, until orchestrator
|
||||
startup reconciliation returns.
|
||||
- A backend whose recovery exceeds the health window is killed and restarted
|
||||
with at least as much recovery work as before, indefinitely.
|
||||
|
||||
## Desired behavior
|
||||
|
||||
- The backend binds its socket and answers a liveness probe within the release
|
||||
health window, regardless of how much recovery work is pending.
|
||||
- Startup recovery keeps its current position and ordering: it still runs
|
||||
before the watcher and scheduler start, so it is not exposed to concurrent
|
||||
watcher events, scheduler activity, or API mutations.
|
||||
- Requests that need a started application are answered with an explicit
|
||||
"starting" response rather than by a hung connection or a refused socket.
|
||||
- Readiness is observable and distinct from liveness: an operator can tell
|
||||
"bound, still starting" from "fully started".
|
||||
|
||||
## Regression scenarios
|
||||
|
||||
- **GIVEN** a backend with pending lifecycle tokens whose agent sessions never
|
||||
reach ready, **WHEN** it starts, **THEN** the socket is bound and the
|
||||
liveness probe returns success well inside `healthTimeoutReleaseMS`, and the
|
||||
launcher does not shut the process down.
|
||||
- **GIVEN** the same backend while startup reconciliation is still running,
|
||||
**WHEN** an API route is requested, **THEN** it returns an explicit starting
|
||||
status rather than hanging, and readiness reports not-ready while liveness
|
||||
reports healthy.
|
||||
- **GIVEN** startup reconciliation that has completed, **WHEN** readiness is
|
||||
requested, **THEN** it reports ready and all routes serve normally.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Reordering, parallelising, or moving startup reconciliation relative to the
|
||||
watcher and scheduler. Its current position is load-bearing and moving it
|
||||
introduces races with live traffic.
|
||||
- Changing WIP promotion, feeder pull, or manual-move lifecycle semantics.
|
||||
- Changing `AgentLaunchTimeout`, `agentPromptReadyTimeout`, or the cold-resume
|
||||
retry shape. See the follow-up note below.
|
||||
- Raising `healthTimeoutReleaseMS`, which hides the coupling instead of fixing
|
||||
it. Empirically a 10-minute window did not help.
|
||||
|
||||
## Known follow-up, deliberately separate
|
||||
|
||||
Fixing the ordering stops the crash loop but does not make a large board start
|
||||
quickly: a backend with many pending tokens still spends a long time in
|
||||
"starting" because bulk recovery inherits the interactive 15-minute
|
||||
`AgentLaunchTimeout` per task. A bulk startup sweep needs its own, much
|
||||
smaller per-task budget, and a resume path that can actually be abandoned. That
|
||||
requires changing the `context.WithoutCancel` resume contract and is a separate
|
||||
piece of work with its own spec.
|
||||
@@ -304,6 +304,11 @@ promotion.
|
||||
only after the committed promotion is observable.
|
||||
- Startup reconciliation resumes queued promotion without depending on a new
|
||||
watcher poll or browser connection.
|
||||
- Startup reconciliation keeps its position before the watcher and scheduler
|
||||
start, but it never delays the HTTP socket from binding: resuming a promotion
|
||||
can launch an agent, and agent readiness is not bounded by the launcher's
|
||||
health window. See
|
||||
[`startup-listener-before-recovery`](../../startup-listener-before-recovery/spec.md).
|
||||
|
||||
## Scenarios
|
||||
|
||||
|
||||
@@ -105,7 +105,8 @@ Each Playwright worker spawns an isolated `kandev` backend that also serves the
|
||||
- `KANDEV_DOCKER_ENABLED=false` — no Docker
|
||||
- `KANDEV_WORKTREE_ENABLED=false` — no worktrees
|
||||
- `GH_TOKEN` / `GITHUB_TOKEN` stripped — prevents accidental real API calls
|
||||
3. Spawns `apps/backend/bin/kandev __backend` and waits for `/health` to return 200
|
||||
3. Spawns `apps/backend/bin/kandev __backend` and waits for `/ready` to return 200 (not `/health`,
|
||||
which flips green as soon as the listener is bound, before the mock-harness routes are mounted)
|
||||
|
||||
**Frontend:**
|
||||
1. Sets `KANDEV_WEB_DIST_DIR` to `apps/web/dist`
|
||||
|
||||
+1
-1
@@ -44,7 +44,7 @@ spec:
|
||||
failureThreshold: 3
|
||||
readinessProbe:
|
||||
httpGet:
|
||||
path: /health
|
||||
path: /ready
|
||||
port: backend
|
||||
initialDelaySeconds: 10
|
||||
periodSeconds: 10
|
||||
|
||||
@@ -45,10 +45,14 @@ class Kandev < Formula
|
||||
ENV["KANDEV_SERVER_HOST"] = "127.0.0.1"
|
||||
port = free_port
|
||||
pid = spawn bin/"kandev", "--headless", "--port", port.to_s
|
||||
health_url = "http://127.0.0.1:#{port}/health"
|
||||
# /health flips to 200 as soon as the listener binds, before startup
|
||||
# recovery finishes, so curl's --retry (which only retries on
|
||||
# connection-refused/5xx, never on 200) would stop waiting immediately.
|
||||
# /ready holds at 503 until the app is actually usable.
|
||||
ready_url = "http://127.0.0.1:#{port}/ready"
|
||||
curl = "curl --silent --show-error --fail --retry 30 --retry-connrefused --retry-delay 1"
|
||||
health = shell_output("#{curl} #{health_url}")
|
||||
assert_match '"status":"ok"', health
|
||||
ready = shell_output("#{curl} #{ready_url}")
|
||||
assert_match '"status":"ok"', ready
|
||||
assert_match "<title>Kandev</title>", shell_output("#{curl} http://127.0.0.1:#{port}/")
|
||||
ensure
|
||||
Process.kill("TERM", pid) if pid
|
||||
|
||||
Reference in New Issue
Block a user