From 35d2af3063da80800674cf2a9da9ba4b76d9c0c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 14:03:02 +0000 Subject: [PATCH] fix(watch): Treat an unparseable readiness file as not-ready-yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit waitWatchReadiness returned on the first successful read, so a readiness file caught mid-write — existing but empty or partial — failed json.Unmarshal and aborted the wait immediately, reporting a startup failure for a daemon that had not finished writing. Only os.ErrNotExist counted as "not ready". Measured against the real function: an empty file returns "reading daemon readiness: unexpected end of JSON input" after 0s, without waiting out any part of the 30s timeout. Keep polling on a parse failure until the deadline, and surface the last parse error when the deadline passes, so a file that never becomes valid still says why rather than only that it timed out. publishWatchReadiness already renames its payload into place atomically, so codemap's own daemon does not open this window; the reader was brittle to any writer that is not atomic. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo --- main.go | 23 +++++++++----- watch_readiness_test.go | 66 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 watch_readiness_test.go diff --git a/main.go b/main.go index e14ae1b..27f8d0c 100644 --- a/main.go +++ b/main.go @@ -1153,22 +1153,31 @@ func publishWatchReadiness(path string, readinessErr error) error { func waitWatchReadiness(path string, timeout time.Duration) error { deadline := time.Now().Add(timeout) + // A readiness file that does not parse is a file still being written, not + // a daemon that failed: publishWatchReadiness renames its payload into + // place atomically, but nothing guarantees every writer does, and treating + // the first unparseable read as fatal reported a startup failure for a + // daemon that was starting fine. Keep the last parse error so a file that + // never becomes valid says why, rather than only that it timed out. + var lastParseErr error for { data, err := os.ReadFile(path) if err == nil { var status watchReadiness - if err := json.Unmarshal(data, &status); err != nil { - return fmt.Errorf("reading daemon readiness: %w", err) - } - if status.Error != "" { + if parseErr := json.Unmarshal(data, &status); parseErr != nil { + lastParseErr = parseErr + } else if status.Error != "" { return errors.New(status.Error) + } else { + return nil } - return nil - } - if !errors.Is(err, os.ErrNotExist) { + } else if !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("reading daemon readiness: %w", err) } if time.Now().After(deadline) { + if lastParseErr != nil { + return fmt.Errorf("reading daemon readiness: %w", lastParseErr) + } return fmt.Errorf("daemon readiness timed out after %s", timeout) } time.Sleep(10 * time.Millisecond) diff --git a/watch_readiness_test.go b/watch_readiness_test.go new file mode 100644 index 0000000..e2a10cb --- /dev/null +++ b/watch_readiness_test.go @@ -0,0 +1,66 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// A readiness file caught mid-write reads as zero or partial bytes. Treating +// that as fatal reported a startup failure for a daemon that was starting +// fine, and made TestRunWatchStartWaitsForChildReadinessFailure fail whenever +// the machine was loaded enough to land in the window. +func TestWaitWatchReadinessWaitsThroughPartialWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "ready.json") + if err := os.WriteFile(path, nil, 0o644); err != nil { + t.Fatal(err) + } + go func() { + time.Sleep(50 * time.Millisecond) + _ = os.WriteFile(path, []byte(`{"error":"claim rejected"}`), 0o644) + }() + + err := waitWatchReadiness(path, 5*time.Second) + if err == nil || !strings.Contains(err.Error(), "claim rejected") { + t.Fatalf("waitWatchReadiness() = %v, want the daemon's own error once the file is complete", err) + } +} + +func TestWaitWatchReadinessSucceedsAfterPartialWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "ready.json") + if err := os.WriteFile(path, []byte(`{"err`), 0o644); err != nil { + t.Fatal(err) + } + go func() { + time.Sleep(50 * time.Millisecond) + _ = os.WriteFile(path, []byte(`{}`), 0o644) + }() + + if err := waitWatchReadiness(path, 5*time.Second); err != nil { + t.Fatalf("waitWatchReadiness() = %v, want success once the file is complete", err) + } +} + +// A file that never becomes valid still has to say why, rather than reporting +// only that it timed out. +func TestWaitWatchReadinessReportsPersistentGarbage(t *testing.T) { + path := filepath.Join(t.TempDir(), "ready.json") + if err := os.WriteFile(path, []byte("not json at all"), 0o644); err != nil { + t.Fatal(err) + } + err := waitWatchReadiness(path, 200*time.Millisecond) + if err == nil || !strings.Contains(err.Error(), "reading daemon readiness") { + t.Fatalf("waitWatchReadiness() = %v, want the parse failure surfaced", err) + } +} + +// A missing file must still time out rather than hang. +func TestWaitWatchReadinessTimesOutWhenAbsent(t *testing.T) { + path := filepath.Join(t.TempDir(), "never-written.json") + err := waitWatchReadiness(path, 100*time.Millisecond) + if err == nil || !strings.Contains(err.Error(), "timed out") { + t.Fatalf("waitWatchReadiness() = %v, want a timeout", err) + } +}