diff --git a/cmd/bodek/upgrade.go b/cmd/bodek/upgrade.go index de7ea14..3af7b9b 100644 --- a/cmd/bodek/upgrade.go +++ b/cmd/bodek/upgrade.go @@ -8,6 +8,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "fmt" "io" "net/http" @@ -101,15 +102,27 @@ func archiveName(tag, goos, goarch string) string { return fmt.Sprintf("bodek_%s_%s_%s.%s", tag, goos, goarch, ext) } +// downloadTimeout bounds a single release-asset body read. It must be a +// per-request deadline, not Client.Timeout: the timeout bounds the WHOLE +// transfer including the body, so a multi-MB archive over a slow link would +// fail every time under the short API budget. +const downloadTimeout = 10 * time.Minute + // download fetches url into memory. Release archives are a few MB, so a // buffered read is fine and keeps checksum verification straightforward. func download(ctx context.Context, client *http.Client, url string) ([]byte, error) { + // A copy without the overall Timeout: the deadline below bounds the + // read instead. The transport (TLS cache, dialer) is shared. + dl := *client + dl.Timeout = 0 + ctx, cancel := context.WithTimeout(ctx, downloadTimeout) + defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, fmt.Errorf("build download request: %w", err) } req.Header.Set("User-Agent", "bodek-updater") - resp, err := client.Do(req) + resp, err := dl.Do(req) if err != nil { return nil, fmt.Errorf("download %s: %w", url, err) } @@ -210,10 +223,18 @@ func extractZip(archive []byte) ([]byte, error) { return nil, fmt.Errorf("archive contains no bodek binary") } +// renameFn and syncFile are test hooks over os.Rename and (*os.File).Sync +// so the swap sequence is injectable. +var ( + renameFn = os.Rename + syncFile = (*os.File).Sync +) + // replaceExecutable atomically swaps the binary at target with data: the new -// file is written next to the target and renamed over it, so a crash -// mid-upgrade never leaves a truncated binary. target is resolved through -// symlinks first so `go install` shims and PATH links are not clobbered. +// file is written next to the target, fsynced, and renamed over the old one, +// so a crash mid-upgrade never leaves a truncated binary. target is resolved +// through symlinks first so `go install` shims and PATH links are not +// clobbered. func replaceExecutable(data []byte, target string) error { resolved, err := filepath.EvalSymlinks(target) if err != nil { @@ -224,8 +245,15 @@ func replaceExecutable(data []byte, target string) error { return fmt.Errorf("create temp file next to %s: %w", resolved, err) } tmpName := tmp.Name() - // No-op once the rename below has moved the temp file into place. - defer func() { _ = os.Remove(tmpName) }() + // No-op once the rename below has moved the temp file into place — but + // only the success path: the Windows rollback-fail path leaves the new + // binary at tmpName on purpose, so installFailed keeps it alive. + installFailed := false + defer func() { + if !installFailed { + _ = os.Remove(tmpName) + } + }() if _, err := tmp.Write(data); err != nil { _ = tmp.Close() return fmt.Errorf("write new binary: %w", err) @@ -237,21 +265,54 @@ func replaceExecutable(data []byte, target string) error { if err := tmp.Close(); err != nil { return fmt.Errorf("flush new binary: %w", err) } - if err := os.Rename(tmpName, resolved); err != nil { + reopened, err := os.Open(tmpName) + if err == nil { + err = syncFile(reopened) + _ = reopened.Close() + } + if err != nil { + return fmt.Errorf("sync new binary: %w", err) + } + if err := renameFn(tmpName, resolved); err != nil { if runtime.GOOS != "windows" { return fmt.Errorf("replace %s: %w", resolved, err) } - // Windows refuses to rename over a running executable; move the old - // one aside first, then drop the new binary into place. - old := resolved + ".old" - _ = os.Remove(old) - if rerr := os.Rename(resolved, old); rerr != nil { - return fmt.Errorf("move current executable aside: %w", rerr) + // Windows refuses to rename over a running executable: swap the old + // one aside, then drop the new binary into place (with rollback). + if serr := swapAsideWindows(resolved, tmpName); serr != nil { + // On the double-failure path the new binary deliberately + // survives at tmpName — keep the deferred cleanup off it. + var kept tmpKeptError + installFailed = errors.As(serr, &kept) + return serr } - if rerr := os.Rename(tmpName, resolved); rerr != nil { - return fmt.Errorf("install new binary: %w", rerr) + return nil + } + return nil +} + +// tmpKeptError marks a failure where the staged new binary intentionally +// survives at its temp path (both Windows renames failed) — the deferred +// cleanup in replaceExecutable must not delete it. +type tmpKeptError struct{ error } + +// swapAsideWindows installs tmpName over resolved on Windows, moving the +// running binary to resolved+".old" first. A failed install rename rolls +// the old binary back — the swap must never strand the executable as .old. +func swapAsideWindows(resolved, tmpName string) error { + old := resolved + ".old" + _ = os.Remove(old) + if rerr := renameFn(resolved, old); rerr != nil { + return fmt.Errorf("move current executable aside: %w", rerr) + } + if rerr := renameFn(tmpName, resolved); rerr != nil { + if rberr := renameFn(old, resolved); rberr != nil { + // Both renames failed: the new binary stays at tmpName on + // purpose — tell the operator where both halves live. + return tmpKeptError{fmt.Errorf("install new binary: %w (rollback also failed: %v — old binary is at %s, new binary is at %s)", rerr, rberr, old, tmpName)} } - _ = os.Remove(old) // best effort: a locked .old goes away on a later run + return fmt.Errorf("install new binary: %w", rerr) } + _ = os.Remove(old) // best effort: a locked .old goes away on a later run return nil } diff --git a/cmd/bodek/upgrade_wave3_test.go b/cmd/bodek/upgrade_wave3_test.go new file mode 100644 index 0000000..dc6c845 --- /dev/null +++ b/cmd/bodek/upgrade_wave3_test.go @@ -0,0 +1,117 @@ +package main + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/BackendStack21/bodek/internal/update" +) + +// TestSwapAsideWindowsRollback guards the Windows swap: if the second +// rename (new binary into place) fails after the running binary was moved +// aside, the old binary must be restored — otherwise the install path is +// bricked with the executable stranded as .old. +func TestSwapAsideWindowsRollback(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "bodek") + if err := os.WriteFile(target, []byte("old-binary"), 0o755); err != nil { + t.Fatal(err) + } + tmp := filepath.Join(dir, ".bodek-upgrade-new") + if err := os.WriteFile(tmp, []byte("new-binary"), 0o755); err != nil { + t.Fatal(err) + } + + realRename := renameFn + defer func() { renameFn = realRename }() + calls := 0 + renameFn = func(from, to string) error { + calls++ + if calls == 2 { // the "install new binary" rename fails + return errors.New("access denied") + } + return realRename(from, to) + } + + err := swapAsideWindows(target, tmp) + if err == nil { + t.Fatal("expected the injected rename failure to surface") + } + got, rerr := os.ReadFile(target) + if rerr != nil { + t.Fatalf("executable missing after failed swap: %v", rerr) + } + if string(got) != "old-binary" { + t.Fatalf("old binary not restored after failed swap: %q", got) + } +} + +// TestReplaceExecutableSyncsBeforeRename guards durability: the new binary +// must be fsynced before the rename, or a crash can persist the rename +// with no/partial data behind it — a zero-length bodek — despite the +// "never leaves a truncated binary" contract. +func TestReplaceExecutableSyncsBeforeRename(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "bodek") + if err := os.WriteFile(target, []byte("old"), 0o755); err != nil { + t.Fatal(err) + } + + realSync := syncFile + defer func() { syncFile = realSync }() + synced := false + syncFile = func(f *os.File) error { synced = true; return realSync(f) } + + if err := replaceExecutable([]byte("new"), target); err != nil { + t.Fatalf("replaceExecutable: %v", err) + } + if !synced { + t.Fatal("new binary was never fsynced before the rename") + } +} + +// TestUpgradeSlowLinkDownloads guards the download budget: Client.Timeout +// bounds the whole body read, so reusing the short API client for a +// multi-MB archive fails every transfer slower than that budget. The +// download path must not inherit the API client's overall deadline. +func TestUpgradeSlowLinkDownloads(t *testing.T) { + archive := buildTarGz(t, "bodek", []byte("slow-payload")) + slow := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(2 * time.Second) + _, _ = w.Write(archive) + })) + defer slow.Close() + + // A 300ms overall client budget vs a 2s drip — margins wide enough to + // be CI-stable in both directions. + c := &http.Client{Timeout: 300 * time.Millisecond} + data, err := download(context.Background(), c, slow.URL+"/bodek.tar.gz") + if err != nil { + t.Fatalf("download on a slow link failed: %v", err) + } + if len(data) == 0 { + t.Fatal("empty download") + } +} + +// TestNewerPseudoVersion guards commit-installed builds: a Go pseudo-version +// stamp (v0.1.3-0.20260901abcdef12-abc1234) must compare by its release +// prefix, not report "already up to date" against every future release. +func TestNewerPseudoVersion(t *testing.T) { + if !update.Newer("v9.9.9", "v0.1.3-0.20260901000000-abc1234") { + t.Fatal("v9.9.9 must be newer than a v0.1.3 pseudo-version stamp") + } + if update.Newer("v0.1.3", "v0.1.3-0.20260901000000-abc1234") { + t.Fatal("v0.1.3 must not upgrade over a v0.1.3 pseudo-version") + } + // A genuine semver prerelease must not be misparsed as a pseudo-version. + if !update.Newer("v1.3.0", "v1.2.0-0.1") { + t.Fatal("v1.3.0 must be newer than prerelease v1.2.0-0.1") + } +} diff --git a/internal/client/client.go b/internal/client/client.go index a33514b..b41cd00 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -248,6 +248,12 @@ type Client struct { stopOnce sync.Once // lazy: only jobs-stop pays the longer timeout budget stopClient *http.Client // 12s — odek's stop endpoint blocks stopGrace+4s slowHTTP *http.Client // 30s — bulk payload reads (session detail, export) + + // done is closed by Close. Every Events send selects on it: when a + // reconnect abandons a full Events channel, readLoop must exit instead of + // parking forever on a send nobody will ever receive. + done chan struct{} + closeOnce sync.Once } // Dial connects to an odek serve WebSocket. wsURL is the ws:// endpoint, @@ -281,6 +287,7 @@ func Dial(wsURL, origin, baseURL, token string) (*Client, error) { // dedicated 30s client serves them so the 3s interactive budget does // not cut their body reads. slowHTTP: &http.Client{Timeout: 30 * time.Second}, + done: make(chan struct{}), } go c.readLoop() return c, nil @@ -363,7 +370,7 @@ func (c *Client) readLoop() { if pending == nil { return } - c.Events <- *pending + c.emit(*pending) pending = nil n = 0 } @@ -372,7 +379,7 @@ func (c *Client) readLoop() { _ = c.conn.SetReadDeadline(time.Now().Add(readIdleTimeout)) if err := ws.Message.Receive(c.conn, &data); err != nil { flush() - c.Events <- Event{Type: EventDisconnected} + c.emit(Event{Type: EventDisconnected}) return } _ = c.conn.SetReadDeadline(time.Time{}) // received: drop the deadline while decoding @@ -397,7 +404,7 @@ func (c *Client) readLoop() { continue } flush() - c.Events <- ev + c.emit(ev) } } @@ -524,10 +531,24 @@ func (c *Client) send(v any) error { return ws.JSON.Send(c.conn, v) } -// Close shuts the connection. +// Close shuts the connection and releases a readLoop parked on Events. func (c *Client) Close() error { + c.closeOnce.Do(func() { + if c.done != nil { + close(c.done) + } + }) if c.conn == nil { return nil } return c.conn.Close() } + +// emit delivers ev to Events unless the client is closed. A send to an +// abandoned full channel would otherwise park readLoop forever. +func (c *Client) emit(ev Event) { + select { + case c.Events <- ev: + case <-c.done: + } +} diff --git a/internal/client/reconnect_leak_test.go b/internal/client/reconnect_leak_test.go new file mode 100644 index 0000000..ec9fcc9 --- /dev/null +++ b/internal/client/reconnect_leak_test.go @@ -0,0 +1,62 @@ +package client + +import ( + "net/http" + "net/http/httptest" + "runtime" + "testing" + "time" + + ws "golang.org/x/net/websocket" +) + +// TestReadLoopExitsWhenEventsAbandoned guards the reconnect swap: when the +// consumer abandons a full Events channel (a reconnect during a delta +// firehose), closing the socket must let readLoop exit instead of parking +// forever on a send to a channel nobody reads. +func TestReadLoopExitsWhenEventsAbandoned(t *testing.T) { + // A 10x flood guarantees the parked state: once the consumer stops + // draining, readLoop refills the channel and parks on a send. + flood := eventBuffer * 10 + done := make(chan struct{}) + mux := http.NewServeMux() + mux.Handle("/ws", ws.Handler(func(c *ws.Conn) { + defer close(done) + for i := 0; i < flood; i++ { + if err := ws.Message.Send(c, `{"type":"note","content":"x"}`); err != nil { + return + } + } + })) + srv := httptest.NewServer(mux) + defer srv.Close() + wsURL := "ws" + srv.URL[len("http"):] + base := runtime.NumGoroutine() // before Dial: excludes readLoop entirely + cl, err := Dial(wsURL+"/ws", srv.URL, srv.URL, "test-token") + if err != nil { + t.Fatalf("Dial: %v", err) + } + + // Consume exactly the channel capacity, then vanish — the readLoop is + // provably parked on a send to the full channel, exactly the reconnect + // swap's state when reconnect.go drops the old client mid-firehose. + for i := 0; i < eventBuffer; i++ { + select { + case <-cl.Events: + case <-time.After(10 * time.Second): + t.Fatalf("stalled draining event %d", i) + } + } + // The 104 overflow frames are in flight; give readLoop a moment to park. + time.Sleep(200 * time.Millisecond) + + _ = cl.Close() + + deadline := time.Now().Add(5 * time.Second) + for runtime.NumGoroutine() > base && time.Now().Before(deadline) { + time.Sleep(50 * time.Millisecond) + } + if n := runtime.NumGoroutine(); n > base { + t.Fatalf("readLoop goroutine leaked after Close on an abandoned full Events channel: %d goroutines (base %d)", n, base) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 723299b..6369e82 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -351,7 +351,10 @@ func (s *tokenScanWriter) scan(p []byte) { s.mu.Lock() defer s.mu.Unlock() if s.tok != "" { - return // already found; keep passing bytes through + // Token already found: stop parsing, but keep the tail following the + // server's output so a post-banner failure reaches the error card. + s.appendTail(p) + return } s.buf = append(s.buf, p...) for { @@ -361,10 +364,7 @@ func (s *tokenScanWriter) scan(p []byte) { } line := string(s.buf[:i]) s.buf = s.buf[i+1:] - s.tail = append(s.tail, line) - if len(s.tail) > maxTailLines { - s.tail = s.tail[len(s.tail)-maxTailLines:] - } + s.appendTailLine(line) if tok := parseTokenLine(line); tok != "" { s.tok = tok return @@ -372,6 +372,44 @@ func (s *tokenScanWriter) scan(p []byte) { } } +// appendTail splits p into complete lines and keeps the last maxTailLines +// of them in the diagnostics tail. Callers hold s.mu. +func (s *tokenScanWriter) appendTail(p []byte) { + rest := p + for { + i := bytes.IndexByte(rest, '\n') + if i < 0 { + s.partialTail(rest) + return + } + s.appendTailLine(string(rest[:i])) + rest = rest[i+1:] + } +} + +// partialTail buffers a trailing chunk without a newline so a failure line +// split across Write calls still lands whole in the tail. +func (s *tokenScanWriter) partialTail(chunk []byte) { + if len(chunk) == 0 { + return + } + s.buf = append(s.buf, chunk...) + // Only remember it as a line if it eventually terminates; the next + // appendTail call re-reads s.buf from the start. + if i := bytes.IndexByte(s.buf, '\n'); i >= 0 { + line := string(s.buf[:i]) + s.buf = s.buf[i+1:] + s.appendTailLine(line) + } +} + +func (s *tokenScanWriter) appendTailLine(line string) { + s.tail = append(s.tail, line) + if len(s.tail) > maxTailLines { + s.tail = s.tail[len(s.tail)-maxTailLines:] + } +} + // parseTokenLine extracts the token from a "WS token: " line, falling // back to the "?token=" query in the "odek serve ⚡ " banner line. func parseTokenLine(line string) string { @@ -450,7 +488,7 @@ func waitSpawned(baseURL string, scan *tokenScanWriter, alive func() bool, timeo deadline := time.Now().Add(timeout) readyAt := time.Time{} for time.Now().Before(deadline) { - if scan.Token() != "" { + if scan != nil && scan.Token() != "" { return nil } if probeReady(baseURL) { @@ -467,13 +505,21 @@ func waitSpawned(baseURL string, scan *tokenScanWriter, alive func() bool, timeo // this iteration's top-of-loop check — the token wins. if alive != nil && !alive() { if scan == nil || scan.Token() == "" { - return fmt.Errorf("odek serve exited before becoming ready%w", stderrTail(scan)) + err := fmt.Errorf("odek serve exited before becoming ready") + if tail := stderrTail(scan); tail != nil { + return fmt.Errorf("%w%w", err, tail) + } + return err } return nil } time.Sleep(150 * time.Millisecond) } - return fmt.Errorf("timed out after %s%w", timeout, stderrTail(scan)) + err := fmt.Errorf("timed out after %s", timeout) + if tail := stderrTail(scan); tail != nil { + return fmt.Errorf("%w%w", err, tail) + } + return err } // stderrTail returns the captured server stderr tail for inclusion in a diff --git a/internal/server/spawn_diag_test.go b/internal/server/spawn_diag_test.go new file mode 100644 index 0000000..52595fc --- /dev/null +++ b/internal/server/spawn_diag_test.go @@ -0,0 +1,47 @@ +package server + +import ( + "strings" + "testing" + "time" +) + +// TestWaitSpawnedErrorHasNoNilVerb guards the failure-card text: a dead +// server with an empty (or absent) stderr tail must not render Go's +// "%!w()" artifact into the user-facing error. +func TestWaitSpawnedErrorHasNoNilVerb(t *testing.T) { + err := waitSpawned("http://127.0.0.1:1", nil, func() bool { return false }, 300*time.Millisecond) + if err == nil { + t.Fatal("waitSpawned: want error for a dead server, got nil") + } + if strings.Contains(err.Error(), "%!") { + t.Fatalf("waitSpawned error contains a bad verb: %q", err.Error()) + } + if !strings.Contains(err.Error(), "exited before becoming ready") { + t.Fatalf("waitSpawned error lost its reason: %q", err.Error()) + } + + err = waitSpawned("http://127.0.0.1:1", nil, func() bool { return true }, 250*time.Millisecond) + if err == nil || strings.Contains(err.Error(), "%!") { + t.Fatalf("timeout path rendered a bad verb or no error: %v", err) + } +} + +// TestTokenScanTailKeepsFollowingServerOutput guards the diagnostics tail: +// a server that prints its startup banner (with the token) and THEN fails +// must surface the late failure lines in the tail, not the banner itself. +func TestTokenScanTailKeepsFollowingServerOutput(t *testing.T) { + s := &tokenScanWriter{w: &strings.Builder{}} + s.scan([]byte("odek serve ⚡ http://127.0.0.1:8080/?token=abc\n")) + s.scan([]byte(" WS token: abc\n")) + if s.Token() == "" { + t.Fatal("scan missed the token line") + } + s.scan([]byte("config error: invalid provider\n")) + s.scan([]byte("FATAL: cannot start\n")) + + tail := s.Tail(maxTailLines) + if !strings.Contains(tail, "FATAL: cannot start") { + t.Fatalf("tail froze at the banner; late failure line missing: %q", tail) + } +} diff --git a/internal/tui/approval.go b/internal/tui/approval.go index 2a8b338..3da4073 100644 --- a/internal/tui/approval.go +++ b/internal/tui/approval.go @@ -325,11 +325,13 @@ func (m *Model) answer(action string) tea.Cmd { id := a.ID head := *a var dl time.Time + bell := false if len(m.apprDeadlines) > 0 { dl = m.apprDeadlines[0] m.apprDeadlines = m.apprDeadlines[1:] // keep the parallel expiry queue in lockstep } if len(m.apprBells) > 0 { + bell = m.apprBells[0] m.apprBells = m.apprBells[1:] } m.approvals = m.approvals[1:] @@ -344,7 +346,7 @@ func (m *Model) answer(action string) tea.Cmd { cl := m.cl return func() tea.Msg { if err := cl.SendApproval(id, action); err != nil { - return approvalSendErrMsg{ev: head, dl: dl, err: err} + return approvalSendErrMsg{ev: head, dl: dl, bell: bell, err: err} } return nil } diff --git a/internal/tui/approval_bell_restore_test.go b/internal/tui/approval_bell_restore_test.go new file mode 100644 index 0000000..241a7ff --- /dev/null +++ b/internal/tui/approval_bell_restore_test.go @@ -0,0 +1,39 @@ +package tui + +import ( + "errors" + "testing" + + "github.com/BackendStack21/bodek/internal/client" +) + +// TestApprovalSendErrRestoresBellLatch guards the parallel-array contract: +// apprBells must stay 1:1 with apprDeadlines when a failed send restores +// the popped head — otherwise the urgent-window BEL latch reads a +// neighbor's flag and fires for the wrong request for the rest of the +// session. +func TestApprovalSendErrRestoresBellLatch(t *testing.T) { + m := newTestModel() + busyTurn(m) + m.handleEvent(client.Event{Type: "approval_request", ID: "apr-1", Risk: "shell_exec", Command: "echo a"}) + m.handleEvent(client.Event{Type: "approval_request", ID: "apr-2", Risk: "shell_exec", Command: "echo b"}) + if len(m.approvals) != 2 || len(m.apprBells) != 2 { + t.Fatalf("precondition: 2 approvals expected, got %d/%d", len(m.approvals), len(m.apprBells)) + } + + // Mirror answer()'s pop of the head before the wire write fails: + // head + deadline + bell leave the queues, then the restore must put + // all three back in lockstep. + ev := *m.curApproval() + dl := m.apprDeadlines[0] + m.approvals = m.approvals[1:] + m.apprDeadlines = m.apprDeadlines[1:] + m.apprBells = m.apprBells[1:] + + m.Update(approvalSendErrMsg{ev: ev, dl: dl, err: errors.New("send failed")}) + + if len(m.approvals) != 2 || len(m.apprDeadlines) != 2 || len(m.apprBells) != 2 { + t.Fatalf("send-fail restore lost an array: appr=%d dl=%d bells=%d", + len(m.approvals), len(m.apprDeadlines), len(m.apprBells)) + } +} diff --git a/internal/tui/approval_expiry.go b/internal/tui/approval_expiry.go index da13aad..4e4fd84 100644 --- a/internal/tui/approval_expiry.go +++ b/internal/tui/approval_expiry.go @@ -141,6 +141,11 @@ func (m *Model) handleApprovalExpiry(now time.Time) tea.Cmd { m.resetApprovalInput() m.relayout() } + if dropped > 0 { + // A mid-queue expiry changes the card's "N queued" hint without + // touching the head — the rendered form must repaint anyway. + m.refresh() + } if len(m.approvals) == 0 { // The card is gone — jump to the latest transcript message so the // operator lands on the turn the engine continues, not the scroll diff --git a/internal/tui/events.go b/internal/tui/events.go index 2ac0c95..f4f1285 100644 --- a/internal/tui/events.go +++ b/internal/tui/events.go @@ -322,9 +322,10 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { // (applyCtxWindow). Absent/zero holds the last fill. This-call // tok/s is the same contract: a missing rate is held, never // invented from cumulative outputTokens / wall latency. - // Open a card first so a usage-first remote/wake turn (missed - // turn_started) resets the previous chip before this frame lands. - m.ensureWireTurn() + // No ensureWireTurn here: a usage frame trailing a finalized turn + // (done/usage batching order) must not open an orphan card — the + // lazy fallback exists for thinking/token frames, which always + // precede usage in a genuine turn. m.applyCtxWindow(ev) m.applyCallMetrics(ev) stream = true @@ -414,6 +415,7 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { } else { m.status = "error" } + m.runCtxCum = 0 // same contract as done: the failed run's cumulative is spent m.restoreComposerPrompt() m.relayout() // the busy status line releases its row @@ -533,11 +535,11 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { // Push notification of a job start/exit (≥ v1.40): refresh the // snapshot now instead of waiting for the next watcher tick. The // REST watcher stays as the fallback; applyJobs diffs the - // transition into notes/attention as before. - if cmd := m.kickJobsFetch(); cmd != nil { - m.refresh() - return m, tea.Batch(listen(m.events), m.noticeSweep(), cmd) - } + // transition into notes/attention as before. The kick rides the + // flushKicks flag — a per-event cmd is dropped when the frame + // arrives inside a listen-drained batch. + m.kickJobs = true + m.refresh() case client.EventDisconnected: m.disconn = true @@ -618,6 +620,12 @@ func (m *Model) flushKicks() tea.Cmd { cmds = append(cmds, cmd) } } + if m.kickJobs { + m.kickJobs = false + if cmd := m.kickJobsFetch(); cmd != nil { + cmds = append(cmds, cmd) + } + } if m.kickMemory { m.kickMemory = false if cmd := m.kickMemoryFetch(); cmd != nil { diff --git a/internal/tui/jobs_reopen_test.go b/internal/tui/jobs_reopen_test.go new file mode 100644 index 0000000..2314bf0 --- /dev/null +++ b/internal/tui/jobs_reopen_test.go @@ -0,0 +1,33 @@ +package tui + +import ( + "testing" +) + +// TestJobsReopenKillsStaleTabTick guards the double-arm race: open → close +// → reopen leaves a tab tick in flight from the FIRST open. When it fires, +// its (now stale) seq must be rejected — otherwise it passes the seq check +// against the reopened tab and arms a second permanent watcher chain, +// doubling the /api/jobs poll rate for the session. +func TestJobsReopenKillsStaleTabTick(t *testing.T) { + m := newJobsTestModel(t, nil) + + m.openJobs() // first open: arms a 3s tab chain (tick in flight) + seqFirst := m.jobsSeq + m.jobsSeq-- // simulate: the in-flight tick captured seqFirst + m.jobsSeq++ + + // Reopen (close/reopen cycle): must invalidate the first chain's tick. + m.openJobs() + + stale := jobsTickMsg{seq: seqFirst, watch: false} + cmd := m.handleJobsTick(stale) + if cmd != nil { + // Even if it fetched, it must not hand the cadence to a fresh + // watcher chain on top of the reopened tab's own chain. + t.Fatal("stale tab tick from the first open was accepted after reopen") + } + if m.jobsSeq != seqFirst+1 { + t.Fatalf("reopen did not bump the tab generation: %d vs %d", m.jobsSeq, seqFirst+1) + } +} diff --git a/internal/tui/jobs_tab.go b/internal/tui/jobs_tab.go index 3f7ec66..c3a5ea7 100644 --- a/internal/tui/jobs_tab.go +++ b/internal/tui/jobs_tab.go @@ -61,6 +61,8 @@ func (m *Model) openJobs() tea.Cmd { m.panelSel = 0 m.panelEdit = panelEditNone m.jobsWatchSeq++ // a pending watcher tick would double-fetch + m.jobsSeq++ // a pending TAB tick would too: it would pass the seq + // check after reopen and arm a second watcher chain m.relayout() m.refresh() if m.jobsOff { diff --git a/internal/tui/messages.go b/internal/tui/messages.go index 60ce83d..d9e99a3 100644 --- a/internal/tui/messages.go +++ b/internal/tui/messages.go @@ -36,9 +36,10 @@ func errText(err error) string { // must not end the turn: the engine is still waiting on the request, so the // popped head is restored and remaining queue items stay armed. type approvalSendErrMsg struct { - ev client.Event - dl time.Time - err error + ev client.Event + dl time.Time + bell bool // the popped head's urgent-window BEL latch, restored with it + err error } // skillSendErrMsg is a failed skill_prompt_response write. Unlike errMsg it diff --git a/internal/tui/model.go b/internal/tui/model.go index d2423a4..299f1d6 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -290,6 +290,7 @@ type Model struct { agentsSeq int // agents-tab poll generation; stale ticks drop eventsTabSeq int // events-tab poll generation; stale ticks drop kickAgents bool // pending agents-tab refresh (flushKicks) + kickJobs bool // pending jobs-tab refresh (flushKicks) kickMemory bool // pending memory-tab refresh (flushKicks) planConfirmIssued bool // tool_result debounce fired a confirm fetch @@ -298,6 +299,7 @@ type Model struct { jobs []client.Job jobsPrev map[string]string // watcher diff state: id → last status jobsSeq int // tab poll generation + reconnGen int // reconnect chain generation (manual retry guard) jobsWatchSeq int // 10s watcher generation jobsOff bool // server predates /api/jobs — stop watching jobsOut string // detail: fetched output (sanitized at render) @@ -599,6 +601,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.approvals = append([]client.Event{msg.ev}, m.approvals...) m.apprDeadlines = append([]time.Time{msg.dl}, m.apprDeadlines...) + m.apprBells = append([]bool{msg.bell}, m.apprBells...) m.setRunStatus("approval required") m.resetApprovalInput() m.addNote("approval send failed — " + reason) diff --git a/internal/tui/reconnect.go b/internal/tui/reconnect.go index 3b8763e..22b5ebf 100644 --- a/internal/tui/reconnect.go +++ b/internal/tui/reconnect.go @@ -15,6 +15,7 @@ const maxReconnectAttempts = 5 // reconnectMsg carries the outcome of one redial attempt. type reconnectMsg struct { attempt int + gen int // scheduleReconnect chain that produced this result cl *client.Client err error } @@ -30,15 +31,21 @@ func reconnectBackoff(attempt int) time.Duration { // scheduleReconnect runs one redial (via the Reconnect hook main wires in) // after the attempt's backoff tick. Nil hook means reconnects are disabled. +// reconnGen counts reconnect chains. A manual ⏎ retry must not race a +// pending backoff tick into two concurrent hook dials: every schedule +// bumps the generation, and a reconnectMsg from a superseded chain is +// dropped (and its socket closed). func (m *Model) scheduleReconnect(attempt int) tea.Cmd { hook := m.opts.Reconnect if hook == nil { return nil } + m.reconnGen++ + gen := m.reconnGen m.reconnAttempt = attempt // the status line's backoff readout follows the chain return tea.Tick(reconnectBackoff(attempt), func(time.Time) tea.Msg { cl, err := hook() - return reconnectMsg{attempt: attempt, cl: cl, err: err} + return reconnectMsg{attempt: attempt, gen: gen, cl: cl, err: err} }) } @@ -46,9 +53,9 @@ func (m *Model) scheduleReconnect(attempt int) tea.Cmd { // re-arms the event stream; failure retries with backoff until the attempt // budget is spent, then keeps the terminal disconnected state. func (m *Model) handleReconnect(msg reconnectMsg) (tea.Model, tea.Cmd) { - if !m.disconn { - // Stale result (e.g. the user quit and restarted): a successful dial - // nobody adopted would leak its socket — close it. + if !m.disconn || msg.gen != m.reconnGen { + // Stale result (superseded chain, or the user quit and restarted): + // a successful dial nobody adopted would leak its socket — close it. if msg.cl != nil { _ = msg.cl.Close() } diff --git a/internal/tui/reconnect_gen_test.go b/internal/tui/reconnect_gen_test.go new file mode 100644 index 0000000..03d769a --- /dev/null +++ b/internal/tui/reconnect_gen_test.go @@ -0,0 +1,58 @@ +package tui + +import ( + "errors" + "testing" + + "github.com/BackendStack21/bodek/internal/client" +) + +// TestManualRetryInvalidatesPendingBackoffTick guards the reconnect +// generation: a manual ⏎ retry while a backoff tick is pending must +// supersede that chain — the old chain's reconnectMsg must be dropped (and +// its socket closed) instead of racing the new dial into a double adopt. +func TestManualRetryInvalidatesPendingBackoffTick(t *testing.T) { + m := newTestModel() + m.disconn = true + m.opts.Reconnect = func() (*client.Client, error) { return nil, errors.New("dial refused") } + + // Chain 1: scheduled, tick still in flight. + if cmd := m.scheduleReconnect(0); cmd == nil { + t.Fatal("scheduleReconnect returned no cmd") + } + gen1 := m.reconnGen + + // Manual retry bumps the generation. + _ = m.scheduleReconnect(0) + gen2 := m.reconnGen + if gen2 == gen1 { + t.Fatal("manual retry did not bump the reconnect generation") + } + + // The stale chain's outcome arrives: must be dropped, socket closed. + stale := reconnectMsg{attempt: 0, gen: gen1, err: errors.New("superseded")} + mm, _ := m.handleReconnect(stale) + if mm.(*Model) != m { + t.Fatal("stale reconnectMsg changed the model") + } + // disconn untouched: the current chain stays armed. + if !m.disconn { + t.Fatal("stale reconnectMsg must not clear disconn") + } + + // The current chain's outcome is adopted. It dials and fails (hook + // error), so the retry chain continues — but the result is consumed, + // not dropped. + // The current chain's outcome is consumed: the failure schedules + // attempt+1, which opens the next generation and keeps disconn armed. + mm2, cmd2 := m.handleReconnect(reconnectMsg{attempt: 0, gen: gen2, err: errors.New("dial refused")}) + if !mm2.(*Model).disconn { + t.Fatal("failed redial must stay disconnected") + } + if cmd2 == nil { + t.Fatal("failed redial did not schedule the next attempt") + } + if mm2.(*Model).reconnGen != gen2+1 { + t.Fatalf("next attempt not scheduled in a fresh generation: %d", mm2.(*Model).reconnGen) + } +} diff --git a/internal/tui/reconnect_test.go b/internal/tui/reconnect_test.go index 61305eb..8d614fd 100644 --- a/internal/tui/reconnect_test.go +++ b/internal/tui/reconnect_test.go @@ -55,7 +55,7 @@ func TestReconnectSuccess(t *testing.T) { ch := make(chan client.Event, 1) cl := &client.Client{Events: ch} - _, cmd := m.Update(reconnectMsg{attempt: 0, cl: cl}) + _, cmd := m.Update(reconnectMsg{attempt: 0, gen: m.reconnGen, cl: cl}) if m.disconn { t.Error("should be connected again") @@ -87,12 +87,12 @@ func TestReconnectRetriesThenGivesUp(t *testing.T) { m.disconn = true m.opts.Reconnect = func() (*client.Client, error) { return nil, errors.New("down") } - _, cmd := m.Update(reconnectMsg{attempt: 0, err: errors.New("down")}) + _, cmd := m.Update(reconnectMsg{attempt: 0, gen: m.reconnGen, err: errors.New("down")}) if cmd == nil { t.Fatal("an early failure should schedule the next attempt") } - _, cmd = m.Update(reconnectMsg{attempt: maxReconnectAttempts - 1, err: errors.New("down")}) + _, cmd = m.Update(reconnectMsg{attempt: maxReconnectAttempts - 1, gen: m.reconnGen, err: errors.New("down")}) if cmd != nil { t.Error("no more attempts once the budget is spent") } @@ -117,7 +117,7 @@ func TestReconnectNilClientNoPanic(t *testing.T) { t.Fatalf("reconnect (nil, nil) panicked: %v", r) } }() - _, cmd := m.Update(reconnectMsg{attempt: maxReconnectAttempts - 1}) + _, cmd := m.Update(reconnectMsg{attempt: maxReconnectAttempts - 1, gen: m.reconnGen}) if cmd != nil { t.Error("no more attempts once the budget is spent") } @@ -144,7 +144,7 @@ func TestReconnectNilClientNoPanic(t *testing.T) { func TestReconnectStaleResultIgnored(t *testing.T) { m := newTestModel() // disconn == false - _, cmd := m.Update(reconnectMsg{attempt: 0, cl: &client.Client{Events: make(chan client.Event)}}) + _, cmd := m.Update(reconnectMsg{attempt: 0, gen: m.reconnGen, cl: &client.Client{Events: make(chan client.Event)}}) if cmd != nil { t.Error("stale reconnect result must not re-arm the listener") } diff --git a/internal/tui/round4_events_test.go b/internal/tui/round4_events_test.go new file mode 100644 index 0000000..51ea4e8 --- /dev/null +++ b/internal/tui/round4_events_test.go @@ -0,0 +1,64 @@ +package tui + +import ( + "testing" + + "github.com/BackendStack21/bodek/internal/client" +) + +// Round-4 wave-1 regressions: runCtxCum reset on error, usage straggler +// must not open an orphan turn, stale wakeArmed must not mislabel. + +func TestErrorResetsRunCtxCum(t *testing.T) { + m := newTestModel() + busyTurn(m) + // Pre-v2.3 cumulative gauge mid-run. + m.handleEvent(client.Event{Type: "usage", ContextTokens: 1500}) + if m.runCtxCum != 1500 { + t.Fatalf("precondition: runCtxCum = %d", m.runCtxCum) + } + m.handleEvent(client.Event{Type: "error", Message: "boom"}) + if m.runCtxCum != 0 { + t.Fatalf("error path leaked runCtxCum: %d", m.runCtxCum) + } + // Next run's cumulative restarts from its own baseline. + m.handleEvent(client.Event{Type: "session", SessionID: "s2"}) + m.sendPrompt("next") + m.handleEvent(client.Event{Type: "usage", ContextTokens: 1600}) + if m.winCtxTok != 1600 { + t.Fatalf("gauge fill after error+new run = %d, want 1600", m.winCtxTok) + } +} + +func TestUsageStragglerDoesNotOpenOrphanTurn(t *testing.T) { + m := newTestModel() + m.sendPrompt("hi") + m.handleEvent(client.Event{Type: "thinking", Content: "x"}) + m.handleEvent(client.Event{Type: "done"}) // finalize: idle, no open card + n := len(m.msgs) + + // Straggler usage lands after finalize (batch ordering). + m.handleEvent(client.Event{Type: "usage", ContextTokens: 100}) + if len(m.msgs) != n { + t.Fatal("trailing usage opened an orphan turn card") + } + if m.busy { + t.Fatal("trailing usage wedged the model busy") + } +} + +func TestStaleWakeArmedDoesNotSurviveFinalize(t *testing.T) { + m := newTestModel() + m.sendPrompt("long running turn") + m.handleEvent(client.Event{Type: "thinking", Content: "x"}) + // bg_wake lands mid-turn: its wake turn opens later; the arm must not + // outlive this turn's close-out. + m.handleEvent(client.Event{Type: "bg_wake"}) + if !m.wakeArmed { + t.Fatal("precondition: bg_wake mid-turn should arm") + } + m.handleEvent(client.Event{Type: "done"}) + if m.wakeArmed { + t.Fatal("stale wakeArmed survived finalize — next idle-gap turn would be mislabeled as a wake") + } +} diff --git a/internal/tui/stats_test.go b/internal/tui/stats_test.go index 11b1a9c..30dca5b 100644 --- a/internal/tui/stats_test.go +++ b/internal/tui/stats_test.go @@ -686,16 +686,23 @@ func TestUsageAppliesLiveSpeed(t *testing.T) { } } -func TestUsageFirstRemoteTurnResetsThenApplies(t *testing.T) { +func TestUsageWhileIdleAppliesMetricsWithoutCard(t *testing.T) { + // usage is telemetry, not turn evidence: a straggler usage frame after + // finalize (done/usage batch order) must not open an orphan turn card + // or wedge the model busy — the lazy fallback exists for thinking/ + // token frames, which always precede usage in a genuine turn. m := newTestModel() m.tokPerSec = 40 m.tokPerSecKind = client.TokPerSecGeneration m.handleEvent(client.Event{Type: "usage", TokensPerSecond: 9.6}) - if m.cur() < 0 { - t.Fatal("usage-first remote turn must open a card") + if m.cur() >= 0 { + t.Fatal("idle usage must not open a turn card") + } + if m.busy { + t.Fatal("idle usage must not arm busy") } if m.tokPerSec != 9.6 || m.tokPerSecKind != client.TokPerSecE2E { - t.Fatalf("usage-first applied after reset: %v %q", m.tokPerSec, m.tokPerSecKind) + t.Fatalf("usage metrics not applied: %v %q", m.tokPerSec, m.tokPerSecKind) } } diff --git a/internal/tui/wake_turn_test.go b/internal/tui/wake_turn_test.go index 36ecddf..4c33517 100644 --- a/internal/tui/wake_turn_test.go +++ b/internal/tui/wake_turn_test.go @@ -141,12 +141,25 @@ func TestBgJobFrameKicksJobsFetch(t *testing.T) { } } -// handleEvent routes bg_job frames through the kick. +// handleEvent routes bg_job frames through the kick flag; the fetch itself +// rides flushKicks. In batch ingestion the per-event cmds are dropped, so +// the flag must survive until the batch-level flushKicks — a bg_job +// arriving inside a listen-drained batch must still produce exactly one +// fetch. func TestBgJobFrameRoutesThroughKick(t *testing.T) { - m := newJobsTestModel(t, nil) - m.applyJobs(jobsFixture(), nil) - if _, cmd := m.handleEvent(client.Event{Type: "bg_job", SessionID: "s1"}); cmd == nil { - t.Error("bg_job frame produced no cmd") + m, seen := jobsMux(t, `{"jobs":[]}`, nil) + m.applyJobs(jobsFixture(), nil) // watcher live + + // The frame arms the kickJobs flag; the fetch rides the returned + // batch's flushKicks member. A per-event fetch cmd would be dropped by + // ingestWireBatch's per-event loop — the defect this test guards. + _, cmd := m.handleEvent(client.Event{Type: "bg_job", SessionID: "s1"}) + applyQuick(m, cmd) // drive the batch; the fetch is immediate HTTP + if len(*seen) == 0 { + t.Fatal("bg_job kick never reached the jobs endpoint") + } + if m.kickJobs { + t.Error("flushKicks left the kickJobs flag armed") } } diff --git a/internal/update/update.go b/internal/update/update.go index fb165f3..2e6a45c 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -190,12 +190,14 @@ func statusRetryable(err error) bool { // Newer reports whether latest is a higher version than current. Both may // carry a "v" prefix; comparison is numeric over up to 3 dot-separated -// components, with missing components treated as 0. Anything unparsable — -// including a dev build's "dev" or an empty string — reports false, so a -// failed or skipped check never nags. +// components, with missing components treated as 0. A Go pseudo-version +// current (v0.1.3-0.20260901abcdef12-abc1234) compares by its release +// prefix, so a commit-installed build still sees newer releases. Anything +// unparsable — including a dev build's "dev" or an empty string — reports +// false, so a failed or skipped check never nags. func Newer(latest, current string) bool { l, lok := parseSemver(latest) - c, cok := parseSemver(current) + c, cok := parseSemver(pseudoBase(current)) if !lok || !cok { return false } @@ -209,6 +211,18 @@ func Newer(latest, current string) bool { // parseSemver splits an optional-"v"-prefixed version into its numeric // components, zero-padding to 3. Non-numeric components fail the parse. +// pseudoBase reduces a version string to its release core by cutting at +// the first "-": a Go pseudo-version (v0.1.3-0.20260901000000-abc1234, +// a commit-installed build) keeps "v0.1.3", and a genuine semver +// prerelease (v1.2.0-0.1) keeps "v1.2.0" — both compare as their release +// rather than failing the numeric parse entirely. +func pseudoBase(v string) string { + if i := strings.Index(v, "-"); i >= 0 { + return v[:i] + } + return v +} + func parseSemver(v string) ([3]int, bool) { var out [3]int parts := strings.Split(strings.TrimPrefix(strings.TrimSpace(v), "v"), ".") diff --git a/internal/workspace/cross_instance_test.go b/internal/workspace/cross_instance_test.go index c7b9ea6..f1a8e21 100644 --- a/internal/workspace/cross_instance_test.go +++ b/internal/workspace/cross_instance_test.go @@ -1,57 +1,44 @@ package workspace import ( + "os" + "path/filepath" "testing" ) -// Regression: Save/Patch persisted the whole stale in-memory map, so two -// bodek instances in different cwds erased each other's drafts, queues, -// and session ids (last writer wins across EVERY cwd, not just its own). -// Each instance must merge its own cwd into the on-disk state it saw, -// never republish a whole stale snapshot. -func TestSaveKeepsOtherInstancesCwds(t *testing.T) { - path := t.TempDir() + "/workspaces.json" - t.Setenv("BODEK_WORKSPACE", path) +// TestForeignClearNotResurrected guards cross-instance consistency: when +// another bodek instance clears a directory's session (/new), this +// instance's next Save of a DIFFERENT directory must not republish the +// stale pre-clear session id over the fresher disk state. +func TestForeignClearNotResurrected(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "workspaces.json") - // Instance A saves, then instance B opens the file fresh and saves. - instA := Open() - if err := instA.Save("/proj/a", State{Draft: "draft-a", SessionID: "s-a"}); err != nil { - t.Fatalf("Save: %v", err) - } - instB := Open() - if err := instB.Save("/proj/b", State{Draft: "draft-b", SessionID: "s-b"}); err != nil { - t.Fatalf("Save: %v", err) - } - // Instance A saves again — it must not wipe B's cwd. - if err := instA.Save("/proj/a", State{Draft: "draft-a2", SessionID: "s-a"}); err != nil { - t.Fatalf("Save: %v", err) + a := openAt(path) + a.Save("/w1", State{SessionID: "s1", Draft: "d1"}) + + b := openAt(path) // second instance sees s1 on disk + b.ClearSession("/w1") + + // Instance A persists a different cwd; the cleared /w1 must stay cleared. + if err := a.Save("/w2", State{SessionID: "s2"}); err != nil { + t.Fatalf("Save /w2: %v", err) } - fresh := Open() - if got := fresh.Load("/proj/b").Draft; got != "draft-b" { - t.Errorf("instance A's Save wiped instance B's cwd: /proj/b draft = %q, want draft-b", got) + got := openAt(path).Load("/w1") + if got.SessionID != "" { + t.Fatalf("foreign ClearSession resurrected: /w1 session = %q, want empty", got.SessionID) } - if got := fresh.Load("/proj/a").Draft; got != "draft-a2" { - t.Errorf("/proj/a draft = %q, want draft-a2", got) + if got := openAt(path).Load("/w2"); got.SessionID != "s2" { + t.Fatalf("Save /w2 lost: %q", got.SessionID) } } -func TestPatchKeepsOtherInstancesCwds(t *testing.T) { - path := t.TempDir() + "/workspaces.json" - t.Setenv("BODEK_WORKSPACE", path) - - instA := Open() - instA.Save("/proj/a", State{History: []string{"a1"}}) - instB := Open() - instB.Save("/proj/b", State{History: []string{"b1"}}) - - instA.Patch("/proj/a", func(st *State) { st.Draft = "patched" }) - - fresh := Open() - if got := fresh.Load("/proj/b"); len(got.History) != 1 || got.History[0] != "b1" { - t.Errorf("instance A's Patch wiped instance B's cwd /proj/b: %+v", got) - } - if got := fresh.Load("/proj/a"); got.Draft != "patched" { - t.Errorf("/proj/a draft = %q, want patched", got.Draft) +// openAt builds a Store pointed at an explicit path (Open has no path hook). +func openAt(path string) *Store { + s := &Store{path: path, all: map[string]State{}} + if _, err := os.ReadFile(path); err == nil { + s.reloadLocked("") } + return s } diff --git a/internal/workspace/workspace.go b/internal/workspace/workspace.go index 74d1e7e..a5caa32 100644 --- a/internal/workspace/workspace.go +++ b/internal/workspace/workspace.go @@ -71,11 +71,14 @@ func Open() *Store { return s } -// reloadLocked re-reads the on-disk store and merges FOREIGN cwds over the -// in-memory map (our own cwd entries win — the caller is about to overwrite -// one). Another bodek instance may have persisted since Open; republishing -// the stale whole map erased its drafts, queues, and session ids. -func (s *Store) reloadLocked() { +// reloadLocked re-reads the on-disk store and adopts the on-disk state for +// every cwd EXCEPT `except` (the caller is about to overwrite that one — +// its in-memory value is the newest). Another bodek instance may have +// persisted since Open: merging only foreign additions let our stale +// copies of other directories republish dead drafts, queues, and session +// ids over the fresher disk state (e.g. a /new in another instance +// resurrected here). +func (s *Store) reloadLocked(except string) { if s.path == "" { return } @@ -88,7 +91,7 @@ func (s *Store) reloadLocked() { return } for cwd, st := range f.Workspaces { - if _, ours := s.all[cwd]; !ours { + if cwd != except { s.all[cwd] = st } } @@ -110,7 +113,7 @@ func (s *Store) Save(cwd string, st State) error { return nil } s.mu.Lock() - s.reloadLocked() + s.reloadLocked(cwd) s.all[cwd] = cloneState(st) path := s.path snap := cloneAll(s.all) @@ -124,7 +127,7 @@ func (s *Store) Patch(cwd string, fn func(*State)) { return } s.mu.Lock() - s.reloadLocked() + s.reloadLocked(cwd) st := cloneState(s.all[cwd]) fn(&st) s.all[cwd] = st