Skip to content

Commit 23749f6

Browse files
SantiagoDePoloniaSantiagoDePolonia
committed
fix(tracking): tighten user-path fallback handling
1 parent 7d3979c commit 23749f6

4 files changed

Lines changed: 81 additions & 12 deletions

File tree

‎internal/executionplans/store_sqlite.go‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"encoding/json"
77
"errors"
88
"fmt"
9-
"strings"
109
"time"
1110

1211
"github.com/google/uuid"
@@ -51,19 +50,44 @@ func NewSQLiteStore(db *sql.DB) (*SQLiteStore, error) {
5150
return nil, fmt.Errorf("initialize execution plan versions table: %w", err)
5251
}
5352
}
54-
if _, err := db.Exec(`ALTER TABLE execution_plan_versions ADD COLUMN scope_user_path TEXT`); err != nil && !isSQLiteDuplicateColumnError(err) {
53+
hasUserPathColumn, err := sqliteTableHasColumn(db, "execution_plan_versions", "scope_user_path")
54+
if err != nil {
5555
return nil, fmt.Errorf("initialize execution plan versions table: %w", err)
5656
}
57+
if !hasUserPathColumn {
58+
if _, err := db.Exec(`ALTER TABLE execution_plan_versions ADD COLUMN scope_user_path TEXT`); err != nil {
59+
return nil, err
60+
}
61+
}
5762

5863
return &SQLiteStore{db: db}, nil
5964
}
6065

61-
func isSQLiteDuplicateColumnError(err error) bool {
62-
if err == nil {
63-
return false
66+
func sqliteTableHasColumn(db *sql.DB, tableName, columnName string) (bool, error) {
67+
rows, err := db.Query(fmt.Sprintf(`PRAGMA table_info('%s')`, tableName))
68+
if err != nil {
69+
return false, err
70+
}
71+
defer rows.Close()
72+
73+
for rows.Next() {
74+
var cid int
75+
var name string
76+
var columnType string
77+
var notNull int
78+
var defaultValue sql.NullString
79+
var primaryKey int
80+
if err := rows.Scan(&cid, &name, &columnType, &notNull, &defaultValue, &primaryKey); err != nil {
81+
return false, err
82+
}
83+
if name == columnName {
84+
return true, nil
85+
}
86+
}
87+
if err := rows.Err(); err != nil {
88+
return false, err
6489
}
65-
message := strings.ToLower(err.Error())
66-
return strings.Contains(message, "duplicate column") || strings.Contains(message, "already exists")
90+
return false, nil
6791
}
6892

6993
func (s *SQLiteStore) ListActive(ctx context.Context) ([]Version, error) {

‎internal/executionplans/store_sqlite_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,48 @@ func TestNewSQLiteStore_SkipsExistingScopeUserPathMigration(t *testing.T) {
4444
t.Fatal("NewSQLiteStore() = nil, want store")
4545
}
4646
}
47+
48+
func TestNewSQLiteStore_AddsMissingScopeUserPathColumn(t *testing.T) {
49+
t.Parallel()
50+
51+
db, err := sql.Open("sqlite", ":memory:")
52+
if err != nil {
53+
t.Fatalf("sql.Open() error = %v", err)
54+
}
55+
defer db.Close()
56+
57+
_, err = db.Exec(`
58+
CREATE TABLE execution_plan_versions (
59+
id TEXT PRIMARY KEY,
60+
scope_provider TEXT,
61+
scope_model TEXT,
62+
scope_key TEXT NOT NULL,
63+
version INTEGER NOT NULL,
64+
active INTEGER NOT NULL DEFAULT 1,
65+
name TEXT NOT NULL,
66+
description TEXT NOT NULL DEFAULT '',
67+
plan_payload JSON NOT NULL,
68+
plan_hash TEXT NOT NULL,
69+
created_at INTEGER NOT NULL
70+
)
71+
`)
72+
if err != nil {
73+
t.Fatalf("create execution_plan_versions table: %v", err)
74+
}
75+
76+
store, err := NewSQLiteStore(db)
77+
if err != nil {
78+
t.Fatalf("NewSQLiteStore() error = %v", err)
79+
}
80+
if store == nil {
81+
t.Fatal("NewSQLiteStore() = nil, want store")
82+
}
83+
84+
hasColumn, err := sqliteTableHasColumn(db, "execution_plan_versions", "scope_user_path")
85+
if err != nil {
86+
t.Fatalf("sqliteTableHasColumn() error = %v", err)
87+
}
88+
if !hasColumn {
89+
t.Fatal("scope_user_path column missing after initialization")
90+
}
91+
}

‎internal/usage/stream_observer.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ func NewStreamUsageObserver(logger LoggerInterface, model, provider, requestID,
2828
if normalized, err := core.NormalizeUserPath(userPath[0]); err == nil {
2929
normalizedUserPath = normalized
3030
} else {
31-
slog.Warn("stream usage observer received invalid user_path; preserving raw value", "user_path", userPath[0], "error", err)
32-
normalizedUserPath = userPath[0]
31+
slog.Warn("stream usage observer received invalid user_path; using root fallback", "error", err)
32+
normalizedUserPath = "/"
3333
}
3434
}
3535
return &StreamUsageObserver{

‎internal/usage/stream_observer_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ func TestStreamUsageObserverNormalizesUserPath(t *testing.T) {
192192
}
193193
}
194194

195-
func TestStreamUsageObserverPreservesInvalidUserPath(t *testing.T) {
195+
func TestStreamUsageObserverFallsBackToRootForInvalidUserPath(t *testing.T) {
196196
logger := &trackingLogger{enabled: true}
197197
observer := NewStreamUsageObserver(logger, "gpt-4", "openai", "req-123", "/v1/chat/completions", nil, "/team/../alpha")
198198
observer.OnJSONEvent(map[string]any{
@@ -209,8 +209,8 @@ func TestStreamUsageObserverPreservesInvalidUserPath(t *testing.T) {
209209
if len(entries) != 1 {
210210
t.Fatalf("expected 1 entry, got %d", len(entries))
211211
}
212-
if got := entries[0].UserPath; got != "/team/../alpha" {
213-
t.Fatalf("UserPath = %q, want /team/../alpha", got)
212+
if got := entries[0].UserPath; got != "/" {
213+
t.Fatalf("UserPath = %q, want /", got)
214214
}
215215
}
216216

0 commit comments

Comments
 (0)