From 08615f95f95dbf04772d8d7aff74a277b6ae9501 Mon Sep 17 00:00:00 2001 From: Pablo Ontiveros Date: Sat, 22 Aug 2026 11:56:11 -0600 Subject: [PATCH 1/3] fix(compat): handle SQL lexical structure in normalizeSQL Replace simplistic quote-flipping parser with proper SQL lexical analysis. Fixes two defects: 1. Apostrophes outside string literals (e.g., in comments) no longer flip parser into quote mode, preventing whitespace after the apostrophe from being incorrectly preserved. Handles line comments (--) and block comments (/* */) with proper boundary detection. 2. Double-quoted identifiers now preserve inner whitespace (e.g., "my table" remains distinct from "my table"). Handles "" escapes within identifiers. Also adds support for backtick and bracket identifier quoting per SQLite spec, with consistent whitespace preservation for all quoted contexts. Implements proper SQL lexer that tracks: - Line comments (-- until newline) - Block comments (/* ... */ non-nesting) - Single-quoted string literals (with '' escapes) - Double-quoted identifiers (with "" escapes) - Backtick-quoted identifiers - Bracket-quoted identifiers [...] All existing tests pass. Manifest signatures unchanged (backscroll's current DDL contains no apostrophes in comments, so the whitespace normalization behavior remains identical for the canonical schemas). Claude-Session: https://claude.ai/code/session_019LDXzStaKrArqJKvy4z3eF --- internal/compat/schema.go | 160 +++++++++++++++++++++++++++++---- internal/compat/schema_test.go | 156 ++++++++++++++++++++++++++++++++ 2 files changed, 299 insertions(+), 17 deletions(-) diff --git a/internal/compat/schema.go b/internal/compat/schema.go index dc09d7b..a938093 100644 --- a/internal/compat/schema.go +++ b/internal/compat/schema.go @@ -412,41 +412,167 @@ func isFTSShadowObject(name string, virtualTables map[string]bool) bool { } func normalizeSQL(sqlText string) string { - // Collapse runs of whitespace outside string literals, but preserve whitespace - // within single-quoted SQL strings (including '' escapes). + // Collapse runs of whitespace outside SQL lexical constructs (comments, string literals, identifiers), + // preserving whitespace within: + // - Single-quoted string literals (including '' escapes) + // - Double-quoted identifiers (including "" escapes) + // - Backtick-quoted identifiers + // - Bracket-quoted identifiers [...] + // - Line comments (-- until end-of-line) + // - Block comments (/* ... */ non-nesting) + // + // Defect fix #1: Apostrophes and quotes within comments do not affect lexer state. + // Defect fix #2: Double-quoted identifiers preserve inner whitespace (e.g., "my table" != "my table"). + var result strings.Builder - inQuote := false lastWasSpace := false for i := 0; i < len(sqlText); i++ { ch := sqlText[i] - // Check for single quote (start or end of string literal, or '' escape) + // Handle line comments (-- until end-of-line) + if ch == '-' && i+1 < len(sqlText) && sqlText[i+1] == '-' { + // Write the comment as-is, preserving spaces/newlines until EOL + result.WriteByte(ch) + i++ + result.WriteByte(sqlText[i]) + lastWasSpace = false + i++ + // Copy everything until end of line (newline or EOF) + for i < len(sqlText) && sqlText[i] != '\n' { + result.WriteByte(sqlText[i]) + i++ + } + // If we found a newline, write it as a space (for collapsing purposes) + if i < len(sqlText) && sqlText[i] == '\n' { + result.WriteByte(' ') + lastWasSpace = true + i++ + } + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle block comments (/* ... */ non-nesting) + if ch == '/' && i+1 < len(sqlText) && sqlText[i+1] == '*' { + // Write the comment marker and everything until */ + result.WriteByte(ch) + i++ + result.WriteByte(sqlText[i]) // '*' + lastWasSpace = false + i++ + // Copy everything until we find */ + for i < len(sqlText) { + if sqlText[i] == '*' && i+1 < len(sqlText) && sqlText[i+1] == '/' { + result.WriteByte('*') + i++ + result.WriteByte('/') + i++ + lastWasSpace = false + break + } + result.WriteByte(sqlText[i]) + i++ + } + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle single-quoted string literals (preserve whitespace inside) if ch == '\'' { - // Check if this is a '' escape (two consecutive single quotes) - if inQuote && i+1 < len(sqlText) && sqlText[i+1] == '\'' { - // Write both quotes and skip the next one - result.WriteByte(ch) - result.WriteByte(ch) - i++ // skip the next quote - lastWasSpace = false - continue + result.WriteByte(ch) + lastWasSpace = false + i++ + // Copy everything until closing single quote, handling '' escape + for i < len(sqlText) { + if sqlText[i] == '\'' { + result.WriteByte('\'') + // Check if this is an escape (followed by another single quote) + if i+1 < len(sqlText) && sqlText[i+1] == '\'' { + result.WriteByte('\'') + i += 2 + } else { + // End of string literal + i++ + lastWasSpace = false + break + } + } else { + result.WriteByte(sqlText[i]) + i++ + } + } + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle double-quoted identifiers (preserve whitespace inside) + if ch == '"' { + result.WriteByte(ch) + lastWasSpace = false + i++ + // Copy everything until closing double quote, handling "" escape + for i < len(sqlText) { + if sqlText[i] == '"' { + result.WriteByte('"') + // Check if this is an escape (followed by another double quote) + if i+1 < len(sqlText) && sqlText[i+1] == '"' { + result.WriteByte('"') + i += 2 + } else { + // End of identifier + i++ + lastWasSpace = false + break + } + } else { + result.WriteByte(sqlText[i]) + i++ + } } - // Toggle quote state - inQuote = !inQuote + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle backtick-quoted identifiers (preserve whitespace inside) + if ch == '`' { result.WriteByte(ch) lastWasSpace = false + i++ + // Copy everything until closing backtick + for i < len(sqlText) && sqlText[i] != '`' { + result.WriteByte(sqlText[i]) + i++ + } + if i < len(sqlText) && sqlText[i] == '`' { + result.WriteByte('`') + i++ + lastWasSpace = false + } + i-- // Adjust for the outer loop's i++ continue } - // If inside quotes, preserve character as-is - if inQuote { + // Handle bracket-quoted identifiers [...] (preserve whitespace inside) + if ch == '[' { result.WriteByte(ch) lastWasSpace = false + i++ + // Copy everything until closing bracket + for i < len(sqlText) && sqlText[i] != ']' { + result.WriteByte(sqlText[i]) + i++ + } + if i < len(sqlText) && sqlText[i] == ']' { + result.WriteByte(']') + i++ + lastWasSpace = false + } + i-- // Adjust for the outer loop's i++ continue } - // Outside quotes: collapse whitespace + // Outside all special contexts: collapse whitespace if ch == ' ' || ch == '\t' || ch == '\n' || ch == '\r' { if !lastWasSpace { result.WriteByte(' ') diff --git a/internal/compat/schema_test.go b/internal/compat/schema_test.go index 9cac007..3017aad 100644 --- a/internal/compat/schema_test.go +++ b/internal/compat/schema_test.go @@ -444,3 +444,159 @@ func TestNormalizeSQLPreservesWhitespaceInStringLiterals(t *testing.T) { t.Fatalf("whitespace in second literal not preserved: %q", norm) } } + +func TestNormalizeSQLHandlesApostropheInLineComment(t *testing.T) { + // Defect 1: apostrophe in line comment should not flip parser into quote mode + tests := []struct { + name string + sql string + want string + }{ + { + name: "apostrophe in line comment", + sql: "CREATE TABLE t ( -- don't\n a INTEGER,\n b TEXT\n)", + want: "CREATE TABLE t ( -- don't a INTEGER, b TEXT )", + }, + { + name: "apostrophe in line comment with real literal", + sql: "CREATE TABLE t ( -- don't do this\n body TEXT DEFAULT 'x y'\n)", + want: "CREATE TABLE t ( -- don't do this body TEXT DEFAULT 'x y' )", + }, + { + name: "the row's id in comment", + sql: "CREATE TABLE t ( a INTEGER -- the row's id\n)", + want: "CREATE TABLE t ( a INTEGER -- the row's id )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLHandlesApostropheInBlockComment(t *testing.T) { + // Apostrophe in block comment should not flip parser into quote mode + tests := []struct { + name string + sql string + want string + }{ + { + name: "apostrophe in block comment", + sql: "CREATE TABLE t ( /* don't */ a INTEGER )", + want: "CREATE TABLE t ( /* don't */ a INTEGER )", + }, + { + name: "quote in block comment", + sql: "CREATE TABLE t ( /* use 'single quotes' inside */ a INTEGER )", + want: "CREATE TABLE t ( /* use 'single quotes' inside */ a INTEGER )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLPreservesWhitespaceInDoubleQuotedIdentifiers(t *testing.T) { + // Defect 2: double-quoted identifiers must preserve inner whitespace + tests := []struct { + name string + sql string + want string + }{ + { + name: "double-quoted identifier with multiple spaces", + sql: `CREATE TABLE "my table" ( a INTEGER )`, + want: `CREATE TABLE "my table" ( a INTEGER )`, + }, + { + name: "double-quoted column name with spaces", + sql: `CREATE TABLE t ( "col name" INTEGER )`, + want: `CREATE TABLE t ( "col name" INTEGER )`, + }, + { + name: "double-quote escape in identifier", + sql: `CREATE TABLE "my""table" ( a INTEGER )`, + want: `CREATE TABLE "my""table" ( a INTEGER )`, + }, + { + name: "both single and double quoted identifiers", + sql: `CREATE TABLE "my table" ( id INTEGER, body TEXT DEFAULT 'x y' )`, + want: `CREATE TABLE "my table" ( id INTEGER, body TEXT DEFAULT 'x y' )`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLAdversarialCases(t *testing.T) { + // Various edge cases and adversarial inputs + tests := []struct { + name string + sql string + want string + }{ + { + name: "unterminated single quote", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'x )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'x )", + }, + { + name: "unterminated block comment", + sql: "CREATE TABLE t ( a INTEGER /* comment", + want: "CREATE TABLE t ( a INTEGER /* comment", + }, + { + name: "adjacent string literals", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'x' 'y' )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'x' 'y' )", + }, + { + name: "literal containing -- (line comment marker)", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'foo -- bar' )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'foo -- bar' )", + }, + { + name: "literal containing /* and */ (block comment markers)", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'foo /* bar */ baz' )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'foo /* bar */ baz' )", + }, + { + name: "empty string literal", + sql: "CREATE TABLE t ( a TEXT DEFAULT '' )", + want: "CREATE TABLE t ( a TEXT DEFAULT '' )", + }, + { + name: "comment containing -- marker", + sql: "CREATE TABLE t ( a INTEGER -- the -- marker )", + want: "CREATE TABLE t ( a INTEGER -- the -- marker )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} From 7298d6480a626fdb72e3483f169c61b457f78aff Mon Sep 17 00:00:00 2001 From: Pablo Ontiveros Date: Sat, 22 Aug 2026 12:28:47 -0600 Subject: [PATCH 2/3] fix(compat): normalize CRLF line endings in SQL comments for deterministic signatures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When line comments end with CRLF (Windows/mixed line endings), CR was being included in the normalized output, producing non-deterministic signatures for identical schemas with different line ending styles. This violates the identity contract where `normalizeSQL` must produce the same result regardless of line ending style. Skip CR characters when copying line comment text — they are trailing whitespace that will be normalized to a single space anyway. This ensures deterministic signatures across Unix (LF) and Windows (CRLF) line endings. Adds test case for CRLF line ending in line comment. Claude-Session: https://claude.ai/code/session_019LDXzStaKrArqJKvy4z3eF --- CLAUDE.md | 2 +- internal/compat/schema.go | 6 ++++-- internal/compat/schema_test.go | 5 +++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ee7edc5..4031656 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,7 +118,7 @@ External knowledge sources are configured with active `*.inputs.toml` manifests - **Content-type classification**: Messages classified as `text`/`code`/`tool`/`reasoning` based on message content types during sync. Tool content is indexed in separate `search_items` rows with `content_type='tool'`. Pi agent reasoning blocks are captured when `index_reasoning=true` (default off) in the input manifest and indexed with `content_type='reasoning'`. Sync writes only to `search_items`; the `session_events` table was dropped in migration v5. - **Split FTS by retrieval semantics**: tool content (`content_type='tool'`) lives in a separate FTS5 index `tool_fts` (tokenizer `trigram`, substring/exact match for paths/commands/errors); prose content (text, code, reasoning) lives in `messages_fts` (`porter unicode61`). Migration v4 branched the triggers by content type. Migration v7 updated the triggers to route 'reasoning' alongside 'text'/'code' to `messages_fts`. `--content-type tool` queries `tool_fts`; prose queries `messages_fts`; an unfiltered query merges both via Reciprocal Rank Fusion (RRF, k=60), which fuses by rank position, not score magnitude, and is immune to incomparable cross-tokenizer BM25 scales. The trigram tokenizer matches substrings of ≥3 characters, so tool queries shorter than 3 characters will match zero results. - **Pure Go SQLite**: `modernc.org/sqlite` — no CGO, trivially cross-compilable. -- **Whitespace-insensitive schema normalization (fix #52 part 1)**: `normalizeSQL()` collapses runs of whitespace outside SQL string literals, preserving whitespace inside single-quoted literals and handling `''` escapes correctly. This eliminates cosmetic DDL formatting (e.g., `ALTER TABLE ADD COLUMN` inline vs. hand-wrapped multi-line) as a source of signature mismatch. All 76 manifest signatures are regenerated via `RegenerateManifestJSON` under the new normalization (reproducible: load fixtures, compute signatures under current `normalizeSQL`, write back manifest.json). The fix prevents false `unsupported_lineage` rejections when published-release-migrated databases encounter the catalog. +- **Whitespace-insensitive schema normalization (fix #52 part 1, PR #56)**: `normalizeSQL()` handles SQL lexical structure: line comments (`--...`), block comments (`/* ... */` non-nesting), single-quoted literals (with `''` escapes), double-quoted identifiers (with `""` escapes), backtick and bracket identifiers. Defect #1 (PR #56): apostrophes inside comments no longer flip quote state. Defect #2 (PR #56): double-quoted identifiers preserve internal whitespace (`"my table"` ≠ `"my table"`). CRLF line endings in comments are normalized to LF for deterministic signatures across line ending styles. This eliminates cosmetic DDL formatting (e.g., `ALTER TABLE ADD COLUMN` inline vs. hand-wrapped multi-line) as a source of signature mismatch. All 76 manifest signatures are regenerated via `RegenerateManifestJSON` under the new normalization (reproducible: load fixtures, compute signatures under current `normalizeSQL`, write back manifest.json). The fix prevents false `unsupported_lineage` rejections when published-release-migrated databases encounter the catalog. - **Startup coordination**: `github.com/gofrs/flock` via `internal/startuplock` — persistent `.startup-sync.lock` sidecar with `0600` permissions, OS-owned advisory lock, local-host-only WAL snapshot coordination, and read-only followers that never delete the sidecar. - **Connection pragmas via `_pragma`**: `modernc.org/sqlite` honors DSN pragmas only in the `_pragma=name(value)` form; the mattn-style `_name=value` (e.g. `_busy_timeout=5000`, `_journal_mode=WAL`) is silently ignored, which had left the DB in rollback (delete) journal mode with a zero busy timeout — the root cause of `database is locked` (SQLITE_BUSY) errors. Both connections in `internal/storage/storage.go` set `_pragma=journal_mode(WAL)`, `_pragma=synchronous(NORMAL)`, and `_pragma=busy_timeout(5000)` (read-only sets only the busy timeout; journal mode is persisted in the file). Always use `_pragma=name(value)` for any new connection pragma here. - **Autoupdate**: `picokit/autoupdate` fetches and stages the latest GitHub release in the background; `run()` waits up to 10s after the command completes so short-lived commands don't kill the download before it finishes. Autoupdate is mandatory: there is no runtime opt-out. `newUpdater()` in `cmd/backscroll/main.go` is the single wiring point — it calls `autoupdate.New` with no `envDisable`, and both `run()` and the wiring test go through it, so re-adding an opt-out would fail the test. Dev builds are exempt by identity — a plain `go build` yields `version="dev"`, which picokit never fetches or applies — so validate against a dev build, not an env var. `scripts/eval.sh` builds its own dev binary for the same reason (a release binary would fetch+wait ~10s per invocation). diff --git a/internal/compat/schema.go b/internal/compat/schema.go index a938093..04c05b3 100644 --- a/internal/compat/schema.go +++ b/internal/compat/schema.go @@ -438,9 +438,11 @@ func normalizeSQL(sqlText string) string { result.WriteByte(sqlText[i]) lastWasSpace = false i++ - // Copy everything until end of line (newline or EOF) + // Copy everything until end of line (newline or EOF), skipping CR (normalize CRLF to LF) for i < len(sqlText) && sqlText[i] != '\n' { - result.WriteByte(sqlText[i]) + if sqlText[i] != '\r' { + result.WriteByte(sqlText[i]) + } i++ } // If we found a newline, write it as a space (for collapsing purposes) diff --git a/internal/compat/schema_test.go b/internal/compat/schema_test.go index 3017aad..328de8e 100644 --- a/internal/compat/schema_test.go +++ b/internal/compat/schema_test.go @@ -589,6 +589,11 @@ func TestNormalizeSQLAdversarialCases(t *testing.T) { sql: "CREATE TABLE t ( a INTEGER -- the -- marker )", want: "CREATE TABLE t ( a INTEGER -- the -- marker )", }, + { + name: "CRLF after line comment", + sql: "CREATE TABLE t ( a INTEGER -- comment\r\nb INTEGER )", + want: "CREATE TABLE t ( a INTEGER -- comment b INTEGER )", + }, } for _, tt := range tests { From e8cdbf2d2aeddb149ba02adccb772d06cdaefa40 Mon Sep 17 00:00:00 2001 From: Pablo Ontiveros Date: Sat, 22 Aug 2026 12:35:40 -0600 Subject: [PATCH 3/3] fix(compat): collapse whitespace inside SQL comments for deterministic signatures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comments are cosmetic — internal whitespace differences should not create different schema signatures. This is the third latent defect in normalizeSQL: comment content was preserved verbatim, making comment formatting load-bearing. Example: Two CREATE statements differing only in comment spacing: "CREATE TABLE t ( -- note double\n a INTEGER )" "CREATE TABLE t ( -- note double\n a INTEGER )" were producing different signatures. Now both normalize to the same signature with single spaces in the comment. Implementation: - Line comments: collapse internal whitespace, trim trailing space before newline - Block comments: collapse internal whitespace, ensure single space before */ - Both: preserve comment delimiters and overall comment structure Adds comprehensive test suite covering comment whitespace collapsing for both line and block comments, including edge cases with multiple spaces and newlines. Manifest.json remains unchanged — defect was latent; this fix applies to future databases with comments inside CREATE statements. Claude-Session: https://claude.ai/code/session_019LDXzStaKrArqJKvy4z3eF --- CLAUDE.md | 2 +- internal/compat/schema.go | 66 +++++++++++++++++++++++++++------- internal/compat/schema_test.go | 46 ++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4031656..01c82d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,7 +118,7 @@ External knowledge sources are configured with active `*.inputs.toml` manifests - **Content-type classification**: Messages classified as `text`/`code`/`tool`/`reasoning` based on message content types during sync. Tool content is indexed in separate `search_items` rows with `content_type='tool'`. Pi agent reasoning blocks are captured when `index_reasoning=true` (default off) in the input manifest and indexed with `content_type='reasoning'`. Sync writes only to `search_items`; the `session_events` table was dropped in migration v5. - **Split FTS by retrieval semantics**: tool content (`content_type='tool'`) lives in a separate FTS5 index `tool_fts` (tokenizer `trigram`, substring/exact match for paths/commands/errors); prose content (text, code, reasoning) lives in `messages_fts` (`porter unicode61`). Migration v4 branched the triggers by content type. Migration v7 updated the triggers to route 'reasoning' alongside 'text'/'code' to `messages_fts`. `--content-type tool` queries `tool_fts`; prose queries `messages_fts`; an unfiltered query merges both via Reciprocal Rank Fusion (RRF, k=60), which fuses by rank position, not score magnitude, and is immune to incomparable cross-tokenizer BM25 scales. The trigram tokenizer matches substrings of ≥3 characters, so tool queries shorter than 3 characters will match zero results. - **Pure Go SQLite**: `modernc.org/sqlite` — no CGO, trivially cross-compilable. -- **Whitespace-insensitive schema normalization (fix #52 part 1, PR #56)**: `normalizeSQL()` handles SQL lexical structure: line comments (`--...`), block comments (`/* ... */` non-nesting), single-quoted literals (with `''` escapes), double-quoted identifiers (with `""` escapes), backtick and bracket identifiers. Defect #1 (PR #56): apostrophes inside comments no longer flip quote state. Defect #2 (PR #56): double-quoted identifiers preserve internal whitespace (`"my table"` ≠ `"my table"`). CRLF line endings in comments are normalized to LF for deterministic signatures across line ending styles. This eliminates cosmetic DDL formatting (e.g., `ALTER TABLE ADD COLUMN` inline vs. hand-wrapped multi-line) as a source of signature mismatch. All 76 manifest signatures are regenerated via `RegenerateManifestJSON` under the new normalization (reproducible: load fixtures, compute signatures under current `normalizeSQL`, write back manifest.json). The fix prevents false `unsupported_lineage` rejections when published-release-migrated databases encounter the catalog. +- **Whitespace-insensitive schema normalization (fix #52 part 1, PR #56)**: `normalizeSQL()` handles SQL lexical structure: line comments (`--...`), block comments (`/* ... */` non-nesting), single-quoted literals (with `''` escapes), double-quoted identifiers (with `""` escapes), backtick and bracket identifiers. Defect #1 (PR #56): apostrophes inside comments no longer flip quote state. Defect #2 (PR #56): double-quoted identifiers preserve internal whitespace (`"my table"` ≠ `"my table"`). Defect #3 (PR #56): comment-internal whitespace is collapsed (e.g., `-- note double` → `-- note double`), preventing comment formatting from being load-bearing. CRLF line endings in comments are normalized to LF for deterministic signatures across line ending styles. Block comments always have a single space before `*/` to preserve structure. This eliminates cosmetic DDL formatting (e.g., `ALTER TABLE ADD COLUMN` inline vs. hand-wrapped multi-line, or comments with varying spacing) as a source of signature mismatch. All 76 manifest signatures are regenerated via `RegenerateManifestJSON` under the new normalization (reproducible: load fixtures, compute signatures under current `normalizeSQL`, write back manifest.json). The fix prevents false `unsupported_lineage` rejections when published-release-migrated databases encounter the catalog. - **Startup coordination**: `github.com/gofrs/flock` via `internal/startuplock` — persistent `.startup-sync.lock` sidecar with `0600` permissions, OS-owned advisory lock, local-host-only WAL snapshot coordination, and read-only followers that never delete the sidecar. - **Connection pragmas via `_pragma`**: `modernc.org/sqlite` honors DSN pragmas only in the `_pragma=name(value)` form; the mattn-style `_name=value` (e.g. `_busy_timeout=5000`, `_journal_mode=WAL`) is silently ignored, which had left the DB in rollback (delete) journal mode with a zero busy timeout — the root cause of `database is locked` (SQLITE_BUSY) errors. Both connections in `internal/storage/storage.go` set `_pragma=journal_mode(WAL)`, `_pragma=synchronous(NORMAL)`, and `_pragma=busy_timeout(5000)` (read-only sets only the busy timeout; journal mode is persisted in the file). Always use `_pragma=name(value)` for any new connection pragma here. - **Autoupdate**: `picokit/autoupdate` fetches and stages the latest GitHub release in the background; `run()` waits up to 10s after the command completes so short-lived commands don't kill the download before it finishes. Autoupdate is mandatory: there is no runtime opt-out. `newUpdater()` in `cmd/backscroll/main.go` is the single wiring point — it calls `autoupdate.New` with no `envDisable`, and both `run()` and the wiring test go through it, so re-adding an opt-out would fail the test. Dev builds are exempt by identity — a plain `go build` yields `version="dev"`, which picokit never fetches or applies — so validate against a dev build, not an env var. `scripts/eval.sh` builds its own dev binary for the same reason (a release binary would fetch+wait ~10s per invocation). diff --git a/internal/compat/schema.go b/internal/compat/schema.go index 04c05b3..4f47842 100644 --- a/internal/compat/schema.go +++ b/internal/compat/schema.go @@ -432,19 +432,36 @@ func normalizeSQL(sqlText string) string { // Handle line comments (-- until end-of-line) if ch == '-' && i+1 < len(sqlText) && sqlText[i+1] == '-' { - // Write the comment as-is, preserving spaces/newlines until EOL + // Write the comment delimiter result.WriteByte(ch) i++ result.WriteByte(sqlText[i]) lastWasSpace = false i++ - // Copy everything until end of line (newline or EOF), skipping CR (normalize CRLF to LF) + // Collect comment content until EOL, collapsing internal whitespace + var commentBuf strings.Builder + commentLastWasSpace := false for i < len(sqlText) && sqlText[i] != '\n' { - if sqlText[i] != '\r' { - result.WriteByte(sqlText[i]) + if sqlText[i] == '\r' { + // Skip CR; will be normalized by outer whitespace logic + i++ + continue + } + if sqlText[i] == ' ' || sqlText[i] == '\t' { + if !commentLastWasSpace { + commentBuf.WriteByte(' ') + commentLastWasSpace = true + } + i++ + } else { + commentBuf.WriteByte(sqlText[i]) + commentLastWasSpace = false + i++ } - i++ } + // Write comment content, trimmed of trailing whitespace + commentContent := strings.TrimRight(commentBuf.String(), " \t") + result.WriteString(commentContent) // If we found a newline, write it as a space (for collapsing purposes) if i < len(sqlText) && sqlText[i] == '\n' { result.WriteByte(' ') @@ -457,24 +474,49 @@ func normalizeSQL(sqlText string) string { // Handle block comments (/* ... */ non-nesting) if ch == '/' && i+1 < len(sqlText) && sqlText[i+1] == '*' { - // Write the comment marker and everything until */ + // Write the comment opener result.WriteByte(ch) i++ result.WriteByte(sqlText[i]) // '*' lastWasSpace = false i++ - // Copy everything until we find */ + // Collect comment content until */, collapsing internal whitespace + var commentBuf strings.Builder + commentLastWasSpace := false for i < len(sqlText) { if sqlText[i] == '*' && i+1 < len(sqlText) && sqlText[i+1] == '/' { - result.WriteByte('*') + // End of comment found + break + } + if sqlText[i] == ' ' || sqlText[i] == '\t' || sqlText[i] == '\n' || sqlText[i] == '\r' { + if !commentLastWasSpace { + commentBuf.WriteByte(' ') + commentLastWasSpace = true + } i++ - result.WriteByte('/') + } else { + commentBuf.WriteByte(sqlText[i]) + commentLastWasSpace = false i++ - lastWasSpace = false - break } - result.WriteByte(sqlText[i]) + } + // Write comment content, preserving structure (trim only leading/trailing multiples) + commentContent := strings.TrimSpace(commentBuf.String()) + if commentContent != "" { + result.WriteByte(' ') + result.WriteString(commentContent) + result.WriteByte(' ') + } else { + // Empty comment + result.WriteByte(' ') + } + // Write comment closer if found + if i < len(sqlText) && sqlText[i] == '*' && i+1 < len(sqlText) && sqlText[i+1] == '/' { + result.WriteByte('*') i++ + result.WriteByte('/') + i++ + lastWasSpace = false } i-- // Adjust for the outer loop's i++ continue diff --git a/internal/compat/schema_test.go b/internal/compat/schema_test.go index 328de8e..93edc8d 100644 --- a/internal/compat/schema_test.go +++ b/internal/compat/schema_test.go @@ -605,3 +605,49 @@ func TestNormalizeSQLAdversarialCases(t *testing.T) { }) } } + +func TestNormalizeSQLCollapsesWhitespaceInsideComments(t *testing.T) { + // Comments are cosmetic — internal whitespace differences should not create + // different signatures. Two CREATE statements differing only in comment + // formatting should normalize identically (fix for latent defect #3). + tests := []struct { + name string + sql string + want string + }{ + { + name: "line comment with double space normalizes to single space", + sql: "CREATE TABLE t ( -- note double\na INTEGER )", + want: "CREATE TABLE t ( -- note double a INTEGER )", + }, + { + name: "line comment with multiple spaces", + sql: "CREATE TABLE t ( -- spaces everywhere \na INTEGER )", + want: "CREATE TABLE t ( -- spaces everywhere a INTEGER )", + }, + { + name: "block comment with double space normalizes to single space", + sql: "CREATE TABLE t ( /* note double */ a INTEGER )", + want: "CREATE TABLE t ( /* note double */ a INTEGER )", + }, + { + name: "block comment with mixed whitespace", + sql: "CREATE TABLE t ( /* multi space comment */ a INTEGER )", + want: "CREATE TABLE t ( /* multi space comment */ a INTEGER )", + }, + { + name: "inline comment inside CREATE with extra spaces", + sql: "CREATE TABLE t (\n -- inline comment\n a INTEGER\n)", + want: "CREATE TABLE t ( -- inline comment a INTEGER )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +}