From 7f0a7edf44b26a9322920cefa724809713a9aed4 Mon Sep 17 00:00:00 2001 From: Snider Date: Tue, 17 Mar 2026 08:49:46 +0000 Subject: [PATCH] fix(dx): audit and fix error handling, SPDX headers, coverage, and CLAUDE.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix coreerr.E() calls missing third arg in FetchSession and RenderMP4 - Add SPDX-Licence-Identifier headers to parser.go, html.go, video.go, search.go - Add tests for PruneSessions, IsExpired, and FetchSession (coverage 87.7% → 92.1%) - Document coreerr.E() error handling convention in CLAUDE.md - No fmt.Errorf or os.ReadFile/os.WriteFile violations found in source files Co-Authored-By: Virgil --- CLAUDE.md | 1 + html.go | 1 + parser.go | 3 +- parser_test.go | 121 +++++++++++++++++++++++++++++++++++++++++++++++++ search.go | 1 + video.go | 3 +- 6 files changed, 128 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e82d30f..59d2750 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,5 +45,6 @@ Coverage target: maintain ≥90.9%. - Explicit types on all function signatures and struct fields - `go test ./...` and `go vet ./...` must pass before commit - SPDX header on all source files: `// SPDX-Licence-Identifier: EUPL-1.2` +- Error handling: all errors must use `coreerr.E(op, msg, err)` from `forge.lthn.ai/core/go-log`, never `fmt.Errorf` or `errors.New` - Conventional commits: `type(scope): description` - Co-Author trailer: `Co-Authored-By: Virgil ` diff --git a/html.go b/html.go index 7113659..db36ac8 100644 --- a/html.go +++ b/html.go @@ -1,3 +1,4 @@ +// SPDX-Licence-Identifier: EUPL-1.2 package session import ( diff --git a/parser.go b/parser.go index 1390d92..dc877e4 100644 --- a/parser.go +++ b/parser.go @@ -1,3 +1,4 @@ +// SPDX-Licence-Identifier: EUPL-1.2 package session import ( @@ -238,7 +239,7 @@ func (s *Session) IsExpired(maxAge time.Duration) bool { // It ensures the ID does not contain path traversal characters. func FetchSession(projectsDir, id string) (*Session, *ParseStats, error) { if strings.Contains(id, "..") || strings.ContainsAny(id, `/\`) { - return nil, nil, coreerr.E("FetchSession", "invalid session id") + return nil, nil, coreerr.E("FetchSession", "invalid session id", nil) } path := filepath.Join(projectsDir, id+".jsonl") diff --git a/parser_test.go b/parser_test.go index 2f5c68d..41065bd 100644 --- a/parser_test.go +++ b/parser_test.go @@ -1311,6 +1311,127 @@ func TestParseTranscript_AllBadLines_Good(t *testing.T) { // --- ListSessions with truncated files --- +// --- PruneSessions tests --- + +func TestPruneSessions_DeletesOldFiles_Good(t *testing.T) { + dir := t.TempDir() + + // Create a session file with an old modification time. + path := writeJSONL(t, dir, "old-session.jsonl", + userTextEntry(ts(0), "old"), + ) + // Backdate the file's mtime by 2 hours. + oldTime := time.Now().Add(-2 * time.Hour) + require.NoError(t, os.Chtimes(path, oldTime, oldTime)) + + // Create a recent session file. + writeJSONL(t, dir, "new-session.jsonl", + userTextEntry(ts(0), "new"), + ) + + // Prune sessions older than 1 hour. + deleted, err := PruneSessions(dir, 1*time.Hour) + require.NoError(t, err) + assert.Equal(t, 1, deleted) + + // Verify only the new file remains. + sessions, err := ListSessions(dir) + require.NoError(t, err) + require.Len(t, sessions, 1) + assert.Equal(t, "new-session", sessions[0].ID) +} + +func TestPruneSessions_NothingToDelete_Good(t *testing.T) { + dir := t.TempDir() + + writeJSONL(t, dir, "recent.jsonl", + userTextEntry(ts(0), "fresh"), + ) + + deleted, err := PruneSessions(dir, 24*time.Hour) + require.NoError(t, err) + assert.Equal(t, 0, deleted) +} + +func TestPruneSessions_EmptyDir_Good(t *testing.T) { + dir := t.TempDir() + + deleted, err := PruneSessions(dir, 1*time.Hour) + require.NoError(t, err) + assert.Equal(t, 0, deleted) +} + +// --- IsExpired tests --- + +func TestIsExpired_RecentSession_Good(t *testing.T) { + sess := &Session{ + EndTime: time.Now().Add(-5 * time.Minute), + } + assert.False(t, sess.IsExpired(1*time.Hour)) +} + +func TestIsExpired_OldSession_Good(t *testing.T) { + sess := &Session{ + EndTime: time.Now().Add(-2 * time.Hour), + } + assert.True(t, sess.IsExpired(1*time.Hour)) +} + +func TestIsExpired_ZeroEndTime_Bad(t *testing.T) { + sess := &Session{} + assert.False(t, sess.IsExpired(1*time.Hour)) +} + +// --- FetchSession tests --- + +func TestFetchSession_ValidID_Good(t *testing.T) { + dir := t.TempDir() + writeJSONL(t, dir, "abc123.jsonl", + userTextEntry(ts(0), "hello"), + ) + + sess, stats, err := FetchSession(dir, "abc123") + require.NoError(t, err) + require.NotNil(t, sess) + require.NotNil(t, stats) + assert.Equal(t, "abc123", sess.ID) + assert.Len(t, sess.Events, 1) +} + +func TestFetchSession_PathTraversal_Ugly(t *testing.T) { + dir := t.TempDir() + + _, _, err := FetchSession(dir, "../etc/passwd") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid session id") +} + +func TestFetchSession_BackslashTraversal_Ugly(t *testing.T) { + dir := t.TempDir() + + _, _, err := FetchSession(dir, `foo\bar`) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid session id") +} + +func TestFetchSession_ForwardSlash_Ugly(t *testing.T) { + dir := t.TempDir() + + _, _, err := FetchSession(dir, "foo/bar") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid session id") +} + +func TestFetchSession_NotFound_Bad(t *testing.T) { + dir := t.TempDir() + + _, _, err := FetchSession(dir, "nonexistent") + require.Error(t, err) + assert.Contains(t, err.Error(), "open transcript") +} + +// --- ListSessions with truncated files --- + func TestListSessions_TruncatedFile_Good(t *testing.T) { dir := t.TempDir() // A .jsonl file where some lines are truncated — ListSessions should diff --git a/search.go b/search.go index 403428a..232446c 100644 --- a/search.go +++ b/search.go @@ -1,3 +1,4 @@ +// SPDX-Licence-Identifier: EUPL-1.2 package session import ( diff --git a/video.go b/video.go index 9b525b8..24839d8 100644 --- a/video.go +++ b/video.go @@ -1,3 +1,4 @@ +// SPDX-Licence-Identifier: EUPL-1.2 package session import ( @@ -12,7 +13,7 @@ import ( // RenderMP4 generates an MP4 video from session events using VHS (charmbracelet). func RenderMP4(sess *Session, outputPath string) error { if _, err := exec.LookPath("vhs"); err != nil { - return coreerr.E("RenderMP4", "vhs not installed (go install github.com/charmbracelet/vhs@latest)") + return coreerr.E("RenderMP4", "vhs not installed (go install github.com/charmbracelet/vhs@latest)", nil) } tape := generateTape(sess, outputPath)