Skip to content

Commit c6d0bf2

Browse files
committed
fix(session): unbreak CI - small-cap trim fixtures + incremental redaction
TestSave_TrimsOversizedSession timed out at 10m in CI: the 40 MiB fixture forced redact.RedactSecrets (20+ regexes) over the whole transcript on save, and saveLocked re-redacts the ENTIRE history on every save - O(history) per write, O(n^2) over a session's life. - MaxSessionFileBytes is now a var (comment: production treats it as fixed) so cap-trimming tests use a 64 KiB cap with tiny fixtures instead of multi-MiB transcripts; the oversized trim tests drop from ~25s to <0.1s and the session package from ~42s to ~1.5s. - Real production win behind the same failure: sessions are append-only, so saveLocked now redacts only messages at or beyond a persisted RedactBoundary instead of the full transcript on every save. Old files default to 0 (= redact all once); the boundary is set to the surviving message count before the final marshal so it persists. - New TestSave_IncrementalRedaction pins the boundary behavior; small-cap fixtures keep identical trim semantics coverage.
1 parent 84df555 commit c6d0bf2

3 files changed

Lines changed: 105 additions & 15 deletions

File tree

internal/session/session.go

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ import (
3535
// MaxSessionFileBytes caps the on-disk size of a session file that Load will
3636
// read into memory. This prevents a tampered or corrupted multi-gigabyte
3737
// session file from causing an OOM when any caller loads it.
38-
const MaxSessionFileBytes = 32 * 1024 * 1024 // 32 MiB
38+
// It is a var, not a const, so tests can temporarily shrink it instead of
39+
// building multi-MiB fixtures — production code should treat it as fixed.
40+
var MaxSessionFileBytes = 32 * 1024 * 1024 // 32 MiB
3941

4042
// ── Types ──────────────────────────────────────────────────────────────
4143

@@ -52,6 +54,14 @@ type Session struct {
5254
Sandbox bool `json:"sandbox"` // was sandboxed — auto-apply on resume
5355
Messages []llm.Message `json:"messages"` // full conversation history
5456
Buffer []string `json:"buffer,omitempty"` // last N turn summaries (memory tier 2)
57+
58+
// RedactBoundary records how many leading messages have already been
59+
// secret-redacted by a previous save. Redacting the full transcript on
60+
// every save is O(history) per write — O(n²) over a session's life —
61+
// and dominates save time on long sessions (20+ regexes over tens of
62+
// MB). Sessions are append-only, so only messages at or beyond the
63+
// boundary need scanning. Old files default to 0 (= redact all once).
64+
RedactBoundary int `json:"redact_boundary,omitempty"`
5565
}
5666

5767
// ── Store ──────────────────────────────────────────────────────────────
@@ -355,13 +365,22 @@ func (s *Store) saveLocked(sess *Session) error {
355365
return fmt.Errorf("session: refusing unsafe save: %w", err)
356366
}
357367

358-
// Redact secrets from all messages and the task label before writing to
359-
// disk. This is defense-in-depth: the loop engine already redacts tool
360-
// outputs, but this catches any secrets that slipped through
361-
// (e.g. LLM hallucinations, direct API usage, or the first user prompt
362-
// stored as the session title).
368+
// Redact secrets before writing to disk. This is defense-in-depth: the
369+
// loop engine already redacts tool outputs, but this catches any secrets
370+
// that slipped through (e.g. LLM hallucinations, direct API usage, or
371+
// the first user prompt stored as the session title). Sessions are
372+
// append-only, so only messages at or beyond sess.RedactBoundary are
373+
// scanned — messages already redacted by a previous save are not
374+
// re-scanned (see the field comment for the O(n²) rationale).
363375
sess.Task = redact.RedactSecrets(sess.Task)
364-
for i := range sess.Messages {
376+
boundary := sess.RedactBoundary
377+
if boundary < 0 {
378+
boundary = 0
379+
}
380+
if boundary > len(sess.Messages) {
381+
boundary = len(sess.Messages)
382+
}
383+
for i := boundary; i < len(sess.Messages); i++ {
365384
sess.Messages[i].Content = redact.RedactSecrets(sess.Messages[i].Content)
366385
sess.Messages[i].ReasoningContent = redact.RedactSecrets(sess.Messages[i].ReasoningContent)
367386
}
@@ -377,8 +396,9 @@ func (s *Store) saveLocked(sess *Session) error {
377396
// the most recent turns, mirroring the loop's trim semantics) until the
378397
// serialized form fits.
379398
if len(data) > MaxSessionFileBytes {
380-
data, err = s.trimToFileCapLocked(sess, data)
381-
if err != nil {
399+
// The trim's own marshaled form is discarded: the final marshal
400+
// below recomputes it after RedactBoundary is updated.
401+
if _, err = s.trimToFileCapLocked(sess, data); err != nil {
382402
return err
383403
}
384404
if s.trimWarned == nil {
@@ -389,6 +409,15 @@ func (s *Store) saveLocked(sess *Session) error {
389409
fmt.Fprintf(os.Stderr, "odek: warning: session %s exceeded %d bytes on write — oldest messages trimmed to stay within the load cap\n", sess.ID, MaxSessionFileBytes)
390410
}
391411
}
412+
// Every surviving message is now redacted: those before the boundary by
413+
// earlier saves, the rest just now — and trimming only removes messages,
414+
// so the boundary is simply the surviving count. Set it before the final
415+
// marshal so it is actually persisted.
416+
sess.RedactBoundary = len(sess.Messages)
417+
data, err = json.Marshal(sess)
418+
if err != nil {
419+
return fmt.Errorf("session: marshal: %w", err)
420+
}
392421

393422
if err := fsatomic.WriteFile(s.path(sess.ID), data, 0600); err != nil {
394423
return fmt.Errorf("session: write: %w", err)
@@ -472,7 +501,7 @@ func (s *Store) Load(id string) (*Session, error) {
472501
if err != nil {
473502
return nil, fmt.Errorf("session: load %q: %w", id, err)
474503
}
475-
if info.Size() > MaxSessionFileBytes {
504+
if info.Size() > int64(MaxSessionFileBytes) {
476505
return nil, fmt.Errorf("session: load %q: file too large (%d bytes, max %d)", id, info.Size(), MaxSessionFileBytes)
477506
}
478507
data, err := os.ReadFile(s.path(id))

internal/session/session_savecap_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,20 @@ func TestSave_IndexWriteError(t *testing.T) {
4343
}
4444
}
4545

46+
// withFileCap temporarily shrinks MaxSessionFileBytes so cap-trimming tests
47+
// can use small fixtures instead of multi-MiB transcripts (which made the
48+
// session suite time out in CI).
49+
func withFileCap(t *testing.T, n int) {
50+
t.Helper()
51+
orig := MaxSessionFileBytes
52+
MaxSessionFileBytes = n
53+
t.Cleanup(func() { MaxSessionFileBytes = orig })
54+
}
55+
4656
// TestTrimToFileCap_NothingDroppable covers the degenerate break branch: a
4757
// single system message with nothing left to drop is returned as-is.
4858
func TestTrimToFileCap_NothingDroppable(t *testing.T) {
59+
withFileCap(t, 64<<10)
4960
store, err := NewStoreWithDir(t.TempDir())
5061
if err != nil {
5162
t.Fatalf("NewStoreWithDir() error: %v", err)
@@ -71,6 +82,7 @@ func TestTrimToFileCap_NothingDroppable(t *testing.T) {
7182
// an assistant tool_calls message is dropped together with its tool results,
7283
// and the trim ends once the re-marshaled session fits.
7384
func TestTrimToFileCap_DropsToolGroups(t *testing.T) {
85+
withFileCap(t, 64<<10)
7486
store, err := NewStoreWithDir(t.TempDir())
7587
if err != nil {
7688
t.Fatalf("NewStoreWithDir() error: %v", err)
@@ -107,3 +119,48 @@ func TestTrimToFileCap_DropsToolGroups(t *testing.T) {
107119
}
108120
}
109121
}
122+
123+
// TestSave_IncrementalRedaction verifies secrets are redacted only in
124+
// messages at or beyond the persisted RedactBoundary: the first save redacts
125+
// everything, later saves scan only newly appended messages.
126+
func TestSave_IncrementalRedaction(t *testing.T) {
127+
store, err := NewStoreWithDir(t.TempDir())
128+
if err != nil {
129+
t.Fatalf("NewStoreWithDir() error: %v", err)
130+
}
131+
secret1 := "sk-" + strings.Repeat("a1", 20)
132+
secret2 := "sk-" + strings.Repeat("b2", 20)
133+
134+
sess, err := store.Create([]llm.Message{
135+
{Role: "system", Content: "you are odek"},
136+
{Role: "user", Content: "here is my key " + secret1},
137+
}, "test", "redact boundary")
138+
if err != nil {
139+
t.Fatalf("Create() error: %v", err)
140+
}
141+
loaded, err := store.Load(sess.ID)
142+
if err != nil {
143+
t.Fatalf("Load() error: %v", err)
144+
}
145+
if !strings.Contains(loaded.Messages[1].Content, "[REDACTED]") {
146+
t.Errorf("secret in first save not redacted: %q", loaded.Messages[1].Content)
147+
}
148+
if loaded.RedactBoundary != len(loaded.Messages) {
149+
t.Errorf("RedactBoundary = %d, want %d after first save", loaded.RedactBoundary, len(loaded.Messages))
150+
}
151+
152+
loaded.Messages = append(loaded.Messages, llm.Message{Role: "assistant", Content: "try " + secret2})
153+
if err := store.Save(loaded); err != nil {
154+
t.Fatalf("Save() error: %v", err)
155+
}
156+
reloaded, err := store.Load(sess.ID)
157+
if err != nil {
158+
t.Fatalf("Load() after second save: %v", err)
159+
}
160+
if !strings.Contains(reloaded.Messages[2].Content, "[REDACTED]") {
161+
t.Errorf("secret in appended message not redacted: %q", reloaded.Messages[2].Content)
162+
}
163+
if reloaded.RedactBoundary != len(reloaded.Messages) {
164+
t.Errorf("RedactBoundary = %d, want %d after second save", reloaded.RedactBoundary, len(reloaded.Messages))
165+
}
166+
}

internal/session/session_test.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,10 +1291,12 @@ func TestBuildConversationText(t *testing.T) {
12911291
// trailing messages must survive; the oldest turns are dropped.
12921292
func TestSave_TrimsOversizedSession(t *testing.T) {
12931293
store := newTestStore(t)
1294+
withFileCap(t, 64<<10)
12941295

1295-
// ~10 MiB per message × 4 messages ≈ 40 MiB serialized — past the 32 MiB
1296-
// cap. Few large messages keep the trim loop's re-marshal cycles cheap.
1297-
big := strings.Repeat("x", 10<<20)
1296+
// 20 KiB per message × 4 messages ≈ 80 KiB serialized — past the 64 KiB
1297+
// test cap. Few large messages keep the trim loop's re-marshal cycles
1298+
// cheap; the small cap (see withFileCap) keeps this fast in CI.
1299+
big := strings.Repeat("x", 20<<10)
12981300
msgs := []llm.Message{{Role: "system", Content: "you are odek"}}
12991301
for i := 0; i < 4; i++ {
13001302
role := "user"
@@ -1318,7 +1320,7 @@ func TestSave_TrimsOversizedSession(t *testing.T) {
13181320
if err != nil {
13191321
t.Fatalf("stat session file: %v", err)
13201322
}
1321-
if info.Size() > MaxSessionFileBytes {
1323+
if info.Size() > int64(MaxSessionFileBytes) {
13221324
t.Errorf("session file size = %d, exceeds MaxSessionFileBytes %d", info.Size(), MaxSessionFileBytes)
13231325
}
13241326

@@ -1352,8 +1354,10 @@ func TestSave_TrimsOversizedSession(t *testing.T) {
13521354
// transcript never starts with an orphaned tool message.
13531355
func TestSave_TrimKeepsToolGroupsIntact(t *testing.T) {
13541356
store := newTestStore(t)
1357+
withFileCap(t, 64<<10)
13551358

1356-
big := strings.Repeat("y", 7<<20)
1359+
// 6 × 12 KiB ≈ 72 KiB — past the 64 KiB test cap.
1360+
big := strings.Repeat("y", 12<<10)
13571361
msgs := []llm.Message{{Role: "system", Content: "you are odek"}}
13581362
for i := 0; i < 3; i++ {
13591363
call := llm.Message{Role: "assistant", Content: big}

0 commit comments

Comments
 (0)