From c8752cef3261154f20e15666f6acc0fd045df2ec Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:12 +0200 Subject: [PATCH 01/28] fix(server): keep odek serve alive and stoppable around stderr A failing log sink (full disk) surfaced to os/exec, whose copy goroutine closed the stderr pipe and killed odek on SIGPIPE: the scan writer now swallows sink errors. WaitDelay bounds Wait when a grandchild that left the process group holds fd 2, so Stop can no longer hang forever, and the leader-pid fallback is skipped once the child is reaped (its pid may be recycled). Unterminated stderr lines are capped at 64 KiB in both the token scanner and the startup ring buffer. Co-Authored-By: Claude Opus 5.5 --- cmd/bodek/log.go | 7 ++++++ cmd/bodek/log_test.go | 11 ++++++++ internal/server/pgroup_unix.go | 6 ++--- internal/server/scan_bounds_test.go | 39 +++++++++++++++++++++++++++++ internal/server/server.go | 26 ++++++++++++++++++- 5 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 internal/server/scan_bounds_test.go 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/internal/server/pgroup_unix.go b/internal/server/pgroup_unix.go index 62bb758..78b9976 100644 --- a/internal/server/pgroup_unix.go +++ b/internal/server/pgroup_unix.go @@ -19,13 +19,13 @@ 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 is skipped once the reaper +// has collected the child: its pid may already belong to another process. 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 { + if err := syscall.Kill(-c.proc.Process.Pid, sig); err != nil && !c.reaped.Load() { _ = syscall.Kill(c.proc.Process.Pid, 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..b9908b5 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. @@ -424,12 +427,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 +476,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 +525,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) { From de680d5203f46198335356d60b7861d01189e78d Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:12 +0200 Subject: [PATCH 02/28] fix(watchdog): identify the parent by start time, not pid alone A recycled parent pid kept the guard waiting forever after bodek was killed, orphaning odek serve. The parent now gets the same start-time token check as the target. Co-Authored-By: Claude Opus 5.5 --- internal/watchdog/watchdog.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 } From 0cac82f5acd650b1d42d0bc3fd836e925185c79e Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:12 +0200 Subject: [PATCH 03/28] fix(bodek): let a second signal abort a slow shutdown The signal channel stayed registered after the first SIGINT/SIGTERM, so later signals were swallowed while Stop waited up to 30s. Co-Authored-By: Claude Opus 5.5 --- cmd/bodek/main.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/bodek/main.go b/cmd/bodek/main.go index 68d0c81..6bd2e61 100644 --- a/cmd/bodek/main.go +++ b/cmd/bodek/main.go @@ -370,6 +370,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() }() } From 86959f4d0f02b187e9890a72e1f240268b6cb23e Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:12 +0200 Subject: [PATCH 04/28] fix(bodek): never overwrite a settings file broken mid-run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save fell back to the startup baseline when the file became unreadable or unparsable, then replaced it — clobbering the user's hand edits, the case persistDisabled already guards at startup. It now refuses. Co-Authored-By: Claude Opus 5.5 --- cmd/bodek/main.go | 6 +++++- cmd/bodek/settings_persist_test.go | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cmd/bodek/main.go b/cmd/bodek/main.go index 6bd2e61..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() { 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) + } +} From b3030d5595d6316d125d00e9c27c83aa59b50d8b Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:12 +0200 Subject: [PATCH 05/28] fix(update): rank a release above its own pseudo-version builds Go pseudo-versions (and goreleaser -next snapshots) name the next patch: v0.1.3-0.- is a commit after v0.1.2, so the v0.1.3 release is newer. Equal cores read as up to date, hiding the upgrade. The existing test asserting the opposite is flipped deliberately. Co-Authored-By: Claude Opus 5.5 --- cmd/bodek/upgrade_swap_test.go | 9 +++++++-- internal/update/update.go | 12 ++++++++---- internal/update/update_test.go | 5 +++++ 3 files changed, 20 insertions(+), 6 deletions(-) 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/update/update.go b/internal/update/update.go index 3b9ea01..ad8669a 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -196,9 +196,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 +213,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 { From 209781dd3843fd688efe5e93f7e1c5508d853d15 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:12 +0200 Subject: [PATCH 06/28] fix(update): bound the release metadata decode Co-Authored-By: Claude Opus 5.5 --- internal/update/update.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/update/update.go b/internal/update/update.go index ad8669a..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 == "" { From 4723a053124f415109e90148f19426f7259f075c Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:23 +0200 Subject: [PATCH 07/28] fix(client): bound every REST JSON decode Seven decodes read resp.Body without the maxJSONBytes cap getJSON documents, so a misbehaving server could stream until bodek ran out of memory. Co-Authored-By: Claude Opus 5.5 --- internal/client/client.go | 3 ++- internal/client/jobs.go | 7 ++++--- internal/client/plan.go | 3 ++- internal/client/rest.go | 2 +- internal/client/runs.go | 3 ++- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 10a797b..5800941 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" @@ -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 diff --git a/internal/client/jobs.go b/internal/client/jobs.go index 2864c7e..64f6b42 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 @@ -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/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 From d41a48a5b3d91d737c3dcfef9bcfc1b8771a1a8b Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:24 +0200 Subject: [PATCH 08/28] fix(client): never follow redirects on authenticated REST calls Go strips Authorization on a cross-host redirect but copies custom headers, so X-Odek-Ws-Token and X-Session-Token (and a replayed POST body on 307/308) reached whatever host a proxy redirected to. odek's API never redirects; the 3xx now surfaces as a status error. Co-Authored-By: Claude Opus 5.5 --- internal/client/client.go | 13 ++++++-- internal/client/jobs.go | 2 +- internal/client/redirect_test.go | 52 ++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 internal/client/redirect_test.go diff --git a/internal/client/client.go b/internal/client/client.go index 5800941..13ec322 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -310,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). @@ -319,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() @@ -626,6 +626,15 @@ func (c *Client) send(v any) error { 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 64f6b42..a474fb2 100644 --- a/internal/client/jobs.go +++ b/internal/client/jobs.go @@ -98,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 } 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) + } + } +} From 6a62c7390eaa85b16fa505a190cf8de77a71897a Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:24 +0200 Subject: [PATCH 09/28] fix(client): bound websocket frame writes with a deadline A peer that stopped reading parked send forever while holding wmu, wedging every later approval, cancel, and ping behind it. Co-Authored-By: Claude Opus 5.5 --- internal/client/client.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/client/client.go b/internal/client/client.go index 13ec322..3dfaed1 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -619,10 +619,17 @@ 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) } From e775ba975a6eab57d9d340235492fd359585bcf2 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:32 +0200 Subject: [PATCH 10/28] fix(tokens): keep unpersisted tokens across the disk merge mergeLocked adopted the on-disk map wholesale and re-applied only the current id, so a token whose persist failed (full disk, read-only dir) was erased for good by the next successful Set. Mutations not yet on disk are now tracked and replayed after the merge. Co-Authored-By: Claude Opus 5.5 --- internal/tokens/tokens.go | 23 +++++++++++++++++++++++ internal/tokens/tokens_merge_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) 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) + } + } +} From 5f6d4945b280abcec4eba0c3f7b7b150479ff53b Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:32 +0200 Subject: [PATCH 11/28] fix(settings): create ~/.bodek with 0700 A first save from /theme created the directory 0755, exposing the names of the token and workspace stores that share it. Co-Authored-By: Claude Opus 5.5 --- internal/settings/settings.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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, "", " ") From 60d82df7f11f50818d9c5c1fe459991fd0e5cb93 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:00:32 +0200 Subject: [PATCH 12/28] fix(workspace): persist under the store mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Save and Patch wrote a snapshot after unlocking, so interleaved writes could land out of order on disk — the reorder tokens already fixed. Co-Authored-By: Claude Opus 5.5 --- internal/workspace/workspace.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) 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) } } From 947984d3bf3921187c49e9a073c76cffb6237b23 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:00 +0200 Subject: [PATCH 13/28] fix(tui): sanitize wire text at the remaining terminal sinks The panel status line (which can quote raw HTTP error bodies), sub-agent result status, and shortID'd session/run ids reached the terminal without sanitize(); shortID also cut mid-rune. sanitize's fast path let invalid UTF-8 through, so lone 0x9b/0x9d bytes (8-bit CSI/OSC) survived, and it missed U+061C, U+180E, U+2061-2064, U+FFF9-FFFB and the tag block. Export filenames now map a wire id to [A-Za-z0-9._-] so it cannot traverse out of the working directory. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 64 ++++++++++++++++++++++++++++++ internal/tui/model.go | 13 +++++- internal/tui/panels.go | 21 ++++++++-- internal/tui/subagents.go | 2 +- internal/tui/view.go | 2 +- 5 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 internal/tui/bughunt_audit_test.go diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go new file mode 100644 index 0000000..9ec739c --- /dev/null +++ b/internal/tui/bughunt_audit_test.go @@ -0,0 +1,64 @@ +package tui + +import ( + "path/filepath" + "strings" + "testing" + "unicode/utf8" +) + +// 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") + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index f41497e..93919a6 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" @@ -1592,7 +1593,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 +1642,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/panels.go b/internal/tui/panels.go index d7db384..4fa036e 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -859,7 +859,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 { @@ -1356,7 +1356,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 +1490,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/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"), ) From fe4b63f8408086a80002ad0291076d104d12f132 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:11 +0200 Subject: [PATCH 14/28] fix(tui): attach regular files only, with a capped read A FIFO blocked the Update goroutine forever (and, persisted, on every launch in that cwd); /dev/zero reports size 0 and read without bound. Co-Authored-By: Claude Opus 5.5 --- internal/tui/attach.go | 26 +++++++++++++++++++++++++- internal/tui/attach_fifo_unix_test.go | 22 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 internal/tui/attach_fifo_unix_test.go diff --git a/internal/tui/attach.go b/internal/tui/attach.go index c7623f5..2f0cb8c 100644 --- a/internal/tui/attach.go +++ b/internal/tui/attach.go @@ -3,6 +3,7 @@ package tui import ( "bytes" "fmt" + "io" "os" "path/filepath" "strings" @@ -54,6 +55,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 +71,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 +134,21 @@ 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) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer func() { _ = f.Close() }() + 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 +} diff --git a/internal/tui/attach_fifo_unix_test.go b/internal/tui/attach_fifo_unix_test.go new file mode 100644 index 0000000..1de5fa0 --- /dev/null +++ b/internal/tui/attach_fifo_unix_test.go @@ -0,0 +1,22 @@ +//go:build unix + +package tui + +import ( + "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") + } +} From 78f6de9c6b0851361765a1f2d779ffd6224adc52 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:11 +0200 Subject: [PATCH 15/28] fix(tui): keep armed actions and selections on their target Several lists are replaced under a live selection or armed gate, so a keypress could act on a row the operator never chose: - runs tab: a poll re-indexed rows under a/D/t/c (now anchored by id; reopening drops the stale list) - /queue: a head drain left the delete armed on the shifted index - memory: a refresh that dropped the confirmed fact retargeted y - sessions: a late list/search page or delete reply moved the armed delete onto another session - palette: late session rows re-ranked the highlighted entry Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 132 +++++++++++++++++++++++++++++ internal/tui/drawer.go | 13 +++ internal/tui/input.go | 11 ++- internal/tui/mgmt.go | 15 +++- internal/tui/palette.go | 12 +++ internal/tui/panels.go | 34 ++++++++ 6 files changed, 214 insertions(+), 3 deletions(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 9ec739c..97d9480 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -5,6 +5,10 @@ import ( "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 @@ -62,3 +66,131 @@ func TestPanelMsgRenderedSanitized(t *testing.T) { 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) + } +} + +// 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") + } +} + +// 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) + } +} + +// 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") + } +} 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..9f86fb7 100644 --- a/internal/tui/input.go +++ b/internal/tui/input.go @@ -587,9 +587,18 @@ func (m *Model) sendQueued() tea.Cmd { 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 } diff --git a/internal/tui/mgmt.go b/internal/tui/mgmt.go index 34b439c..84d3403 100644 --- a/internal/tui/mgmt.go +++ b/internal/tui/mgmt.go @@ -211,9 +211,20 @@ func (m *Model) handleMgmtMsg(msg mgmtMsg) { 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 && (m.panelSel >= len(m.memRows) || m.memRows[m.panelSel].text != sel) { + // The confirmed row is gone: y would act on a neighbour the + // operator never confirmed. + m.confirm = confirmNone + armed = false + } + 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: 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 4fa036e..3820d9d 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -959,8 +959,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 +972,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 +1002,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 +1013,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 + "”" @@ -1261,6 +1291,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 +1303,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" } From f888df06cd516debeabe077ace6f529b39e8ac8a Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:22 +0200 Subject: [PATCH 16/28] fix(tui): refuse session resume while a turn runs Resuming mid-turn swapped the transcript without resetting curIdx, so the running turn's remaining frames streamed into an arbitrary replayed message and its approval cards stayed up. Resume is refused while busy (the sessions panel says why), and a resume now also resets the turn-scoped focus, find, copy, approval and clarify state. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 ++ README.md | 3 ++- internal/tui/bughunt_audit_test.go | 25 +++++++++++++++++++++++++ internal/tui/panels.go | 19 +++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) 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/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 97d9480..6d16b37 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -102,6 +102,31 @@ func TestRunsPollAnchorsSelectionByID(t *testing.T) { } } +// 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() diff --git a/internal/tui/panels.go b/internal/tui/panels.go index 3820d9d..968b903 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -1060,6 +1060,15 @@ func (m *Model) handleSessionDetail(msg sessionDetailMsg) tea.Cmd { } return m.transientNoteCmd("resume failed — /new or ^R") } + // 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 // buffer; the reply's session event re-syncs state). @@ -1089,7 +1098,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 From 9e7fdeac78ce2528354dcfa004da7077f4b03c36 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:23 +0200 Subject: [PATCH 17/28] fix(tui): run one approval expiry sweep at a time Every wire batch called approvalSweep while an approval was pending and each tick re-armed its own chain, so a long wait under streaming frames built hundreds of 1s chains, each refreshing the view. Co-Authored-By: Claude Opus 5.5 --- internal/tui/approval_expiry.go | 7 +++++-- internal/tui/bughunt_audit_test.go | 19 +++++++++++++++++++ internal/tui/model.go | 2 ++ 3 files changed, 26 insertions(+), 2 deletions(-) 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/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 6d16b37..a7c0dea 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -137,6 +137,25 @@ func TestRunsReopenDropsStaleList(t *testing.T) { } } +// 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 memory refresh that drops the confirmed fact must disarm the gate. func TestFactDeleteDisarmsWhenRowVanishes(t *testing.T) { m := newTestModel() diff --git a/internal/tui/model.go b/internal/tui/model.go index 93919a6..c276230 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -246,6 +246,7 @@ 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 + 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) @@ -896,6 +897,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 From 5120f2f468f12cb197744e14f84ef0cb9d4b1979 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:23 +0200 Subject: [PATCH 18/28] fix(tui): drop jobs and plan replies for a session already left An in-flight jobs snapshot from the previous session became the new session's baseline (wrong rows, stop with a foreign job id, lost exit notes), and a late plan 404 for the old session disabled the plan strip before the stale-reply guard ran. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 21 +++++++++++++++++++++ internal/tui/jobs_tab.go | 3 ++- internal/tui/model.go | 5 +++++ internal/tui/plan.go | 3 +++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index a7c0dea..c1b893f 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -156,6 +156,27 @@ func TestApprovalSweepSingleChain(t *testing.T) { } } +// 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() 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/model.go b/internal/tui/model.go index c276230..3ff206b 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -803,6 +803,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) 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 From e0db69e629335acbb8f2d763f44c19d1486241a7 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:32 +0200 Subject: [PATCH 19/28] fix(tui): never type key sentinels or chords into hidden inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disarming a confirm forwarded any rune key to the composer, so Alt decisions, pastes and the shift+enter/shift+delete sentinels landed as text — under an approval this also killed the bare-key path. Only plain typing falls through now (clarify gets it in its own buffer), and the shift+delete sentinel is a no-op on the approval, clarify, popup, palette and panel surfaces instead of splicing in its name. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 31 ++++++++++++++++++++++++++++++ internal/tui/input.go | 7 +++++++ internal/tui/model.go | 7 +++++++ internal/tui/panels.go | 14 +++++++++++--- 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index c1b893f..edde298 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -210,6 +210,37 @@ func TestPaletteSelectionSurvivesLateSessions(t *testing.T) { } } +// 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) { diff --git a/internal/tui/input.go b/internal/tui/input.go index 9f86fb7..c9cb4f3 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 diff --git a/internal/tui/model.go b/internal/tui/model.go index 3ff206b..6273c06 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -985,6 +985,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). diff --git a/internal/tui/panels.go b/internal/tui/panels.go index 968b903..5a8f8fa 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 } From f4ba41e8e935e767da177adf11c4798b8078d5b4 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:32 +0200 Subject: [PATCH 20/28] fix(tui): clamp an expired poll timeout to zero An EINTR retry past the deadline passed a negative timeout to poll(2), which blocks indefinitely: a held ESC or partial sequence then waited for the next keystroke instead of its 10ms budget. Co-Authored-By: Claude Opus 5.5 --- internal/tui/input_reassembler_poll.go | 3 +++ 1 file changed, 3 insertions(+) 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 From 610275a30a45ec98d35e964ded8e475843bf6d95 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:01:32 +0200 Subject: [PATCH 21/28] fix(tui): show tool arguments without HTML escaping json.MarshalIndent escaped <, > and & to \u003c etc. in the inspector and in alt+i copies. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 11 +++++++++++ internal/tui/invocation.go | 18 ++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index edde298..45c2107 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -210,6 +210,17 @@ func TestPaletteSelectionSurvivesLateSessions(t *testing.T) { } } +// 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) { 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 +} From 9d9c142f3c637dfe2b6e97b6823ccf0b9f1b55e3 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:43:52 +0200 Subject: [PATCH 22/28] fix(server): keep the orphan watchdog armed until odek exits Stop retired the watchdog before signalling and waiting. With a second Ctrl+C now restoring the default signal action, bodek could die mid-Stop with nothing left to escalate a hung odek serve, orphaning it. The watchdog is retired in a defer once the child is gone; it stays inert while bodek is alive. Co-Authored-By: Claude Opus 5.5 --- internal/server/server.go | 19 ++++++++++++------- internal/server/stop_event_test.go | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index b9908b5..7c83c73 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -300,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 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) + } +} From c6f6a4fde8e529fc644ed3ebb50c486a45c77907 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:43:52 +0200 Subject: [PATCH 23/28] fix(server): send the leader fallback through Process.Signal The reaper flags the child only after Wait returns, up to WaitDelay after wait4 freed the pid, so the raw kill(pid) fallback could hit a recycled process. Process.Signal reports ErrProcessDone from the moment the child is reaped. Co-Authored-By: Claude Opus 5.5 --- internal/server/pgroup_unix.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/server/pgroup_unix.go b/internal/server/pgroup_unix.go index 78b9976..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. The leader fallback is skipped once the reaper -// has collected the child: its pid may already belong to another process. +// 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 && !c.reaped.Load() { - _ = syscall.Kill(c.proc.Process.Pid, sig) + _ = c.proc.Process.Signal(sig) } } From 6091813a0f5bcd4b8e1ab364b120f133d4c11f1c Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:43:53 +0200 Subject: [PATCH 24/28] fix(tui): hold sends until an in-flight session resume lands With --resume and a persisted queue, the connect-time session frame drained the queue before SessionDetail returned; the turn went busy and the resume was refused, so the saved session was lost. Queue drains, composer submits and /queue Enter now wait for the startup resume, and every outcome releases the hold. A resume generation drops replies made stale by /new or by a later explicit ^R pick. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 62 +++++++++++++++++++++++++++++ internal/tui/input.go | 9 +++-- internal/tui/local.go | 8 +++- internal/tui/model.go | 64 ++++++++++++++++-------------- internal/tui/panels.go | 14 +++++-- internal/tui/queue.go | 3 ++ 6 files changed, 122 insertions(+), 38 deletions(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 45c2107..45fea55 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -301,3 +301,65 @@ func TestSessionDeletedDisarmsGateOnRemovedRow(t *testing.T) { t.Fatal("gate slid onto the neighbouring session") } } + +// 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) + } +} + +// /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) + } +} diff --git a/internal/tui/input.go b/internal/tui/input.go index c9cb4f3..a571dd8 100644 --- a/internal/tui/input.go +++ b/internal/tui/input.go @@ -479,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 @@ -499,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() @@ -587,7 +590,7 @@ 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] @@ -618,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/local.go b/internal/tui/local.go index a91e34c..fb34e95 100644 --- a/internal/tui/local.go +++ b/internal/tui/local.go @@ -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/model.go b/internal/tui/model.go index 6273c06..b06742d 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -248,36 +248,38 @@ type Model struct { apprBells []bool // per-approval urgent-window BEL latch, parallel to apprDeadlines 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 - 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 + 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 @@ -1312,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) } diff --git a/internal/tui/panels.go b/internal/tui/panels.go index 5a8f8fa..d2481f2 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -192,6 +192,7 @@ type sessionDetailMsg struct { sess client.Session token string err error + gen int // resumeGen when the fetch was issued } type sessionDeletedMsg struct { @@ -774,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} } } @@ -1062,11 +1066,15 @@ 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. 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:]...) From d7bf6b548c3a1d30ae5bcda42c6c101f9504b69d Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:44:04 +0200 Subject: [PATCH 25/28] fix(tui): keep memory confirms on the exact row identity The armed-confirm check after a refresh matched on text alone, so the same fact under user and env (or two episodes with one summary) could move y onto a row never confirmed. The gate now re-finds the row by kind, text, session and hash, or disarms. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 19 +++++++++++++++++++ internal/tui/mgmt.go | 28 +++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 45fea55..16e17b3 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -302,6 +302,25 @@ func TestSessionDeletedDisarmsGateOnRemovedRow(t *testing.T) { } } +// 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() diff --git a/internal/tui/mgmt.go b/internal/tui/mgmt.go index 84d3403..7e00085 100644 --- a/internal/tui/mgmt.go +++ b/internal/tui/mgmt.go @@ -204,8 +204,10 @@ 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) @@ -213,11 +215,21 @@ func (m *Model) handleMgmtMsg(msg mgmtMsg) { m.panelSel = anchorRow(m.panelSel, m.memRows, sel) armed := m.confirm == confirmFactDelete || m.confirm == confirmEpisodePromote || m.confirm == confirmEpisodeDiscard - if armed && (m.panelSel >= len(m.memRows) || m.memRows[m.panelSel].text != sel) { - // The confirmed row is gone: y would act on a neighbour the - // operator never confirmed. - m.confirm = confirmNone + 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: @@ -302,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 { From e3e57bb1a9ad40e569bff142ea58938ad9c04d27 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:44:04 +0200 Subject: [PATCH 26/28] fix(tui): sanitize plan step status in the detail view The REST plan snapshot's status reached the terminal raw when a step was opened, an escape-injection sink the sanitize sweep missed. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 14 ++++++++++++++ internal/tui/mgmt.go | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 16e17b3..578f496 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -343,6 +343,20 @@ func TestStartupResumeHoldsQueue(t *testing.T) { } } +// 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) { diff --git a/internal/tui/mgmt.go b/internal/tui/mgmt.go index 7e00085..de34f05 100644 --- a/internal/tui/mgmt.go +++ b/internal/tui/mgmt.go @@ -898,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)) } From a684756c1ae91f64cc85a759469ae18bacba5622 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:44:04 +0200 Subject: [PATCH 27/28] fix(tui): never silently re-attach a path a symlink was planted on Persisted attachments are re-read at startup without the operator choosing them. An agent with workspace write access could swap one (or a parent directory) for a symlink to a host secret, which then went out with the next prompt. attachFile now stages and persists the fully resolved path, restore skips any path that no longer resolves to itself, and readCapped opens O_NONBLOCK and vets the descriptor so a path swapped for a FIFO after the stat cannot block the UI. Co-Authored-By: Claude Opus 5.5 --- internal/tui/attach.go | 30 +++++++++++- internal/tui/attach_fifo_unix_test.go | 69 +++++++++++++++++++++++++++ internal/tui/local.go | 2 +- 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/internal/tui/attach.go b/internal/tui/attach.go index 2f0cb8c..4c05a64 100644 --- a/internal/tui/attach.go +++ b/internal/tui/attach.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "syscall" tea "github.com/charmbracelet/bubbletea" @@ -48,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()) @@ -138,11 +144,19 @@ func expandTilde(p string) string { // 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) { - f, err := os.Open(path) + // 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 @@ -152,3 +166,17 @@ func readCapped(path string, limit int64) ([]byte, error) { } 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 index 1de5fa0..fb085a0 100644 --- a/internal/tui/attach_fifo_unix_test.go +++ b/internal/tui/attach_fifo_unix_test.go @@ -3,6 +3,7 @@ package tui import ( + "os" "path/filepath" "syscall" "testing" @@ -20,3 +21,71 @@ func TestAttachRejectsNonRegularFile(t *testing.T) { 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/local.go b/internal/tui/local.go index fb34e95..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 { From 45d5a5cfbe08c7ede0c2ec43baaae2bff8c802b4 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 12 Oct 2026 15:44:04 +0200 Subject: [PATCH 28/28] fix(tui): paint /attach and /unattach outcome notes immediately The note (staged, or why not) was pushed into state without a redraw, so it stayed invisible until an unrelated event repainted. Co-Authored-By: Claude Opus 5.5 --- internal/tui/bughunt_audit_test.go | 14 ++++++++++++++ internal/tui/commands.go | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/internal/tui/bughunt_audit_test.go b/internal/tui/bughunt_audit_test.go index 578f496..abf1253 100644 --- a/internal/tui/bughunt_audit_test.go +++ b/internal/tui/bughunt_audit_test.go @@ -396,3 +396,17 @@ func TestExplicitResumeSupersedesStartupResume(t *testing.T) { 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