diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 7e32908..e87d6a6 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -147,3 +147,11 @@ Architecture-shaping decisions graduate to an ADR under - 2026-07-18 — Demo/record banners derive the display URL via `demo.DisplayURL` instead of concatenating `-addr` after a literal "localhost", fixing the broken `http://localhost0.0.0.0:8737` shown for an explicit-host bind. +- 2026-07-18 — `session.Create` uses `os.Mkdir` (after `MkdirAll(outRoot)`) + instead of `os.MkdirAll(dir)`, so two captures starting within the same + wall-clock second fail with EEXIST rather than silently sharing one directory + (which clobbered the first manifest's t0 and conflated append-only streams). +- 2026-07-18 — `session.ReadJSONL` skips whitespace-only lines + (`bytes.TrimSpace`), matching `analyze.Load`, so an exchanged/hand-edited + session's blank line is skipped as documented rather than crashing merge/report + with "unexpected end of JSON input". diff --git a/internal/session/session.go b/internal/session/session.go index 4b5a9ed..fcccb15 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -15,6 +15,7 @@ package session import ( "bufio" + "bytes" "encoding/json" "errors" "fmt" @@ -61,8 +62,16 @@ const dirLayout = "2006-01-02_150405" // created directory path. Both demo and record call this so the manifest is // written once, by one code path. func Create(outRoot string, now time.Time, m Manifest) (dir string, err error) { + if err := os.MkdirAll(outRoot, 0o755); err != nil { + return "", err + } dir = filepath.Join(outRoot, now.Format(dirLayout)) - if err := os.MkdirAll(dir, 0o755); err != nil { + // os.Mkdir (not MkdirAll) fails with EEXIST if the directory already exists, + // so two captures starting within the same second-granularity instant cannot + // silently resolve to — and share — one directory. Reusing it would clobber + // the first session's manifest (its t0 anchor) and conflate the two sessions' + // append-only streams and capture files. + if err := os.Mkdir(dir, 0o755); err != nil { return "", err } m.Session = filepath.Base(dir) @@ -173,7 +182,10 @@ func ReadJSONL[T any](path string) ([]T, error) { for sc.Scan() { line++ raw := sc.Bytes() - if len(raw) == 0 { + // Skip blank lines, including whitespace-only ones (as may appear in a + // hand-edited or exchanged session), matching analyze.Load so the two + // JSONL readers agree on what counts as blank. + if len(bytes.TrimSpace(raw)) == 0 { continue } var v T diff --git a/internal/session/session_test.go b/internal/session/session_test.go index bbd0cce..e86e520 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -48,6 +48,53 @@ func TestCreate(t *testing.T) { } } +// TestCreateRefusesSameSecondCollision is the conflated-session regression: two +// captures starting within the same wall-clock second derive the same +// second-granularity directory name. Create must not silently reuse the first +// session's directory (pre-fix os.MkdirAll returned it, clobbering the first +// manifest and conflating the two sessions' append-only streams). +func TestCreateRefusesSameSecondCollision(t *testing.T) { + root := t.TempDir() + now := time.Date(2026, 7, 17, 15, 30, 45, 0, time.UTC) + + if _, err := Create(root, now, Manifest{App: "first"}); err != nil { + t.Fatalf("first Create: %v", err) + } + dir2, err := Create(root, now, Manifest{App: "second"}) + if err == nil { + t.Fatalf("second Create reused existing dir %q; want refusal", dir2) + } + + // The first session's manifest must be intact, not overwritten by the second + // run's metadata. + m, err := LoadManifest(filepath.Join(root, now.Format(dirLayout))) + if err != nil { + t.Fatalf("LoadManifest: %v", err) + } + if m.App != "first" { + t.Fatalf("first manifest clobbered: App=%q, want %q", m.App, "first") + } +} + +// TestReadJSONLSkipsWhitespaceOnlyLine is the blank-line regression: ReadJSONL +// documents that blank lines are skipped, so a whitespace-only line (as may +// appear in a hand-edited or exchanged session) must be skipped rather than fed +// to json.Unmarshal (pre-fix it crashed with "unexpected end of JSON input"). +func TestReadJSONLSkipsWhitespaceOnlyLine(t *testing.T) { + path := filepath.Join(t.TempDir(), TimelineFile) + content := "{\"a\":1}\n \n\t\n{\"a\":2}\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + got, err := ReadJSONL[map[string]any](path) + if err != nil { + t.Fatalf("ReadJSONL: %v", err) + } + if len(got) != 2 { + t.Fatalf("got %d records, want 2 (whitespace lines skipped): %v", len(got), got) + } +} + // TestWriteJSONLRefusesSymlink is the arbitrary-file-overwrite regression: a // session artefact planted as a symlink (e.g. in a downloaded session) must not // be followed, so the write cannot escape the session directory. Pre-fix