diff --git a/AGENTS.md b/AGENTS.md index 27b9f30..b9b4938 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -256,6 +256,8 @@ modifier routes to the composer), and `Alt+A`/`Alt+D`/`Alt+T` minted token, and never returns that stub as a session. Replayed user turns show `principal_prompt` (what was typed), not `content`, which carries attachments and @-resources inlined in untrusted wrappers. + `handleSessionDetail` refuses a resume while `busy` — the running turn's + frames would stream into a replayed message at `curIdx`. - The TUI reconnects with backoff and resumes the session after a socket drop (`reconnect.go`) — don't break that by assuming a single connection per run. diff --git a/README.md b/README.md index ffdd58a..101bf45 100644 --- a/README.md +++ b/README.md @@ -256,7 +256,8 @@ own front-end settings are separate; see [Configuration](#configuration). - **Session browser** (`^R`) — resume, replay, delete, pin (`p`), rename (`r`), export a transcript (`e` markdown, `E` JSON), and search server-side (`/`); `n` loads the next page. Resuming sends a `session_switch` so the - server-side memory buffer is restored before you type. + server-side memory buffer is restored before you type. Resume is refused + while a turn is running — finish it or cancel with `^X` first. - **Last-session resume (opt-in)** — by default a launch starts fresh. Enable continuation per launch with `--resume`, or persistently with `{"resume": true}` in `~/.bodek/config.json`. Resuming replays the diff --git a/cmd/bodek/log.go b/cmd/bodek/log.go index 9fbb1c1..d013eb4 100644 --- a/cmd/bodek/log.go +++ b/cmd/bodek/log.go @@ -41,6 +41,9 @@ type ringWriter struct { partial []byte } +// maxPartialBytes caps the unterminated trailing line a ringWriter keeps. +const maxPartialBytes = 64 << 10 + func newRingWriter(maxLine int) *ringWriter { return &ringWriter{maxLine: maxLine} } @@ -60,6 +63,10 @@ func (w *ringWriter) Write(p []byte) (int, error) { w.lines = w.lines[len(w.lines)-w.maxLine:] } } + // Newline-free output (\r progress bars) must not grow without bound. + if n := len(w.partial); n > maxPartialBytes { + w.partial = append([]byte(nil), w.partial[n-maxPartialBytes:]...) + } return len(p), nil } diff --git a/cmd/bodek/log_test.go b/cmd/bodek/log_test.go index 1117b74..aa6d6e0 100644 --- a/cmd/bodek/log_test.go +++ b/cmd/bodek/log_test.go @@ -74,3 +74,14 @@ func TestOpenServerLogTruncates(t *testing.T) { t.Fatal("prior run's log was not truncated") } } + +func TestRingWriterCapsPartialLine(t *testing.T) { + w := newRingWriter(3) + chunk := []byte(strings.Repeat("x", 4096)) + for range 64 { + _, _ = w.Write(chunk) + } + if len(w.partial) > maxPartialBytes { + t.Fatalf("partial = %d bytes, want <= %d", len(w.partial), maxPartialBytes) + } +} diff --git a/cmd/bodek/main.go b/cmd/bodek/main.go index 68d0c81..21cc29c 100644 --- a/cmd/bodek/main.go +++ b/cmd/bodek/main.go @@ -74,7 +74,11 @@ func (c *config) save() error { } cur, err := settings.Load() if err != nil { - cur = c.persisted // file vanished or changed shape: fall back to our baseline + // A missing file loads clean; an error means the file became + // unreadable or unparsable mid-run (a hand-edit typo). Replacing it + // would clobber those edits — the same rule persistDisabled enforces + // at startup. + return fmt.Errorf("settings file could not be read; change not saved: %w", err) } rb, rc, rd := reflect.ValueOf(c.persisted), reflect.ValueOf(c.persist), reflect.ValueOf(&cur).Elem() for i := range rc.NumField() { @@ -370,6 +374,9 @@ func setupSignalHandler(p *tea.Program) { signal.Notify(ch, os.Interrupt, syscall.SIGTERM) go func() { <-ch + // Restore default handling so a second Ctrl+C / SIGTERM can abort a + // slow shutdown instead of being swallowed by this channel. + signal.Stop(ch) p.Quit() }() } diff --git a/cmd/bodek/settings_persist_test.go b/cmd/bodek/settings_persist_test.go index b7a18ce..9fd9f37 100644 --- a/cmd/bodek/settings_persist_test.go +++ b/cmd/bodek/settings_persist_test.go @@ -65,3 +65,25 @@ func TestParseConfigBrokenFileRefusesPersistence(t *testing.T) { t.Fatalf("expected saved settings file: %v", err) } } + +// A file broken mid-run (a hand-edit typo after a clean startup) must not +// be replaced by the next preference change either. +func TestSaveRefusesFileBrokenMidRun(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.json") + t.Setenv("BODEK_CONFIG", path) + cfg, err := parseConfig(nil, &bytes.Buffer{}) + if err != nil || cfg.persistDisabled { + t.Fatalf("parseConfig: %v, disabled=%v", err, cfg.persistDisabled) + } + broken := `{"theme": "dracula",` + if err := os.WriteFile(path, []byte(broken), 0o600); err != nil { + t.Fatal(err) + } + cfg.persist.Theme = "ember-light" + if err := cfg.save(); err == nil { + t.Fatal("save over a broken file must report an error") + } + if data, _ := os.ReadFile(path); string(data) != broken { + t.Fatalf("broken settings file was overwritten: %q", data) + } +} diff --git a/cmd/bodek/upgrade_swap_test.go b/cmd/bodek/upgrade_swap_test.go index dc6c845..5d8b81b 100644 --- a/cmd/bodek/upgrade_swap_test.go +++ b/cmd/bodek/upgrade_swap_test.go @@ -103,12 +103,17 @@ func TestUpgradeSlowLinkDownloads(t *testing.T) { // 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. +// The stamp names the *next* patch (a commit after v0.1.2), so the v0.1.3 +// release itself is newer. 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") + if !update.Newer("v0.1.3", "v0.1.3-0.20260901000000-abc1234") { + t.Fatal("v0.1.3 must be newer than the v0.1.3 pseudo-version that predates it") + } + if update.Newer("v0.1.2", "v0.1.3-0.20260901000000-abc1234") { + t.Fatal("v0.1.2 must not upgrade over a later commit") } // A genuine semver prerelease must not be misparsed as a pseudo-version. if !update.Newer("v1.3.0", "v1.2.0-0.1") { diff --git a/internal/client/client.go b/internal/client/client.go index 10a797b..3dfaed1 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -11,6 +11,7 @@ import ( "crypto/tls" "encoding/json" "fmt" + "io" "net" "net/http" "net/url" @@ -309,7 +310,7 @@ func Dial(wsURL, origin, baseURL, token string) (*Client, error) { conn: conn, baseURL: baseURL, serveToken: token, - http: &http.Client{Timeout: 3 * time.Second}, + http: &http.Client{Timeout: 3 * time.Second, CheckRedirect: noRedirect}, // Thinking models stream hundreds of delta frames per turn. The TUI // batch-drains this channel, but a deep buffer keeps readLoop from // blocking (which trips odek's 30s write watchdog and drops the socket). @@ -318,7 +319,7 @@ func Dial(wsURL, origin, baseURL, token string) (*Client, error) { // exports) legitimately take longer than interactive REST calls; a // dedicated 30s client serves them so the 3s interactive budget does // not cut their body reads. - slowHTTP: &http.Client{Timeout: 30 * time.Second}, + slowHTTP: &http.Client{Timeout: 30 * time.Second, CheckRedirect: noRedirect}, done: make(chan struct{}), } go c.readLoop() @@ -399,7 +400,7 @@ func (c *Client) Resources(query string, limit int) ([]Resource, error) { return nil, fmt.Errorf("resources: status %s", resp.Status) } var out []Resource - if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&out); err != nil { return nil, err } return out, nil @@ -618,13 +619,29 @@ func (c *Client) SendSkillPromptResponse(action, skillName string) error { }{Type: "skill_prompt_response", Action: action, SkillName: skillName}) } +// writeTimeout bounds one frame write. A peer that stops reading would +// otherwise park send forever while holding wmu, wedging every later +// approval and cancel behind it. Generous enough for a 10 MB attachment +// prompt on a slow link. +var writeTimeout = 60 * time.Second + // send marshals v and writes it as one frame, serialized against other writers. func (c *Client) send(v any) error { c.wmu.Lock() defer c.wmu.Unlock() + _ = c.conn.SetWriteDeadline(time.Now().Add(writeTimeout)) return ws.JSON.Send(c.conn, v) } +// noRedirect keeps REST calls on the origin they were addressed to. Go +// strips Authorization on cross-host redirects but copies custom headers, +// so following one would hand X-Odek-Ws-Token / X-Session-Token (and a +// replayed POST body on 307/308) to another host. odek's API never +// redirects; the 3xx reaches the caller as a status error. +func noRedirect(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse +} + // Close shuts the connection and releases a readLoop parked on Events. func (c *Client) Close() error { c.closeOnce.Do(func() { diff --git a/internal/client/jobs.go b/internal/client/jobs.go index 2864c7e..a474fb2 100644 --- a/internal/client/jobs.go +++ b/internal/client/jobs.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/url" "time" @@ -63,7 +64,7 @@ func (c *Client) Jobs(sessionID, token string) ([]Job, error) { return nil, fmt.Errorf("jobs: status %s", resp.Status) } var env jobsEnvelope - if err := json.NewDecoder(resp.Body).Decode(&env); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&env); err != nil { return nil, err } return env.Jobs, nil @@ -86,7 +87,7 @@ func (c *Client) JobOutput(sessionID, token, jobID string, since int) (JobOutput case resp.StatusCode != http.StatusOK: return out, fmt.Errorf("job output: status %s", resp.Status) } - if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&out); err != nil { return out, err } return out, nil @@ -97,7 +98,7 @@ func (c *Client) JobOutput(sessionID, token, jobID string, since int) (JobOutput // bgproc.Manager.Stop), which the shared 3s client would abort mid-kill. func (c *Client) stopHTTP() *http.Client { c.stopOnce.Do(func() { - c.stopClient = &http.Client{Timeout: 12 * time.Second} + c.stopClient = &http.Client{Timeout: 12 * time.Second, CheckRedirect: noRedirect} }) return c.stopClient } @@ -127,7 +128,7 @@ func (c *Client) StopJob(sessionID, token, jobID string) (string, error) { var body struct { Status string `json:"status"` } - if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&body); err != nil { return "", err } return body.Status, nil diff --git a/internal/client/plan.go b/internal/client/plan.go index e1ef858..59ed4ad 100644 --- a/internal/client/plan.go +++ b/internal/client/plan.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/url" ) @@ -65,7 +66,7 @@ func (c *Client) SessionPlan(sessionID, sessionToken string) (PlanSnapshot, erro return PlanSnapshot{}, fmt.Errorf("session plan: status %s", resp.Status) } var out PlanSnapshot - if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&out); err != nil { return PlanSnapshot{}, fmt.Errorf("session plan: %w", err) } return out, nil diff --git a/internal/client/redirect_test.go b/internal/client/redirect_test.go new file mode 100644 index 0000000..c991163 --- /dev/null +++ b/internal/client/redirect_test.go @@ -0,0 +1,52 @@ +package client + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + ws "golang.org/x/net/websocket" +) + +// REST calls must never follow a redirect: Go copies the custom token +// headers onto a cross-host hop (and replays POST bodies on 307/308). +func TestStopJobDoesNotFollowRedirect(t *testing.T) { + var leaked atomic.Bool + evil := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-Odek-Ws-Token") != "" || r.Header.Get("X-Session-Token") != "" { + leaked.Store(true) + } + _, _ = w.Write([]byte(`{"status":"stopped"}`)) + })) + defer evil.Close() + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, evil.URL+r.URL.Path, http.StatusTemporaryRedirect) + })) + defer origin.Close() + + c := &Client{baseURL: origin.URL, serveToken: "instance-secret"} + if _, err := c.StopJob("s1", "session-secret", "j1"); err == nil { + t.Fatal("StopJob followed a redirect; want a status error") + } + if leaked.Load() { + t.Fatal("auth headers reached the redirect target") + } +} + +// The REST clients Dial builds (interactive and slow) refuse redirects too. +func TestDialClientsDoNotFollowRedirects(t *testing.T) { + base := validateServer(t, func(c *ws.Conn) { time.Sleep(2 * time.Second) }, nil) + c, err := Dial("ws"+strings.TrimPrefix(base, "http")+"/ws", base, base, "tok") + if err != nil { + t.Fatal(err) + } + defer func() { _ = c.Close() }() + for name, hc := range map[string]*http.Client{"http": c.http, "slowHTTP": c.slowHTTP} { + if hc.CheckRedirect == nil || hc.CheckRedirect(nil, nil) != http.ErrUseLastResponse { + t.Errorf("%s follows redirects", name) + } + } +} diff --git a/internal/client/rest.go b/internal/client/rest.go index 48c980b..f3e88e4 100644 --- a/internal/client/rest.go +++ b/internal/client/rest.go @@ -215,7 +215,7 @@ func (c *Client) sessionDetailOnce(id, token string) (s Session, eff string, boo Session Bootstrapped bool `json:"bootstrapped"` } - if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&body); err != nil { return s, "", false, err } eff = resp.Header.Get("X-Session-Token") diff --git a/internal/client/runs.go b/internal/client/runs.go index 5e4475d..d49f7da 100644 --- a/internal/client/runs.go +++ b/internal/client/runs.go @@ -3,6 +3,7 @@ package client import ( "encoding/json" "fmt" + "io" "net/http" "net/url" "time" @@ -108,7 +109,7 @@ func (c *Client) StartRun(content string, opts RunOpts) (Run, error) { SessionID string `json:"session_id"` Status string `json:"status"` } - if err := json.NewDecoder(resp.Body).Decode(&started); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxJSONBytes)).Decode(&started); err != nil { return Run{}, err } return Run{ID: started.RunID, SessionID: started.SessionID, Status: started.Status}, nil diff --git a/internal/server/pgroup_unix.go b/internal/server/pgroup_unix.go index 62bb758..b989c3d 100644 --- a/internal/server/pgroup_unix.go +++ b/internal/server/pgroup_unix.go @@ -19,13 +19,15 @@ func setPgroup(cmd *exec.Cmd) { // signalServer sends sig to the spawned server's whole process group (the // server's own subprocesses follow it down), falling back to the leader -// when the group is gone. Safe because Stop holds a live Process handle -// for this pid — no recycled-PID window. +// when the group is gone. The leader fallback goes through Process.Signal, +// which reports ErrProcessDone as soon as wait4 reaped the child — the raw +// pid may already belong to another process, and the reaper's flag lags +// that moment by up to WaitDelay while stderr drains. func (c *Conn) signalServer(sig syscall.Signal) { if c.proc == nil || c.proc.Process == nil { return } - if err := syscall.Kill(-c.proc.Process.Pid, sig); err != nil { - _ = syscall.Kill(c.proc.Process.Pid, sig) + if err := syscall.Kill(-c.proc.Process.Pid, sig); err != nil && !c.reaped.Load() { + _ = c.proc.Process.Signal(sig) } } diff --git a/internal/server/scan_bounds_test.go b/internal/server/scan_bounds_test.go new file mode 100644 index 0000000..d881abf --- /dev/null +++ b/internal/server/scan_bounds_test.go @@ -0,0 +1,39 @@ +package server + +import ( + "errors" + "strings" + "testing" +) + +type failWriter struct{} + +func (failWriter) Write([]byte) (int, error) { return 0, errors.New("disk full") } + +// A failing log sink must not surface to os/exec, whose copy goroutine +// would close the stderr pipe and kill odek with SIGPIPE. +func TestTokenScanWriterSwallowsSinkErrors(t *testing.T) { + s := &tokenScanWriter{w: failWriter{}} + n, err := s.Write([]byte("WS token: abc\n")) + if err != nil || n != 15 { + t.Fatalf("Write = %d, %v; want 15, nil", n, err) + } + if s.Token() != "abc" { + t.Fatalf("token = %q, want abc", s.Token()) + } +} + +// Newline-free output (\r progress bars) must not grow the buffer unbounded, +// before or after the token is found. +func TestTokenScanWriterCapsPartialLine(t *testing.T) { + chunk := []byte(strings.Repeat("x", 4096)) + for _, tok := range []string{"", "found"} { + s := &tokenScanWriter{w: failWriter{}, tok: tok} + for range 64 { + _, _ = s.Write(chunk) + } + if len(s.buf) > maxPartialLine { + t.Fatalf("tok=%q: partial buffer %d bytes, want <= %d", tok, len(s.buf), maxPartialLine) + } + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 1068c70..7c83c73 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -215,6 +215,9 @@ func (c *Conn) spawn(opts Options, addr string) error { cmd := exec.Command(bin, args...) cmd.Stderr = c.scan + // A grandchild that leaves the group but keeps fd 2 open would hold the + // stderr pipe forever, and Wait (hence Stop) with it. + cmd.WaitDelay = stderrWaitDelay cmd.Env = os.Environ() // Own process group so the watchdog (and Stop) can signal the server // and any of its subprocesses as a unit, without touching bodek itself. @@ -297,13 +300,18 @@ func (c *Conn) Stop() { if c.reaped.Load() { return } - // Graceful shutdown owns the exit — retire the orphan watchdog first. - c.watchMu.Lock() - if c.watch != nil { - c.watch() - c.watch = nil - } - c.watchMu.Unlock() + // Retire the orphan watchdog only once the child is gone: if bodek + // itself dies mid-Stop (a second Ctrl+C restores the default signal + // action), the watchdog must still be there to escalate. It stays + // inert while this process is alive. + defer func() { + c.watchMu.Lock() + if c.watch != nil { + c.watch() + c.watch = nil + } + c.watchMu.Unlock() + }() // SIGINT triggers odek serve's graceful shutdown (closes sockets, removes // sandbox containers), delivered to the server's whole process group so // its own subprocesses follow. SIGKILL escalation likewise targets the @@ -424,12 +432,23 @@ type tokenScanWriter struct { // maxTailLines bounds the stderr tail kept for error reporting. const maxTailLines = 4 +// maxPartialLine caps an unterminated stderr line buffered for scanning, so +// newline-free output (\r progress bars) cannot grow memory without bound. +const maxPartialLine = 64 << 10 + +// stderrWaitDelay bounds how long Wait keeps draining stderr after the +// child exits. +const stderrWaitDelay = 2 * time.Second + func (s *tokenScanWriter) Write(p []byte) (int, error) { s.scan(p) if s.act != nil { s.act.Store(time.Now().UnixNano()) } - return s.w.Write(p) + // A failing sink (full log disk) must never surface to os/exec: its + // copy goroutine would close the pipe and odek would die on SIGPIPE. + _, _ = s.w.Write(p) + return len(p), nil } // Token returns the scanned token, or "" if no token line was seen yet. @@ -462,6 +481,7 @@ func (s *tokenScanWriter) scan(p []byte) { for { i := bytes.IndexByte(s.buf, '\n') if i < 0 { + s.capPartial() return } line := string(s.buf[:i]) @@ -510,6 +530,15 @@ func (s *tokenScanWriter) partialTail(chunk []byte) { s.buf = s.buf[i+1:] s.appendTailLine(line) } + s.capPartial() +} + +// capPartial keeps only the newest maxPartialLine bytes of the pending +// partial line. Callers hold s.mu. +func (s *tokenScanWriter) capPartial() { + if n := len(s.buf); n > maxPartialLine { + s.buf = append([]byte(nil), s.buf[n-maxPartialLine:]...) + } } func (s *tokenScanWriter) appendTailLine(line string) { diff --git a/internal/server/stop_event_test.go b/internal/server/stop_event_test.go index 1071907..51368fc 100644 --- a/internal/server/stop_event_test.go +++ b/internal/server/stop_event_test.go @@ -281,3 +281,19 @@ func TestStopProgressEvents(t *testing.T) { t.Errorf("progress not monotonic: %v then %v", prev.Elapsed, last.Elapsed) } } + +// The orphan watchdog must stay armed until the child is gone: a second +// Ctrl+C kills bodek mid-Stop, and only the watchdog can then escalate. +func TestStopRetiresWatchdogOnlyAfterChildExits(t *testing.T) { + oldStop, oldGrace := stopTimeout, activityGrace + stopTimeout, activityGrace = 10*time.Second, 300*time.Millisecond + defer func() { stopTimeout, activityGrace = oldStop, oldGrace }() + var events []StopEvent + c := newFixtureConn(fakeIgnoreINTScript(t), func(e StopEvent) { events = append(events, e) }, nil) + var atRetire []StopEvent + c.watch = func() { atRetire = append([]StopEvent(nil), events...) } + c.Stop() + if len(atRetire) == 0 || atRetire[len(atRetire)-1] != StopEscalated { + t.Fatalf("watchdog retired at events %v; want after the child was killed", atRetire) + } +} diff --git a/internal/settings/settings.go b/internal/settings/settings.go index fc0f581..3ae97d7 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -70,7 +70,8 @@ func Save(s Settings) error { if path == "" { return os.ErrNotExist } - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + // 0700 like the token and workspace stores that share ~/.bodek. + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return err } data, err := json.MarshalIndent(s, "", " ") diff --git a/internal/tokens/tokens.go b/internal/tokens/tokens.go index 518238e..c4cf401 100644 --- a/internal/tokens/tokens.go +++ b/internal/tokens/tokens.go @@ -24,6 +24,10 @@ type Store struct { mu sync.Mutex m map[string]string path string + // pending holds this instance's mutations not yet on disk (token, or "" + // for a delete). mergeLocked replays them so a failed persist cannot be + // erased by the next successful one adopting the older disk map. + pending map[string]string } // Open loads the token store from ~/.bodek/sessions.json. It never fails: @@ -90,6 +94,7 @@ func (s *Store) Set(id, token string) { } s.mergeLocked() s.m[id] = token + s.markPendingLocked(id, token) s.persistLocked() } @@ -105,6 +110,7 @@ func (s *Store) Delete(id string) { } s.mergeLocked() delete(s.m, id) + s.markPendingLocked(id, "") s.persistLocked() } @@ -128,9 +134,24 @@ func (s *Store) mergeLocked() { if disk == nil { disk = map[string]string{} } + for id, tok := range s.pending { + if tok == "" { + delete(disk, id) + } else { + disk[id] = tok + } + } s.m = disk } +// markPendingLocked records a mutation until a persist lands it on disk. +func (s *Store) markPendingLocked(id, token string) { + if s.pending == nil { + s.pending = map[string]string{} + } + s.pending[id] = token +} + // persistLocked writes the store while the mutex is held. Snapshot-then- // persist-outside-the-lock let interleaved Set/Delete writes reorder on // disk: an older snapshot landing last resurrected deleted tokens and @@ -142,7 +163,9 @@ func (s *Store) persistLocked() { } if err := persist(s.path, snapshot); err != nil { warnPersist(err) + return } + s.pending = nil } // persist atomically writes the store (staged .tmp + rename) so a crash diff --git a/internal/tokens/tokens_merge_test.go b/internal/tokens/tokens_merge_test.go index be3b3a0..96afc79 100644 --- a/internal/tokens/tokens_merge_test.go +++ b/internal/tokens/tokens_merge_test.go @@ -76,3 +76,31 @@ func TestQuarantineRotatesBackups(t *testing.T) { t.Fatalf("first quarantine was clobbered: %q err=%v", got, err) } } + +// Regression: a Set whose persist failed lived only in memory, and the next +// successful mutation adopted the older disk map — erasing it for good. +func TestFailedPersistSurvivesNextMerge(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores directory permissions") + } + dir := t.TempDir() + path := filepath.Join(dir, "sessions.json") + s := openAt(path) + s.Set("sess-a", "tok-a") + + if err := os.Chmod(dir, 0o500); err != nil { // staging the tmp file fails + t.Fatal(err) + } + s.Set("sess-x", "tok-x") + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatal(err) + } + s.Set("sess-y", "tok-y") + + c := openAt(path) + for id, want := range map[string]string{"sess-a": "tok-a", "sess-x": "tok-x", "sess-y": "tok-y"} { + if got := c.Get(id); got != want { + t.Fatalf("Get(%s) = %q, want %q", id, got, want) + } + } +} diff --git a/internal/tui/approval_expiry.go b/internal/tui/approval_expiry.go index 4e4fd84..4146078 100644 --- a/internal/tui/approval_expiry.go +++ b/internal/tui/approval_expiry.go @@ -57,11 +57,14 @@ func (m *Model) apprSecondsLeft() int { } // approvalSweep arms the next expiry tick while the form is open; nil when -// nothing is pending, so the timer chain stops itself. +// nothing is pending, so the timer chain stops itself. Every wire batch +// calls it, so it arms only when no tick is already in flight — otherwise +// each batch would start another 1s chain that re-arms itself forever. func (m *Model) approvalSweep() tea.Cmd { - if len(m.approvals) == 0 { + if len(m.approvals) == 0 || m.apprSweeping { return nil } + m.apprSweeping = true return tea.Tick(time.Second, func(time.Time) tea.Msg { return approvalExpireMsg{} }) diff --git a/internal/tui/attach.go b/internal/tui/attach.go index c7623f5..4c05a64 100644 --- a/internal/tui/attach.go +++ b/internal/tui/attach.go @@ -3,9 +3,11 @@ package tui import ( "bytes" "fmt" + "io" "os" "path/filepath" "strings" + "syscall" tea "github.com/charmbracelet/bubbletea" @@ -47,6 +49,11 @@ func (m *Model) attachFile(path string) tea.Cmd { if p, err := filepath.Abs(expandTilde(path)); err == nil { path = p } + // Stage, name and persist the real file: the persisted path then lets a + // later restore detect a symlink planted anywhere along it. + if p, err := filepath.EvalSymlinks(path); err == nil { + path = p + } info, err := os.Stat(path) if err != nil { return m.transientNoteCmd("attach: " + err.Error()) @@ -54,6 +61,11 @@ func (m *Model) attachFile(path string) tea.Cmd { if info.IsDir() { return m.transientNoteCmd("attach: " + path + " is a directory") } + if !info.Mode().IsRegular() { + // A FIFO would block the UI goroutine forever; /dev/zero reports + // size 0 and would read without bound. + return m.transientNoteCmd("attach: " + filepath.Base(path) + " is not a regular file") + } if info.Size() > maxAttachBytes { return m.transientNoteCmd(fmt.Sprintf("attach: %s is %s — over the 5 MB per-file cap (use an @reference)", filepath.Base(path), human(int(info.Size())))) @@ -65,7 +77,7 @@ func (m *Model) attachFile(path string) tea.Cmd { if total+int(info.Size()) > maxAttachTotalBytes { return m.transientNoteCmd("attach: staged files would exceed the 10 MB prompt cap") } - data, err := os.ReadFile(path) + data, err := readCapped(path, maxAttachBytes) if err != nil { return m.transientNoteCmd("attach: " + err.Error()) } @@ -128,3 +140,43 @@ func expandTilde(p string) string { } return p } + +// readCapped reads at most limit bytes of path, failing if the file grew +// past it after the size check. +func readCapped(path string, limit int64) ([]byte, error) { + // O_NONBLOCK: a path swapped for a FIFO after the caller's Stat must + // not block the open; the descriptor is then re-checked, so the bytes + // read always come from the regular file that was vetted. + f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) + if err != nil { + return nil, err + } + defer func() { _ = f.Close() }() + if info, err := f.Stat(); err != nil { + return nil, err + } else if !info.Mode().IsRegular() { + return nil, fmt.Errorf("%s is not a regular file", filepath.Base(path)) + } + data, err := io.ReadAll(io.LimitReader(f, limit+1)) + if err != nil { + return nil, err + } + if int64(len(data)) > limit { + return nil, fmt.Errorf("%s grew past the 5 MB per-file cap", filepath.Base(path)) + } + return data, nil +} + +// restoreAttachment re-stages a path persisted from an earlier run. Unlike +// an explicit /attach, nobody chose it this launch, and attachFile persists +// fully resolved paths: if it no longer resolves to itself, a symlink was +// planted somewhere along it (an agent with workspace write access can, and +// inside a bind-mounted sandbox it resolves on the host). It is skipped, so +// a host secret is never sent with the next prompt. +func (m *Model) restoreAttachment(path string) tea.Cmd { + p := expandTilde(path) + if real, err := filepath.EvalSymlinks(p); err == nil && real != filepath.Clean(p) { + return m.transientNoteCmd("attach: " + filepath.Base(p) + " now resolves elsewhere — not re-attached") + } + return m.attachFile(p) +} diff --git a/internal/tui/attach_fifo_unix_test.go b/internal/tui/attach_fifo_unix_test.go new file mode 100644 index 0000000..fb085a0 --- /dev/null +++ b/internal/tui/attach_fifo_unix_test.go @@ -0,0 +1,91 @@ +//go:build unix + +package tui + +import ( + "os" + "path/filepath" + "syscall" + "testing" +) + +// A FIFO would block the UI goroutine forever on read. +func TestAttachRejectsNonRegularFile(t *testing.T) { + fifo := filepath.Join(t.TempDir(), "pipe") + if err := syscall.Mkfifo(fifo, 0o600); err != nil { + t.Skip("mkfifo unsupported:", err) + } + m := newTestModel() + _ = m.attachFile(fifo) + if len(m.attachments) != 0 { + t.Fatal("a FIFO was attached") + } +} + +// A persisted attachment that became a symlink (planted via the workspace) +// is not silently re-read at startup. +func TestRestoreAttachmentSkipsSymlink(t *testing.T) { + dir := t.TempDir() + secret := filepath.Join(dir, "secret") + if err := os.WriteFile(secret, []byte("key"), 0o600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "notes.txt") + if err := os.Symlink(secret, link); err != nil { + t.Fatal(err) + } + m := newTestModel() + _ = m.restoreAttachment(link) + if len(m.attachments) != 0 { + t.Fatal("a symlinked persisted attachment was re-staged") + } + _ = m.attachFile(link) // an explicit /attach still follows the operator's choice + if len(m.attachments) != 1 { + t.Fatal("explicit attach of a symlink to a regular file should still work") + } +} + +// The descriptor itself is vetted: a non-regular file never reaches the read. +func TestReadCappedRejectsFIFO(t *testing.T) { + fifo := filepath.Join(t.TempDir(), "p") + if err := syscall.Mkfifo(fifo, 0o600); err != nil { + t.Skip("mkfifo unsupported:", err) + } + if _, err := readCapped(fifo, 10); err == nil { + t.Fatal("readCapped read a FIFO") + } +} + +// A symlink planted on a parent directory of a persisted path is caught too. +func TestRestoreAttachmentSkipsSymlinkedParent(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + docs := filepath.Join(root, "docs") + if err := os.Mkdir(docs, 0o700); err != nil { + t.Fatal(err) + } + persisted := filepath.Join(docs, "config") + if err := os.WriteFile(persisted, []byte("ok"), 0o600); err != nil { + t.Fatal(err) + } + secrets := filepath.Join(root, "secrets") + if err := os.Mkdir(secrets, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(secrets, "config"), []byte("key"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(docs); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secrets, docs); err != nil { + t.Fatal(err) + } + m := newTestModel() + _ = m.restoreAttachment(persisted) + if len(m.attachments) != 0 { + t.Fatal("a persisted path behind a planted directory symlink was re-staged") + } +} diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go new file mode 100644 index 0000000..abf1253 --- /dev/null +++ b/internal/tui/bughunt_audit_test.go @@ -0,0 +1,412 @@ +package tui + +import ( + "path/filepath" + "strings" + "testing" + "unicode/utf8" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/BackendStack21/bodek/internal/client" +) + +// Lone 8-bit CSI/OSC bytes (invalid UTF-8) and invisible tag characters +// must not slip through sanitize's fast path. +func TestSanitizeInvalidUTF8AndInvisibles(t *testing.T) { + for _, in := range []string{"ok\x9b2J", "ok\x9d0;title\x07", "a\U000E0041b", "a\u061cb", "a\u2062b", "a\ufffab"} { + out := sanitize(in) + // ContainsAny would decode a raw 0x9b as U+FFFD and match the + // (correct) replacement, so check bytes and runes separately. + if !utf8.ValidString(out) || strings.IndexByte(out, 0x9b) >= 0 || strings.IndexByte(out, 0x9d) >= 0 || + strings.ContainsAny(out, "\u061c\u2062\ufffa\U000E0041\x07") { + t.Errorf("sanitize(%q) = %q, still carries a control/invisible", in, out) + } + } + if got := sanitize("plain text"); got != "plain text" { + t.Errorf("readable text changed: %q", got) + } +} + +// Wire ids reach the terminal and filenames through shortID. +func TestShortIDAndExportNameAreSafe(t *testing.T) { + if got := shortID("\x1b[2J\x1b[Hsess"); strings.Contains(got, "\x1b") { + t.Fatalf("shortID kept an escape: %q", got) + } + if got := shortID(strings.Repeat("é", 20)); !strings.HasSuffix(got, "…") || strings.ContainsRune(got, '�') { + t.Fatalf("shortID split a rune: %q", got) + } + dir := t.TempDir() + path, err := writeExport(dir, "/../../../../tmp/x", "md", []byte("x")) + if err != nil { + t.Fatal(err) + } + if filepath.Dir(path) != dir { + t.Fatalf("export escaped its directory: %s", path) + } +} + +// A sub-agent envelope's status is wire text like every other card field. +func TestAgentResultStatusSanitized(t *testing.T) { + r := parseAgentResult(`{"status":"done\u001b]52;c;Zm9v\u0007","summary":"x"}`) + if r == nil { + t.Fatal("envelope not parsed") + } + if strings.ContainsAny(r.status, "\x1b\x07") { + t.Fatalf("status carries escapes: %q", r.status) + } +} + +// The panel status line may quote raw server bodies. +func TestPanelMsgRenderedSanitized(t *testing.T) { + m := newTestModel() + m.runCommandLine("/queue") + m.panelMsg = "promote failed: \x1b]52;c;Zm9v\x07" + if v := m.View(); strings.Contains(v, "]52;") && strings.Contains(v, "\x1b]52") { + t.Fatal("panel message rendered a raw OSC sequence") + } +} + +// Draining the head under an open /queue panel keeps the highlight on the +// same prompt and disarms a pending delete. +func TestQueueDrainDisarmsDeleteAndKeepsSelection(t *testing.T) { + m := newTestModel() + m.busy = true + m.queue = []string{"A", "B", "C"} + m.runCommandLine("/queue") + m.Update(key("down")) + m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("d")}) + if m.confirm != confirmQueueDelete { + t.Fatalf("delete not armed: %v", m.confirm) + } + m.busy = false + _ = m.sendQueued() + if m.confirm != confirmNone { + t.Fatal("drain left the queue delete armed") + } + if m.queue[m.panelSel] != "B" { + t.Fatalf("selection moved to %q, want B", m.queue[m.panelSel]) + } +} + +// A runs poll that reorders rows must not slide approve/cancel onto +// another run. +func TestRunsPollAnchorsSelectionByID(t *testing.T) { + m := newTestModel() + m.panel = panelRuns + m.runs = []client.Run{{ID: "r1"}, {ID: "r2"}} + m.panelSel = 1 + m.handleRunsMsg(runsMsg{runs: []client.Run{{ID: "r0"}, {ID: "r1"}, {ID: "r2"}}}) + if r := m.selectedRun(); r == nil || r.ID != "r2" { + t.Fatalf("selection = %+v, want r2", r) + } +} + +// Resuming mid-turn would stream the old run into a replayed message. +func TestResumeRefusedWhileBusy(t *testing.T) { + m := newTestModel() + m.msgs = []message{{role: roleUser, content: "q"}, {role: roleAsst, streaming: true}} + m.curIdx = 1 + m.busy = true + m.sessionID = "old" + m.handleSessionDetail(sessionDetailMsg{sess: client.Session{ID: "new"}, token: "t"}) + if m.sessionID != "old" || len(m.msgs) != 2 || m.curIdx != 1 { + t.Fatalf("resume swapped the transcript under a running turn: sid=%s msgs=%d cur=%d", m.sessionID, len(m.msgs), m.curIdx) + } +} + +// The refusal must replace the sessions panel's "loading session…" line. +func TestResumeRefusalClearsLoadingLine(t *testing.T) { + m := newTestModel() + m.busy = true + m.panel = panelSessions + m.panelMsg = "loading session…" + m.handleSessionDetail(sessionDetailMsg{sess: client.Session{ID: "new"}, token: "t"}) + if strings.Contains(m.panelMsg, "loading") { + t.Fatalf("panel still says %q after the refusal", m.panelMsg) + } +} + +// Reopening the runs tab must not anchor the first poll on the old list. +func TestRunsReopenDropsStaleList(t *testing.T) { + m := newTestModel() + m.runs = []client.Run{{ID: "old"}} + _ = m.openRuns() + if m.selectedRun() != nil { + t.Fatal("a stale run is actionable before the first poll lands") + } +} + +// Every wire batch calls approvalSweep: only one tick chain may exist. +func TestApprovalSweepSingleChain(t *testing.T) { + m := newTestModel() + m.approvals = []client.Event{{ID: "a1"}} + if m.approvalSweep() == nil { + t.Fatal("first sweep must arm") + } + if m.approvalSweep() != nil { + t.Fatal("second sweep armed a parallel chain") + } + _, cmd := m.Update(approvalExpireMsg{}) + if cmd == nil { + t.Fatal("landed tick must re-arm while an approval is pending") + } + if !m.apprSweeping { + t.Fatal("re-armed chain not recorded") + } +} + +// A jobs snapshot fetched for the previous session must not baseline the +// current one. +func TestJobsSnapshotFromOldSessionDropped(t *testing.T) { + m := newTestModel() + m.sessionID = "new" + m.Update(jobsFetchedMsg{sid: "old", jobs: []client.Job{{ID: "j-old"}}}) + if len(m.jobs) != 0 { + t.Fatalf("stale jobs adopted: %+v", m.jobs) + } +} + +// A late plan 404 for a session we left must not disable the plan surface. +func TestPlanErrorFromOldSessionIgnored(t *testing.T) { + m := newTestModel() + m.sessionID = "new" + m.handlePlanMsg(planMsg{want: "old", err: client.ErrPlanUnavailable}) + if m.planAvail == planUnavailable { + t.Fatal("old session's 404 disabled the plan surface") + } +} + +// A memory refresh that drops the confirmed fact must disarm the gate. +func TestFactDeleteDisarmsWhenRowVanishes(t *testing.T) { + m := newTestModel() + m.panel = panelMemory + m.memRows = []memRow{{kind: "user", text: "a"}, {kind: "user", text: "b"}} + m.panelSel = 1 + m.armConfirm(confirmFactDelete, "user fact") + var mem client.MemoryView + mem.Facts = map[string][]string{"user": {"a", "c"}} + m.handleMgmtMsg(mgmtMsg{tab: panelMemory, mem: mem}) + if m.confirm != confirmNone { + t.Fatal("delete still armed against a neighbouring fact") + } +} + +// Late session rows re-rank the palette; Enter must keep its target. +func TestPaletteSelectionSurvivesLateSessions(t *testing.T) { + m := newTestModel() + m.pal = palState{open: true, mode: palModeCommands, query: "ca"} + m.pal.all = []palEntry{{title: "xc a run", kind: "action"}} + m.filterPalette() + m.pal.sel = 0 + late := client.Session{ID: "s1", Task: "ca"} + a, _ := fuzzyScore("ca", "resume · ca") + if b, _ := fuzzyScore("ca", "xc a run"); a <= b { + t.Fatal("fixture must rank the late session above the selected entry") + } + m.handlePalSessions(palSessionsMsg{items: []client.Session{late}}) + if got := m.pal.items[m.pal.sel].title; got != "xc a run" { + t.Fatalf("highlight moved to %q", got) + } +} + +// Invocation arguments must show '<' rather than <. +func TestInvocationArgsNotHTMLEscaped(t *testing.T) { + out, err := indentJSON(map[string]any{"q": "a&c"}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "a&c") { + t.Fatalf("arguments HTML-escaped: %s", out) + } +} + +// After disarming a gate, Alt chords and key sentinels must not be typed +// into the composer. +func TestConfirmDisarmDoesNotTypeChordsOrSentinels(t *testing.T) { + m := newTestModel() + for _, k := range []tea.KeyMsg{ + {Type: tea.KeyRunes, Runes: []rune("d"), Alt: true}, + {Type: tea.KeyRunes, Runes: []rune("shift+enter")}, + } { + m.armConfirm(confirmClear, "the conversation") + m.Update(k) + if v := m.ta.Value(); v != "" { + t.Fatalf("key %q typed %q into the composer", k.String(), v) + } + } + m.armConfirm(confirmClear, "the conversation") + m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("h")}) + if m.ta.Value() != "h" { + t.Fatalf("plain typing after disarm lost: %q", m.ta.Value()) + } +} + +// shift+delete's sentinel text must never land in the approval composer. +func TestShiftDeleteSentinelNoOpOnApproval(t *testing.T) { + m := newTestModel() + m.approvals = []client.Event{{ID: "a1"}} + m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("shift+delete")}) + if strings.Contains(m.ta.Value(), "shift") || strings.Contains(m.apprTyped, "shift") { + t.Fatal("sentinel text spliced into the approval surface") + } +} + +// A late list or search page that re-ranks rows under an armed session +// delete must disarm it — y would delete a session never confirmed. +func TestSessionDeleteDisarmsOnReorderedPage(t *testing.T) { + m := newTestModel() + m.panel = panelSessions + m.sessions = []client.Session{{ID: "x"}, {ID: "y"}} + m.panelSel = 0 + m.armConfirm(confirmSessionDelete, "session x") + m.handleSessionsPageMsg(sessionsPageMsg{page: client.SessionsPage{Sessions: []client.Session{{ID: "y"}, {ID: "x"}}}}) + if m.confirm != confirmNone { + t.Fatal("delete still armed after the list re-ranked") + } + if _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("y")}); cmd != nil { + t.Fatal("y fired a command after the target moved") + } + + m.sessions = []client.Session{{ID: "x"}, {ID: "y"}} + m.panelSel = 1 + m.armConfirm(confirmSessionDelete, "session y") + m.handleSessionsMsg(sessionsMsg{items: []client.Session{{ID: "x"}, {ID: "y"}}}) // selection resets to x + if m.confirm != confirmNone { + t.Fatal("initial list reply left the delete armed on a moved target") + } +} + +// An unchanged target keeps the gate (and its prompt). +func TestSessionDeleteStaysArmedWhenTargetUnchanged(t *testing.T) { + m := newTestModel() + m.panel = panelSessions + m.sessions = []client.Session{{ID: "x"}} + m.armConfirm(confirmSessionDelete, "session x") + m.handleSessionsPageMsg(sessionsPageMsg{page: client.SessionsPage{Sessions: []client.Session{{ID: "x"}}}}) + if m.confirm != confirmSessionDelete || m.panelMsg == "" { + t.Fatalf("gate dropped for an unchanged target: confirm=%v msg=%q", m.confirm, m.panelMsg) + } +} + +// A delete reply removing the row a second gate was re-armed on must not +// leave that gate pointing at the next session. +func TestSessionDeletedDisarmsGateOnRemovedRow(t *testing.T) { + m := newTestModel() + m.panel = panelSessions + m.sessions = []client.Session{{ID: "a"}, {ID: "b"}} + m.armConfirm(confirmSessionDelete, "session a") + m.handleSessionDeleted(sessionDeletedMsg{id: "a"}) + if m.confirm != confirmNone { + t.Fatal("gate slid onto the neighbouring session") + } +} + +// The same fact text under user and env: an armed delete on the env row +// must not slide onto the user row after a refresh. +func TestFactDeleteKeepsKindAcrossDuplicateText(t *testing.T) { + m := newTestModel() + m.panel = panelMemory + m.memRows = []memRow{{kind: "user", text: "x"}, {kind: "env", text: "x"}} + m.panelSel = 1 + m.armConfirm(confirmFactDelete, "env fact") + var mem client.MemoryView + mem.Facts = map[string][]string{"user": {"x"}, "env": {"x"}} + m.handleMgmtMsg(mgmtMsg{tab: panelMemory, mem: mem}) + if m.confirm == confirmFactDelete && m.memRows[m.panelSel].kind != "env" { + t.Fatalf("armed delete now targets the %s row", m.memRows[m.panelSel].kind) + } + if m.confirm != confirmFactDelete { + t.Fatal("unchanged target should stay armed") + } +} + +// The startup resume must land before any queued or new prompt sends. +func TestStartupResumeHoldsQueue(t *testing.T) { + m := newTestModel() + m.queue = []string{"held"} + m.resumeInFlight = true + if m.sendQueued() != nil || len(m.queue) != 1 { + t.Fatal("queue drained before the startup resume landed") + } + m.ta.SetValue("typed now") + m.Update(key("enter")) + if m.busy || len(m.queue) != 2 { + t.Fatalf("submit sent while resume in flight (busy=%v queue=%v)", m.busy, m.queue) + } + m.handleSessionDetail(sessionDetailMsg{sess: client.Session{ID: "s1"}, token: "t"}) + if m.resumeInFlight { + t.Fatal("resume outcome did not release the hold") + } + if m.sessionID != "s1" { + t.Fatalf("resume was refused: sessionID=%q", m.sessionID) + } +} + +// The plan step detail renders the REST snapshot's status verbatim. +func TestPlanDetailStatusSanitized(t *testing.T) { + m := newTestModel() + m.panel = panelPlan + m.plan = client.PlanSnapshot{Found: true, Steps: []client.PlanStep{{ID: "a", Title: "t", + Status: client.PlanStepStatus("done\x1b]52;c;Zm9v\x07")}}} + m.planInit = true + for _, l := range m.mgmtDetailLines(80) { + if strings.Contains(l, "\x1b]52") { + t.Fatalf("plan detail line carries an OSC sequence: %q", l) + } + } +} + +// /new during a slow startup resume: the late reply must not adopt the +// abandoned session, and /queue Enter must not send while a resume is held. +func TestStaleResumeAfterNewIsDropped(t *testing.T) { + m := newTestModel() + m.resumeInFlight = true + stale := sessionDetailMsg{sess: client.Session{ID: "old"}, token: "t", gen: m.resumeGen} + _ = m.startFreshSession() + if m.resumeInFlight { + t.Fatal("/new left the startup resume hold set") + } + m.handleSessionDetail(stale) + if m.sessionID == "old" { + t.Fatal("a resume fetched before /new adopted the abandoned session") + } +} + +func TestQueuePanelSendHeldDuringResume(t *testing.T) { + m := newTestModel() + m.queue = []string{"held"} + m.resumeInFlight = true + m.panel = panelQueue + _ = m.queueSendSelected() + if len(m.queue) != 1 || m.busy { + t.Fatal("/queue Enter sent while the startup resume was in flight") + } +} + +// An explicit ^R pick supersedes the startup resume still in flight. +func TestExplicitResumeSupersedesStartupResume(t *testing.T) { + m := newTestModel() + m.resumeInFlight = true + startup := sessionDetailMsg{sess: client.Session{ID: "startup"}, token: "t", gen: m.resumeGen} + _ = m.resumeSession("picked") + m.handleSessionDetail(sessionDetailMsg{sess: client.Session{ID: "picked"}, token: "t", gen: m.resumeGen}) + m.handleSessionDetail(startup) + if m.sessionID != "picked" { + t.Fatalf("late startup resume replaced the operator's pick: %q", m.sessionID) + } +} + +// /attach feedback (staged, or why not) must paint immediately — the note +// sat in state, invisible, until an unrelated redraw. +func TestAttachOutcomeNotePaintsImmediately(t *testing.T) { + for _, cmd := range []string{"/attach /nonexistent-bodek-path", "/unattach nothing-staged"} { + m := newTestModel() + m.msgs = append(m.msgs, message{role: roleUser, content: "q"}, message{role: roleAsst, content: "a"}) + m.refresh() + _ = m.runCommandLine(cmd) + if v := plain(m.View()); !strings.Contains(v, "attach: ") && !strings.Contains(v, "staged") { + t.Fatalf("%s: outcome note not painted", cmd) + } + } +} diff --git a/internal/tui/commands.go b/internal/tui/commands.go index 543ad11..ad18f73 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -150,10 +150,14 @@ func slashCommands() []command { return m.openAgents() }}, {"attach", "stage a file for the next prompt — /attach ", func(m *Model, args string) tea.Cmd { - return m.attachFile(args) + cmd := m.attachFile(args) + m.refresh() // the outcome note (staged or why not) must paint now + return cmd }}, {"unattach", "drop staged files — /unattach [name]", func(m *Model, args string) tea.Cmd { - return m.unattachFile(args) + cmd := m.unattachFile(args) + m.refresh() + return cmd }}, {"quit", "exit bodek", func(m *Model, _ string) tea.Cmd { m.quitting = true diff --git a/internal/tui/drawer.go b/internal/tui/drawer.go index 9bc8d3a..48c1622 100644 --- a/internal/tui/drawer.go +++ b/internal/tui/drawer.go @@ -282,6 +282,7 @@ func (m *Model) cycleDrawerTab(dir int) tea.Cmd { func (m *Model) openRuns() tea.Cmd { m.panel = panelRuns m.panelSel = 0 + m.runs = nil // a stale list would mis-anchor the first poll and target old rows m.panelEdit = panelEditNone m.panelMsg = "loading runs…" m.relayout() @@ -311,7 +312,19 @@ func (m *Model) handleRunsMsg(msg runsMsg) { m.panelMsg = "error: " + msg.err.Error() return } + // Anchor the selection by run id: a/D/t/c act on the highlighted run, + // so a poll that inserts or reaps rows must not slide it onto another. + prevID := "" + if m.panelSel < len(m.runs) { + prevID = m.runs[m.panelSel].ID + } m.runs = msg.runs + for i, r := range m.runs { + if prevID != "" && r.ID == prevID { + m.panelSel = i + break + } + } if m.panelSel >= len(m.runs) { m.panelSel = max(len(m.runs)-1, 0) } diff --git a/internal/tui/input.go b/internal/tui/input.go index 3314865..a571dd8 100644 --- a/internal/tui/input.go +++ b/internal/tui/input.go @@ -74,6 +74,13 @@ func newlineChord(s string) bool { return false } +// isKeySentinel reports whether s is one of the KeyRunes sentinels the +// input decoder synthesizes for enhanced chords; their rune text is a name, +// never something the user typed. +func isKeySentinel(s string) bool { + return newlineChord(s) || s == "shift+delete" || s == "alt+unmapped-chord" +} + // insertNewline drops a line break at the caret and refits the composer. func (m *Model) insertNewline() tea.Cmd { var cmd tea.Cmd @@ -472,7 +479,7 @@ func (m *Model) submit() tea.Cmd { m.refresh() return m.noticeSweep() } - if m.busy { + if m.busy || m.resumeInFlight { if m.status == "cancelling" { // cancelRun handed the queue back to the input so it would // not fire into the cancelled session. Re-queueing here @@ -492,6 +499,9 @@ func (m *Model) submit() tea.Cmd { m.refresh() m.vp.GotoBottom() // Enter means "show me the latest", even mid-turn m.teach(hintQueue, "tip: ^Q manages the queue · /queue opens the full panel") + if !m.busy { + return m.transientNoteCmd("queued — it sends once the session resumes") + } return m.transientNoteCmd("queued — it sends when the turn ends") } m.ta.Reset() @@ -580,16 +590,25 @@ func (m *Model) sendPrompt(text string) tea.Cmd { // sendQueued pops the oldest queued prompt and sends it when the model is // idle and connected; otherwise it is a no-op (nil cmd). func (m *Model) sendQueued() tea.Cmd { - if m.busy || m.disconn || len(m.queue) == 0 { + if m.busy || m.disconn || m.resumeInFlight || len(m.queue) == 0 { return nil } text := m.queue[0] m.queue = m.queue[1:] m.qsel = clampSel(m.qsel, len(m.queue)) if m.panel == panelQueue { - m.panelSel = clampSel(m.panelSel, len(m.queue)) // the drain races the open panel too + // The drain races the open panel too: rows shifted up by one, so + // keep the highlight on the same prompt. + if m.panelSel > 0 { + m.panelSel-- + } + m.panelSel = clampSel(m.panelSel, len(m.queue)) } m.qarm = -1 // the head just drained — never mis-aim the armed confirm + if m.confirm == confirmQueueDelete { + m.confirm = confirmNone // the armed row may have just been sent + m.panelMsg = "" + } if len(m.queue) == 0 { m.qfocus = false } @@ -602,7 +621,7 @@ func (m *Model) retryLast() tea.Cmd { if m.lastPrompt == "" { return m.transientNoteCmd("nothing to retry yet — send a prompt first") } - if m.busy { + if m.busy || m.resumeInFlight { if m.status == "cancelling" { return m.transientNoteCmd("cancelling — retry stays off the queue") } diff --git a/internal/tui/input_reassembler_poll.go b/internal/tui/input_reassembler_poll.go index ba98f47..e689d53 100644 --- a/internal/tui/input_reassembler_poll.go +++ b/internal/tui/input_reassembler_poll.go @@ -16,6 +16,9 @@ func (r *inputReassembler) waitReadable(budget time.Duration) bool { deadline := time.Now().Add(budget) for { ms := int(time.Until(deadline).Milliseconds()) + if ms < 0 { + ms = 0 // an EINTR retry past the deadline: a negative timeout would block forever + } n, err := unix.Poll([]unix.PollFd{{Fd: int32(r.file.Fd()), Events: unix.POLLIN}}, ms) if err == unix.EINTR { continue diff --git a/internal/tui/invocation.go b/internal/tui/invocation.go index 1f006a3..eae21fa 100644 --- a/internal/tui/invocation.go +++ b/internal/tui/invocation.go @@ -1,6 +1,7 @@ package tui import ( + "bytes" "encoding/json" "fmt" "strings" @@ -74,14 +75,14 @@ func invocationText(s step) string { delete(fields, key) out := "invocation · command\n" + visibleInvocation(command) if len(fields) > 0 { - if rest, err := json.MarshalIndent(fields, "", " "); err == nil { + if rest, err := indentJSON(fields); err == nil { out += "\nother arguments\n" + visibleInvocation(string(rest)) } } return appendArgsLimit(out, s.argsOmitted) } } - if pretty, err := json.MarshalIndent(fields, "", " "); err == nil { + if pretty, err := indentJSON(fields); err == nil { return appendArgsLimit("invocation · arguments\n"+visibleInvocation(string(pretty)), s.argsOmitted) } } @@ -112,3 +113,16 @@ func invocationDetailLines(s step, width int, th theme) []string { } return out } + +// indentJSON pretty-prints v without json.Marshal's HTML escaping, which +// would show "<" as \u003c in the inspector and in alt+i copies. +func indentJSON(v any) ([]byte, error) { + var b bytes.Buffer + enc := json.NewEncoder(&b) + enc.SetEscapeHTML(false) + enc.SetIndent("", " ") + if err := enc.Encode(v); err != nil { + return nil, err + } + return bytes.TrimRight(b.Bytes(), "\n"), nil +} diff --git a/internal/tui/jobs_tab.go b/internal/tui/jobs_tab.go index 23b73b0..8f06060 100644 --- a/internal/tui/jobs_tab.go +++ b/internal/tui/jobs_tab.go @@ -31,6 +31,7 @@ const ( type jobsFetchedMsg struct { jobs []client.Job err error + sid string // session the snapshot was fetched for } // jobsTickMsg re-arms the cadence; watch selects the background chain. @@ -90,7 +91,7 @@ func (m *Model) fetchJobs() tea.Cmd { cl, sid, tok := m.cl, m.sessionID, m.authToken return func() tea.Msg { jobs, err := cl.Jobs(sid, tok) - return jobsFetchedMsg{jobs: jobs, err: err} + return jobsFetchedMsg{jobs: jobs, err: err, sid: sid} } } diff --git a/internal/tui/local.go b/internal/tui/local.go index a91e34c..b450232 100644 --- a/internal/tui/local.go +++ b/internal/tui/local.go @@ -40,7 +40,7 @@ func (m *Model) restoreWorkspace() { if p == "" { continue } - _ = m.attachFile(p) + _ = m.restoreAttachment(p) } m.resumeTitle = st.SessionTitle if !m.opts.Fresh { @@ -110,11 +110,15 @@ func (m *Model) resumeLast() tea.Cmd { return nil } m.pendingResume = "" - cl := m.cl + // The connect-time session frame usually beats this round trip, and + // its batch drains the queue: a prompt sent now would make the turn + // busy and the resume would be refused. Hold sends until it lands. + m.resumeInFlight = true + cl, gen := m.cl, m.resumeGen token := m.tokens.Get(id) return func() tea.Msg { sess, eff, err := cl.SessionDetail(id, token) - return sessionDetailMsg{sess: sess, token: eff, err: err} + return sessionDetailMsg{sess: sess, token: eff, err: err, gen: gen} } } diff --git a/internal/tui/mgmt.go b/internal/tui/mgmt.go index 34b439c..de34f05 100644 --- a/internal/tui/mgmt.go +++ b/internal/tui/mgmt.go @@ -204,16 +204,39 @@ func (m *Model) handleMgmtMsg(msg mgmtMsg) { // Anchor the selection by identity across rebuilds: a fact inserted // above the selection must not silently swap the open detail. var sel string + var prev memRow if m.panelSel >= 0 && m.panelSel < len(m.memRows) { - sel = m.memRows[m.panelSel].text + prev = m.memRows[m.panelSel] + sel = prev.text } m.memView = msg.mem m.memRows = buildMemRows(msg.mem) m.pendingMemReload = false // a fresh listing supersedes any stale-row failure m.panelSel = anchorRow(m.panelSel, m.memRows, sel) - if len(m.memRows) == 0 { + armed := m.confirm == confirmFactDelete || m.confirm == confirmEpisodePromote || + m.confirm == confirmEpisodeDiscard + if armed { + // The gate acts on the row at confirm time: re-find the exact + // confirmed row (the same text can exist under user and env, or + // as two episodes), and disarm when it is gone — y must never + // act on a row the operator did not confirm. + armed = false + for i, r := range m.memRows { + if sameMemRow(r, prev) { + m.panelSel, armed = i, true + break + } + } + if !armed { + m.confirm = confirmNone + } + } + switch { + case armed: + // keep the visible "delete …?" prompt for the still-armed gate + case len(m.memRows) == 0: m.panelMsg = "no facts or pending episodes" - } else { + default: m.panelMsg = "" } case panelSkills: @@ -291,6 +314,12 @@ func (m *Model) handleMgmtMsg(msg mgmtMsg) { // anchorRow re-locates a selection by its row text after a rebuild: an // insert above must not silently change what the selection points at. Falls // back to clamping when the row vanished. +// sameMemRow reports whether two rows name the same fact or episode. +func sameMemRow(a, b memRow) bool { + return a.kind == b.kind && a.text == b.text && a.sessionID == b.sessionID && + a.summarySHA256 == b.summarySHA256 +} + func anchorRow(prev int, rows []memRow, text string) int { if text != "" { for i := range rows { @@ -869,7 +898,7 @@ func (m *Model) mgmtDetailLines(w int) []string { return []string{th.acDim.Render("no step selected")} } out = append(out, th.acSel.Render("› "+planGlyph(st.Status)+" "+sanitize(st.ID))) - meta := []string{string(st.Status)} + meta := []string{collapse(string(st.Status))} // REST snapshot status is wire text if m.planInit { meta = append(meta, fmt.Sprintf("v%d", m.plan.Version)) } diff --git a/internal/tui/model.go b/internal/tui/model.go index f41497e..b06742d 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" "time" + "unicode/utf8" "github.com/charmbracelet/bubbles/spinner" "github.com/charmbracelet/bubbles/textarea" @@ -245,37 +246,40 @@ type Model struct { failBellFired bool // (A3) the failure BEL rang for this turn — guard against double-fire apprBells []bool // per-approval urgent-window BEL latch, parallel to apprDeadlines - - approvals []client.Event // pending approval queue — odek runs parallel tools, so requests FIFO - apprDeadlines []time.Time // per-approval expiry, stamped on arrival (parallel to approvals) - apprSel int // highlighted option in the approval panel - apprExpanded bool // tab: show the full command/description text - apprTyped string // friction mode: the literal word being typed ("approve") - apprEditing bool // explicit focus on the friction confirmation editor - apprOffset int // expanded approval command page - ac autocomplete // @-reference completion state - pal palState // ⌘K command palette — the navigation spine - skillSuggest *client.Event // pending skill suggestion card (skill_event "suggested") - clarify *client.Event // pending principal-channel question - clarifyBuf string // typed answer (not the composer) - queue []string // prompts typed mid-turn, sent when the turn ends - qfocus bool // the queue strip owns the keyboard (ctrl+q) - qsel int // selected strip row while qfocus - qarm int // armed-for-delete row while qfocus (-1 = none) - lastPrompt string // most recent prompt sent — /retry re-sends it - ws *workspace.Store // nil in tests unless injected - pendingResume string // cwd last-session id; consumed by resumeLast - resumeTitle string // sanitized last-session title for the home card - attachPaths []string // original paths for staged files (persist only) - persistSeq int // generation guard for persistDraftMsg - copyMark copySpan // alt+m span start; zero = none - homePrompt string // last user prompt, kept across ^L for the session home - homeReceipt string // last turn's coding receipt, shown on the cleared home - homeSess []client.Session // recent sessions for the home dashboard - homeSessDone bool // the current clear's fetch already ran - homeSessGen int // bumped per clear; stale fetches dropped - focusIdx int // transcript cursor: turn head alt+↑/↓ last jumped to (-1 none) - inspect *inspectTarget // explicit keyboard focus on one transcript item + apprSweeping bool // an approvalExpireMsg tick is in flight (one chain at a time) + + approvals []client.Event // pending approval queue — odek runs parallel tools, so requests FIFO + apprDeadlines []time.Time // per-approval expiry, stamped on arrival (parallel to approvals) + apprSel int // highlighted option in the approval panel + apprExpanded bool // tab: show the full command/description text + apprTyped string // friction mode: the literal word being typed ("approve") + apprEditing bool // explicit focus on the friction confirmation editor + apprOffset int // expanded approval command page + ac autocomplete // @-reference completion state + pal palState // ⌘K command palette — the navigation spine + skillSuggest *client.Event // pending skill suggestion card (skill_event "suggested") + clarify *client.Event // pending principal-channel question + clarifyBuf string // typed answer (not the composer) + queue []string // prompts typed mid-turn, sent when the turn ends + qfocus bool // the queue strip owns the keyboard (ctrl+q) + qsel int // selected strip row while qfocus + qarm int // armed-for-delete row while qfocus (-1 = none) + lastPrompt string // most recent prompt sent — /retry re-sends it + ws *workspace.Store // nil in tests unless injected + pendingResume string // cwd last-session id; consumed by resumeLast + resumeInFlight bool // startup resume fetch pending: hold queued/new prompts until it lands + resumeGen int // bumped by /new: a session detail fetched before it is stale + resumeTitle string // sanitized last-session title for the home card + attachPaths []string // original paths for staged files (persist only) + persistSeq int // generation guard for persistDraftMsg + copyMark copySpan // alt+m span start; zero = none + homePrompt string // last user prompt, kept across ^L for the session home + homeReceipt string // last turn's coding receipt, shown on the cleared home + homeSess []client.Session // recent sessions for the home dashboard + homeSessDone bool // the current clear's fetch already ran + homeSessGen int // bumped per clear; stale fetches dropped + focusIdx int // transcript cursor: turn head alt+↑/↓ last jumped to (-1 none) + inspect *inspectTarget // explicit keyboard focus on one transcript item history []string // submitted prompts, newest last (recalled with ↑) histNav bool // true while ^P/^N is walking the history @@ -801,6 +805,11 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.handleJobsTick(msg) case jobsFetchedMsg: + if msg.sid != "" && msg.sid != m.sessionID { + // In flight across a switch/resume: the previous session's jobs + // must not become this session's baseline. Keep the chain alive. + return m, m.rearmJobs() + } attn := m.applyJobs(msg.jobs, msg.err) m.refresh() return m, tea.Batch(m.rearmJobs(), m.noticeSweep(), attn) @@ -895,6 +904,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case approvalExpireMsg: + m.apprSweeping = false // this chain's tick landed; approvalSweep may re-arm it drop := m.handleApprovalExpiry(time.Now()) m.refresh() return m, tea.Batch(drop, m.approvalSweep()) // re-arm while a form is open @@ -977,6 +987,13 @@ func (m *Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if msg.String() == "alt+unmapped-chord" { return m, m.transientNoteCmd("unmapped key chord ignored") } + // shift+delete is the composer's clear chord. Every other capture + // surface (approval composer + friction editor, clarify, popups, panel + // editors) would splice its sentinel text in, so it is a no-op there. + if msg.String() == "shift+delete" && (m.curApproval() != nil || m.clarify != nil || + m.ac.open || m.pal.open || m.panel != panelNone) { + return m, nil + } // The palette works from every rung of the modality ladder — except over // a live approval or clarify card, which captures the keyboard until // answered (a chord answering underneath would be a surprise decision). @@ -1297,6 +1314,8 @@ func (m *Model) startFreshSession() tea.Cmd { m.freshStart = true m.suppressRemember = true // the fresh connection's first session frame carries only a placeholder m.pendingResume = "" + m.resumeInFlight = false // a startup resume still in flight now targets the abandoned session + m.resumeGen++ if m.ws != nil && m.opts.CWD != "" { m.ws.ClearSession(m.opts.CWD) } @@ -1592,7 +1611,9 @@ func (m *Model) staleAge() string { // Tabs are kept — copied prose (Makefiles, gofmt output) must paste intact; // display paths expand them where cell math happens (truncate, clampLines). func sanitize(s string) string { - if !strings.ContainsFunc(s, needsSanitize) { + // Invalid UTF-8 takes the slow path too: a lone 0x9b/0x9d byte is an + // 8-bit CSI/OSC to some emulators; ranging rewrites it as U+FFFD. + if !strings.ContainsFunc(s, needsSanitize) && utf8.ValidString(s) { return s } var b strings.Builder @@ -1639,6 +1660,14 @@ func isInvisible(r rune) bool { return true case r == 0xfeff: // BOM / zero-width no-break space return true + case r == 0x061c, r == 0x180e: // Arabic letter mark, Mongolian vowel separator + return true + case r >= 0x2061 && r <= 0x2064: // invisible math operators + return true + case r >= 0xfff9 && r <= 0xfffb: // interlinear annotation controls + return true + case r >= 0xe0000 && r <= 0xe007f: // tag characters (hidden payload) + return true } return false } diff --git a/internal/tui/palette.go b/internal/tui/palette.go index 40f4252..efe5be9 100644 --- a/internal/tui/palette.go +++ b/internal/tui/palette.go @@ -113,6 +113,12 @@ func (m *Model) handlePalSessions(msg palSessionsMsg) tea.Cmd { return nil } m.pal.loading = false + // Late rows re-rank the list under a typed query: keep the highlight on + // the same entry so Enter never runs one the operator did not pick. + var selTitle, selKind string + if m.pal.sel >= 0 && m.pal.sel < len(m.pal.items) { + selTitle, selKind = m.pal.items[m.pal.sel].title, m.pal.items[m.pal.sel].kind + } if msg.err == nil { for _, s := range msg.items { sess := s @@ -129,6 +135,12 @@ func (m *Model) handlePalSessions(msg palSessionsMsg) tea.Cmd { } } m.filterPalette() + for i, e := range m.pal.items { + if selTitle != "" && e.title == selTitle && e.kind == selKind { + m.pal.sel = i + break + } + } m.refresh() return nil } diff --git a/internal/tui/panels.go b/internal/tui/panels.go index d7db384..d2481f2 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -121,10 +121,18 @@ func (m *Model) handleConfirmKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { // falls through to the composer, so the reflex "esc, keep typing" never // loses a character. Panel contexts keep silent disarm — their keys are // navigation, not typing. - if m.panel == panelNone && msg.Type == tea.KeyRunes && len(msg.Runes) > 0 { - m.ta.InsertString(string(msg.Runes)) - m.syncComposer() + // Only plain typing qualifies: Alt chords (approval decisions), pastes + // and key sentinels ("shift+enter"…) must never splice in as text, and + // a live approval card keeps its keys — the gate ate this one. + if m.panel != panelNone || msg.Type != tea.KeyRunes || len(msg.Runes) == 0 || + msg.Alt || msg.Paste || isKeySentinel(msg.String()) || m.curApproval() != nil { + return m, nil + } + if m.clarify != nil { + return m.handleClarifyKey(msg) // the answer buffer, not the hidden composer } + m.ta.InsertString(string(msg.Runes)) + m.syncComposer() return m, nil } @@ -184,6 +192,7 @@ type sessionDetailMsg struct { sess client.Session token string err error + gen int // resumeGen when the fetch was issued } type sessionDeletedMsg struct { @@ -766,11 +775,14 @@ func (m *Model) panelSelect() tea.Cmd { func (m *Model) resumeSession(id string) tea.Cmd { m.panelMsg = "loading session…" m.refresh() - cl := m.cl + // An explicit pick supersedes a startup resume still in flight: its + // late reply must not replace the session the operator chose. + m.resumeGen++ + cl, gen := m.cl, m.resumeGen token := m.tokens.Get(id) return func() tea.Msg { sess, eff, err := cl.SessionDetail(id, token) - return sessionDetailMsg{sess: sess, token: eff, err: err} + return sessionDetailMsg{sess: sess, token: eff, err: err, gen: gen} } } @@ -859,7 +871,7 @@ func (m *Model) exportSelected(format string) tea.Cmd { // overwriting: an existing file gets a -1, -2… suffix. 0600 — transcripts // can carry sensitive tool output, matching the tokens store standard. func writeExport(dir, id, format string, data []byte) (string, error) { - base := fmt.Sprintf("bodek-%s-%s", shortID(id), time.Now().Format("20060102-150405")) + base := fmt.Sprintf("bodek-%s-%s", fileSafeID(id), time.Now().Format("20060102-150405")) for i := 0; i < 1000; i++ { // O_EXCL: never a racing truncating overwrite suffix := "" if i > 0 { @@ -959,8 +971,12 @@ func (m *Model) handleSessionsMsg(msg sessionsMsg) { m.panelMsg = "error: " + msg.err.Error() return } + prevID := m.selectedSessionID() m.sessions = msg.items m.panelSel = 0 + if m.keepSessionDeleteArmed(prevID) { + return // the "delete …?" prompt stays visible for the still-armed gate + } if len(m.sessions) == 0 { m.panelMsg = "no saved sessions yet" } else { @@ -968,6 +984,29 @@ func (m *Model) handleSessionsMsg(msg sessionsMsg) { } } +// selectedSessionID is the highlighted session row's id, or "". +func (m *Model) selectedSessionID() string { + if m.panelSel >= 0 && m.panelSel < len(m.sessions) { + return m.sessions[m.panelSel].ID + } + return "" +} + +// keepSessionDeleteArmed reports whether an armed session delete still +// targets prevID after a list replacement. A late list or search page can +// re-rank rows under the gate; y must never delete a session the operator +// did not confirm, so a moved target disarms it. +func (m *Model) keepSessionDeleteArmed(prevID string) bool { + if m.confirm != confirmSessionDelete { + return false + } + if prevID != "" && m.selectedSessionID() == prevID { + return true + } + m.confirm = confirmNone + return false +} + // handleSessionsPageMsg applies a search/pagination envelope: replace on // fresh queries, append on "load more". A full page means more may follow. func (m *Model) handleSessionsPageMsg(msg sessionsPageMsg) { @@ -975,6 +1014,7 @@ func (m *Model) handleSessionsPageMsg(msg sessionsPageMsg) { m.panelMsg = "error: " + msg.err.Error() return } + prevID := m.selectedSessionID() if msg.append { m.sessions = append(m.sessions, msg.page.Sessions...) } else { @@ -985,6 +1025,8 @@ func (m *Model) handleSessionsPageMsg(msg sessionsPageMsg) { } m.sessHasMore = msg.page.Count >= msg.page.Limit && msg.page.Count > 0 switch { + case m.keepSessionDeleteArmed(prevID): + // the "delete …?" prompt stays visible for the still-armed gate case len(m.sessions) == 0: if m.sessQuery != "" { m.panelMsg = "no matches for “" + m.sessQuery + "”" @@ -1024,11 +1066,24 @@ func (m *Model) handleLimitsMsg(msg limitsMsg) { } func (m *Model) handleSessionDetail(msg sessionDetailMsg) tea.Cmd { + if msg.gen != m.resumeGen { + return nil // fetched before /new: the operator abandoned that session + } + m.resumeInFlight = false // every outcome releases the held queue if msg.err != nil { if m.panel != panelNone { m.panelMsg = "error: " + msg.err.Error() } - return m.transientNoteCmd("resume failed — /new or ^R") + return tea.Batch(m.transientNoteCmd("resume failed — /new or ^R"), m.sendQueued()) + } + // The running turn's remaining frames would stream into whatever + // replayed message sits at curIdx — never swap transcripts under it. + if m.busy { + if m.panel != panelNone { + m.panelMsg = "a turn is running — ^X to cancel, then retry" // replaces "loading session…" + m.refresh() + } + return m.transientNoteCmd("a turn is running — finish or cancel it (^X) before resuming") } // Replay the saved transcript into the local view, then adopt the session // on the connection (session_switch restores the server-side memory @@ -1059,7 +1114,17 @@ func (m *Model) handleSessionDetail(msg sessionDetailMsg) tea.Cmd { m.lastLatency = 0 m.resetCallMetrics() m.msgs = m.msgs[:0] + m.curIdx = -1 + m.focusIdx = -1 m.inspect = nil // hit targets belong to the replaced transcript + m.find = findState{} + m.copyMark = copySpan{} + // Approvals and clarify belong to the replaced conversation. + m.approvals = nil + m.apprDeadlines = nil + m.apprBells = nil + m.resetApprovalInput() + m.clearClarify() m.resetMsgBlocks() // The plan surface is session-scoped knowledge: drop it and refetch for // the resumed session. Neither live path covers a resume — the replayed @@ -1261,6 +1326,7 @@ func (m *Model) handleSessionDeleted(msg sessionDeletedMsg) tea.Cmd { return nil } m.tokens.Delete(msg.id) + prevID := m.selectedSessionID() for i := range m.sessions { if m.sessions[i].ID != msg.id { continue @@ -1272,6 +1338,9 @@ func (m *Model) handleSessionDeleted(msg sessionDeletedMsg) tea.Cmd { break } m.panelSel = clampSel(m.panelSel, len(m.sessions)) + if armed := m.confirm == confirmSessionDelete; armed && !m.keepSessionDeleteArmed(prevID) { + m.panelMsg = "" // a re-armed gate on the removed row must not slide onto its neighbour + } if len(m.sessions) == 0 { m.panelMsg = "no saved sessions yet" } @@ -1356,7 +1425,7 @@ func (m *Model) renderPanel(w, h int) string { body += "\n" + th.acSel.Render(prompt+": "+collapse(m.panelDraft)+"▏") } if m.panelMsg != "" { - body += "\n" + th.acDim.Render(m.panelMsg) + body += "\n" + th.acDim.Render(sanitize(m.panelMsg)) // may quote server bodies } if m.panel == panelStats { body += "\n" + m.statsBody() @@ -1490,12 +1559,25 @@ func windowRows(rows []string, sel, n int) []string { // shortID trims a session ID for display. func shortID(id string) string { - if len(id) > 17 { - return id[:17] + "…" + // Ids come off the wire: strip escapes and cut on a rune boundary. + id = collapse(id) + if r := []rune(id); len(r) > 17 { + return string(r[:17]) + "…" } return id } +// fileSafeID maps a wire id to a filename fragment: anything outside +// [A-Za-z0-9._-] becomes '_', so a hostile id cannot add path separators. +func fileSafeID(id string) string { + return strings.Map(func(r rune) rune { + if validSessionID(string(r)) { + return r + } + return '_' + }, shortID(id)) +} + // shortHash trims a hex hash to its first 12 characters. func shortHash(h string) string { if len(h) > 12 { diff --git a/internal/tui/plan.go b/internal/tui/plan.go index 3ff9460..eddf99e 100644 --- a/internal/tui/plan.go +++ b/internal/tui/plan.go @@ -120,6 +120,9 @@ func (m *Model) handlePlanMsg(msg planMsg) (cmd tea.Cmd) { cmd = tea.Batch(cmd, m.armPlanPoll()) } }() + if msg.want != m.sessionID { + return nil // fetched for a session we already left: its outcome says nothing about this one + } if msg.err != nil { // Only a 404 (old engine, route absent) is permanent. A transient // error — timeout under load, a 5xx, a transport blip — must not diff --git a/internal/tui/queue.go b/internal/tui/queue.go index 15baaeb..446f55f 100644 --- a/internal/tui/queue.go +++ b/internal/tui/queue.go @@ -331,6 +331,9 @@ func (m *Model) queueSendSelected() tea.Cmd { if m.busy { return m.transientNoteCmd("a turn is running — this sends when it ends · ← raises its priority") } + if m.resumeInFlight { + return m.transientNoteCmd("resuming the session — this sends once it lands") + } i := m.panelSel text := m.queue[i] m.queue = append(m.queue[:i], m.queue[i+1:]...) diff --git a/internal/tui/subagents.go b/internal/tui/subagents.go index 1900b4b..00ab46f 100644 --- a/internal/tui/subagents.go +++ b/internal/tui/subagents.go @@ -855,7 +855,7 @@ func parseAgentResult(data string) *agentResult { if env.Status == "" || env.Summary == "" { return nil } - r := &agentResult{status: env.Status, summary: collapse(env.Summary), tokens: env.TokensUsed, iters: env.Iterations, + r := &agentResult{status: collapse(env.Status), summary: collapse(env.Summary), tokens: env.TokensUsed, iters: env.Iterations, costUSD: env.CostUSD, artifacts: env.Artifacts, denials: env.Denials, denialsTotal: env.DenialsTotal} for _, f := range env.FilesChanged { if f = collapse(f); f != "" { diff --git a/internal/tui/view.go b/internal/tui/view.go index 5b592ce..b92e63f 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -2158,7 +2158,7 @@ func (m *Model) footerContent() string { // ^S arms from the composer — the gate must render where it captured // the keyboard, not only inside the agents panel. return m.panelFooter( - th.footerDanger.Render(strings.SplitN(m.panelMsg, "? ", 2)[0]+"?"), + th.footerDanger.Render(strings.SplitN(sanitize(m.panelMsg), "? ", 2)[0]+"?"), th.footerKey.Render("y")+th.footerDanger.Render(" stop"), th.footer.Render("any other key cancels"), ) diff --git a/internal/update/update.go b/internal/update/update.go index 3b9ea01..cb5cb91 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/url" "os" @@ -76,7 +77,8 @@ func queryAPI(ctx context.Context, client *http.Client, apiURL string) (string, return "", nil, statusError(resp.StatusCode, resp.Status) } var rel release - if err := json.NewDecoder(resp.Body).Decode(&rel); err != nil { + // A release document is a few KiB; bound it like every other decode. + if err := json.NewDecoder(io.LimitReader(resp.Body, 4<<20)).Decode(&rel); err != nil { return "", nil, fmt.Errorf("decode latest release: %w", err) } if rel.TagName == "" { @@ -196,9 +198,10 @@ func statusRetryable(err error) bool { // rule: "v1.2.0-rc1" against current "v1.2.0" compares equal and reports // false — this simple comparator ignores prerelease ordering. Comparison // is numeric over up to 3 dot-separated 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 +// components treated as 0. A Go pseudo-version or prerelease current +// (v0.1.3-0.20260901abcdef12-abc1234) compares by its release prefix and +// sits below the final release of that same core, so a commit-installed +// build still sees the release it predates. 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 { @@ -212,7 +215,10 @@ func Newer(latest, current string) bool { return l[i] > c[i] } } - return false + // Equal cores: a final release outranks a prerelease or Go + // pseudo-version of the same core (v0.1.3-0.2026…-abc is a commit + // built before v0.1.3 shipped). + return !strings.Contains(latest, "-") && strings.Contains(current, "-") } // parseSemver splits an optional-"v"-prefixed version into its numeric diff --git a/internal/update/update_test.go b/internal/update/update_test.go index c791fbd..95cba3d 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -32,6 +32,11 @@ func TestNewer(t *testing.T) { {"v1.2.0-rc1", "v1.1.9", true}, {"v1.2.0-rc1", "v1.2.0", false}, // release core equal → not newer {"1.3.0-rc1", "v1.2.0", true}, + // A pseudo-version names the next patch: the release it predates + // must still read as newer. + {"v0.1.3", "v0.1.3-0.20260901000000-abc1234", true}, + {"v0.1.2", "v0.1.3-0.20260901000000-abc1234", false}, + {"v0.1.3-rc2", "v0.1.3-rc1", false}, } for _, tc := range cases { if got := Newer(tc.latest, tc.current); got != tc.want { diff --git a/internal/watchdog/watchdog.go b/internal/watchdog/watchdog.go index 819577f..322b7f2 100644 --- a/internal/watchdog/watchdog.go +++ b/internal/watchdog/watchdog.go @@ -45,7 +45,10 @@ func Run(ctx context.Context, parentPID, targetPID int, poll, grace time.Duratio poll = DefaultPoll } token := startToken(targetPID) - for processAlive(parentPID) { + // The parent is identified the same way: a recycled parent PID must not + // keep the guard waiting forever. + parentToken := startToken(parentPID) + for sameTarget(parentPID, parentToken) { if !sameTarget(targetPID, token) { return // server already gone (or its PID recycled): nothing to guard } diff --git a/internal/workspace/workspace.go b/internal/workspace/workspace.go index d3cfcd5..8cc435d 100644 --- a/internal/workspace/workspace.go +++ b/internal/workspace/workspace.go @@ -136,13 +136,13 @@ func (s *Store) Save(cwd string, st State) error { if s == nil || cwd == "" { return nil } + // Persist under the lock (as tokens does): writing a snapshot after + // unlocking lets two interleaved saves land out of order on disk. s.mu.Lock() + defer s.mu.Unlock() s.reloadLocked(cwd) s.all[cwd] = cloneState(st) - path := s.path - snap := cloneAll(s.all) - s.mu.Unlock() - return persist(path, snap) + return persist(s.path, cloneAll(s.all)) } // Patch applies fn to the cwd snapshot and persists. @@ -151,16 +151,14 @@ func (s *Store) Patch(cwd string, fn func(*State)) { return } s.mu.Lock() + defer s.mu.Unlock() s.reloadLocked(cwd) st := cloneState(s.all[cwd]) fn(&st) s.all[cwd] = st - path := s.path - snap := cloneAll(s.all) - s.mu.Unlock() // A failed persist is not silent: draft/queue loss on a full disk or a // read-only directory must be diagnosable, like tokens' warnPersist. - if err := persist(path, snap); err != nil { + if err := persist(s.path, cloneAll(s.all)); err != nil { fmt.Fprintf(os.Stderr, "bodek: warning: workspace not saved: %v\n", err) } }