internal/auditpush at de0d9860ee, reviewed by gemini-3.6-flash, verified by codex.
writeLedgerFile unconditionally populates Scan.PushedBy on empty ScanRow structs, creating phantom corral_scans rows for bundles without scan headers.
cat << 'EOF' > test_phantom_scan.go
package main
import (
"fmt"
"os"
"github.com/pdbethke/corralai/internal/auditpush"
_ "github.com/marcboeker/go-duckdb/v2"
)
func main() {
dir, err := os.MkdirTemp("", "ledger_test_*")
if err != nil {
panic(err)
}
defer os.RemoveAll(dir)
b := auditpush.Bundle{
Files: []auditpush.Row{
{Path: "pkg/a.go", Lang: "go"},
},
}
counts, err := auditpush.PushBundle(dir, b)
if err != nil {
fmt.Printf("PushBundle failed: %v\n", err)
os.Exit(1)
}
db, err := auditpush.LoadDir(dir)
if err != nil {
fmt.Printf("LoadDir failed: %v\n", err)
os.Exit(1)
}
defer db.Close()
var scansCount int
if err := db.QueryRow("SELECT count(*) FROM corral_scans").Scan(&scansCount); err != nil {
fmt.Printf("Query corral_scans failed: %v\n", err)
os.Exit(1)
}
if counts.Scans != 0 || scansCount != 0 {
fmt.Printf("Phantom scan created: PushBundle Scans count=%d, LoadDir corral_scans count=%d (expected 0)\n", counts.Scans, scansCount)
os.Exit(0)
}
os.Exit(1)
}
EOF
go run test_phantom_scan.goPhantom scan created: PushBundle Scans count=1, LoadDir corral_scans count=1 (expected 0)
I tried to find a validity guard or later normalization that would preserve the zero ScanRow. PushBundle only returns early when every grain is empty (bundle.go:804), so a files-only bundle reaches writeLedgerFile. That function sets PushedBy before testing whether Scan is zero (ledgerdir.go:306-316), making the comparison true and reporting one scan. The database insertion path instead checks the untouched zero ScanRow and inserts no scan row (bundle.go:1223). Thus the claimed phantom row is not narrower than stated.
confirmed by pdbethke — real, reproduced: writeLedgerFile stamps PushedBy on an empty ScanRow, minting a phantom corral_scans row
ref e00b52bab444de23726c8ede78bc303c094516955f2e48268c90b596bd8540ba#R1
migrateTable hardcodes database_name = 'warehouse' when probing columns, causing EnsureSchema to fail on standard DuckDB database handles.
cat << 'EOF' > test_ensure_schema.go
package main
import (
"database/sql"
"fmt"
"os"
"path/filepath"
"github.com/pdbethke/corralai/internal/auditpush"
_ "github.com/marcboeker/go-duckdb/v2"
)
func main() {
tmp := filepath.Join(os.TempDir(), "test_schema.duckdb")
defer os.Remove(tmp)
b := auditpush.Bundle{
Scan: auditpush.ScanRow{Repo: "test/repo", ScanID: 1},
}
if _, err := auditpush.PushBundle(tmp, b); err != nil {
panic(err)
}
dbDirect, err := sql.Open("duckdb", tmp)
if err != nil {
panic(err)
}
defer dbDirect.Close()
err = auditpush.EnsureSchema(dbDirect)
if err != nil {
fmt.Printf("EnsureSchema failed on direct db handle: %v\n", err)
os.Exit(0)
}
os.Exit(1)
}
EOF
go run test_ensure_schema.goEnsureSchema failed on direct db handle: auditpush: migrate corral_scans: add column started_at: Catalog Error: Column with name started_at already exists!
I tried to establish that EnsureSchema requires a handle with an attached catalog named warehouse. Its public documentation states only that it brings an existing warehouse's tables up to date (bundle.go:1547-1553), while migrateTable unconditionally filters duckdb_columns() by database_name = 'warehouse' (bundle.go:727-728). On a direct DuckDB handle, that probe therefore discovers none of the existing columns and attempts duplicate ALTER TABLE ADD COLUMN operations. Internal callers commonly arrange the alias, but that unstated calling convention does not refute the failure of the exported function on the reviewer's named input.
confirmed by pdbethke — real, reproduced: migrateTable hardcodes database_name = 'warehouse'; EnsureSchema fails on a plain handle
ref e00b52bab444de23726c8ede78bc303c094516955f2e48268c90b596bd8540ba#R2
Recorded so that "found nothing here" can be told apart from "never looked here".
Review of internal/auditpush revealed two defects: (1) `writeLedgerFile` mutates empty `ScanRow` structs by setting `PushedBy = PushedByCertify`, transforming a zero `ScanRow` into a non-zero struct and polluting ledger directory pushes with phantom `corral_scans` rows, and (2) `migrateTable` hardcodes `database_name = 'warehouse'` when querying existing columns via `duckdb_columns()`, causing `EnsureSchema` to fail with duplicate column errors when called on standard DuckDB handles not attached under the `'warehouse'` alias.
Both findings stand. R1 identifies a real divergence between directory and database pushes: the directory path turns an absent scan header into a populated row. R2 correctly identifies that the exported EnsureSchema function assumes a catalog alias that its documented contract does not require.