diff --git a/AGENTS.md b/AGENTS.md index d834899..174a053 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,7 @@ cmd/backscroll/ internal/ ├── config/ — config resolution: backscroll.toml → ~/.config → env → defaults ├── compat/ — stateless schema-shape inspection, release lineage catalog, migration plans, and canonical recovery planning +├── directsearch/ — shared direct-search predicates (IsDirectSearchCommand, IsCodexDirectSearchCall) so readers ingest path and storage replay path use the exact same strings.Fields / argv-shape acceptance ├── input_config/ — input manifest loading, discovery, and legacy session-dirs compatibility ├── models/ — domain types: SessionRecord, MessageContent, ParsedFile, SearchResult, Stats ├── sync/ — WalkDir, SHA-256 dedup, JSONL parsing, noise filtering, content-type classification @@ -235,6 +236,7 @@ Workflows delegate to [pablontiv/crossbeam](https://github.com/pablontiv/crossbe github.com/pablontiv/backscroll/cmd/backscroll — CLI entrypoint github.com/pablontiv/backscroll/internal/config — Config structs and resolution github.com/pablontiv/backscroll/internal/compat — Stateless schema inspection, release lineage catalog, migration planning, and canonical recovery planning +github.com/pablontiv/backscroll/internal/directsearch — Shared direct-search predicates (IsDirectSearchCommand, IsCodexDirectSearchCall) used by readers and storage replay github.com/pablontiv/backscroll/internal/input_config — Input manifest loading, discovery, and legacy session-dirs compatibility github.com/pablontiv/backscroll/internal/models — Domain types and SearchEngine interface github.com/pablontiv/backscroll/internal/sync — Session parsing and noise filtering diff --git a/internal/directsearch/directsearch.go b/internal/directsearch/directsearch.go new file mode 100644 index 0000000..66f5f35 --- /dev/null +++ b/internal/directsearch/directsearch.go @@ -0,0 +1,61 @@ +// Package directsearch holds the shared predicates the Codex ingest path +// and the storage replay path both rely on to decide whether a stored tool +// call is a direct `backscroll search` invocation. Keeping a single source of +// truth here is the only way the SQL "what's pending requeue?" predicate and +// the reader's "did this row get marked?" predicate can stay in lockstep — +// every prior SQL GLOB attempt to enumerate separator byte-sequences missed at +// least one real encoding (Unicode whitespace that strings.Fields accepts but +// JSON escapes in non-uniform ways), so the replay check now decodes the JSON +// argv and re-runs the exact strings.Fields-based acceptance that ingest +// already uses. +package directsearch + +import ( + "encoding/json" + "path" + "strings" +) + +// IsDirectSearchCommand is the one command boundary every reader shares: the +// raw command text must start with the bare `backscroll search` tokens. +// Absolute paths, env/shell wrappers, and other subcommands are not echoes. +// It mirrors strings.Fields splitting, so any unicode.IsSpace separator +// between the two tokens is accepted. +func IsDirectSearchCommand(command string) bool { + fields := strings.Fields(command) + return len(fields) >= 2 && fields[0] == "backscroll" && fields[1] == "search" +} + +// IsCodexDirectSearchCall recognizes Codex's own direct shell invocations of +// `backscroll search`: an `exec_command` whose raw `cmd` starts with the bare +// tokens, or a `shell` call whose argv is exactly a shell, `-c`/`-lc`, and +// that same command string. `arguments` is the JSON object the codex rollout +// stored (e.g. `{"command":["sh","-c","backscroll search"]}` or +// `{"cmd":"backscroll search"}`). +func IsCodexDirectSearchCall(tool, arguments string) bool { + switch tool { + case "exec_command": + var obj struct { + Cmd string `json:"cmd"` + } + if json.Unmarshal([]byte(arguments), &obj) != nil { + return false + } + return IsDirectSearchCommand(obj.Cmd) + case "shell": + var obj struct { + Command []string `json:"command"` + } + if json.Unmarshal([]byte(arguments), &obj) != nil || len(obj.Command) != 3 { + return false + } + if !strings.HasSuffix(path.Base(obj.Command[0]), "sh") { + return false + } + if obj.Command[1] != "-c" && obj.Command[1] != "-lc" { + return false + } + return IsDirectSearchCommand(obj.Command[2]) + } + return false +} diff --git a/internal/directsearch/directsearch_test.go b/internal/directsearch/directsearch_test.go new file mode 100644 index 0000000..6a36243 --- /dev/null +++ b/internal/directsearch/directsearch_test.go @@ -0,0 +1,176 @@ +package directsearch + +import "testing" + +func TestIsDirectSearchCommand(t *testing.T) { + tests := []struct { + name, command string + want bool + }{ + {"bare", "backscroll search", true}, + {"with_flags", "backscroll search --text orchard", true}, + {"only_two_tokens", "backscroll search", true}, + {"missing_search", "backscroll", false}, + {"missing_backscroll", "search", false}, + {"empty", "", false}, + {"whitespace_separated", "backscroll\tsearch", true}, + {"newline_separated", "backscroll\nsearch", true}, + {"nbsp_separated", "backscroll\u00a0search", true}, + {"em_space_separated", "backscroll\u2003search", true}, + {"wrapped_in_another", "env backscroll search", false}, // Fields[0]="env" not "backscroll" + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsDirectSearchCommand(tt.command); got != tt.want { + t.Errorf("IsDirectSearchCommand(%q) = %v, want %v", tt.command, got, tt.want) + } + }) + } +} + +func TestIsCodexDirectSearchCall_Shell(t *testing.T) { + tests := []struct { + name, tool, arguments string + want bool + }{ + { + name: "bare_three_element_c", + tool: "shell", + arguments: `{"command":["sh","-c","backscroll search"]}`, + want: true, + }, + { + name: "three_element_with_extra_flags", + tool: "shell", + arguments: `{"command":["sh","-c","backscroll search --text orchard"]}`, + want: true, + }, + { + name: "three_element_lc", + tool: "shell", + arguments: `{"command":["bash","-lc","backscroll search --text orchard"]}`, + want: true, + }, + { + name: "shell_binary_path", + tool: "shell", + arguments: `{"command":["/bin/bash","-c","backscroll search"]}`, + want: true, + }, + { + name: "four_elements_rejected", + tool: "shell", + arguments: `{"command":["sh","-c","backscroll search","extra"]}`, + want: false, + }, + { + name: "wrong_flag", + tool: "shell", + arguments: `{"command":["sh","-x","backscroll search"]}`, + want: false, + }, + { + name: "different_command", + tool: "shell", + arguments: `{"command":["sh","-c","ls"]}`, + want: false, + }, + { + name: "not_a_shell_binary", + tool: "shell", + arguments: `{"command":["python","-c","backscroll search"]}`, + want: false, + }, + { + name: "arguments_extra_fields_decoded", + tool: "shell", + arguments: `{"additional_permissions":{"network":false},"command":["bash","-lc","backscroll search"]}`, + want: true, + }, + { + name: "arguments_extra_fields_in_storage_position", + tool: "shell", + arguments: `{"command":["sh","-c","backscroll search"],"workdir":"/tmp","timeout_ms":10000}`, + want: true, + }, + { + name: "malformed_arguments", + tool: "shell", + arguments: `not json`, + want: false, + }, + { + name: "command_separator_tab_accepted", + tool: "shell", + arguments: `{"command":["sh","-c","backscroll\tsearch"]}`, + want: true, + }, + { + name: "command_separator_nbsp_accepted", + tool: "shell", + arguments: `{"command":["sh","-c","backscroll\u00a0search"]}`, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsCodexDirectSearchCall(tt.tool, tt.arguments); got != tt.want { + t.Errorf("IsCodexDirectSearchCall(%q, %q) = %v, want %v", tt.tool, tt.arguments, got, tt.want) + } + }) + } +} + +func TestIsCodexDirectSearchCall_ExecCommand(t *testing.T) { + tests := []struct { + name, tool, arguments string + want bool + }{ + { + name: "bare", + tool: "exec_command", + arguments: `{"cmd":"backscroll search --text orchard"}`, + want: true, + }, + { + name: "no_args", + tool: "exec_command", + arguments: `{"cmd":"backscroll search"}`, + want: true, + }, + { + name: "different_command", + tool: "exec_command", + arguments: `{"cmd":"rg orchard"}`, + want: false, + }, + { + name: "malformed_arguments", + tool: "exec_command", + arguments: `not json`, + want: false, + }, + { + name: "missing_cmd_key", + tool: "exec_command", + arguments: `{}`, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsCodexDirectSearchCall(tt.tool, tt.arguments); got != tt.want { + t.Errorf("IsCodexDirectSearchCall(%q, %q) = %v, want %v", tt.tool, tt.arguments, got, tt.want) + } + }) + } +} + +func TestIsCodexDirectSearchCall_UnknownTool(t *testing.T) { + if got := IsCodexDirectSearchCall("unknown", `{"command":["sh","-c","backscroll search"]}`); got { + t.Errorf("unknown tool should not match, got true") + } + if got := IsCodexDirectSearchCall("", `{"command":["sh","-c","backscroll search"]}`); got { + t.Errorf("empty tool should not match, got true") + } +} diff --git a/internal/readers/claude_reader.go b/internal/readers/claude_reader.go index ff35b9e..7602a13 100644 --- a/internal/readers/claude_reader.go +++ b/internal/readers/claude_reader.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/pablontiv/backscroll/internal/directsearch" "github.com/pablontiv/backscroll/internal/input_config" "github.com/pablontiv/backscroll/internal/models" "github.com/pablontiv/backscroll/internal/sync" @@ -250,15 +251,7 @@ func isDirectSearchInput(tool string, input json.RawMessage) bool { if err := json.Unmarshal(input, &obj); err != nil { return false } - return isDirectSearchCommand(obj.Command) -} - -// isDirectSearchCommand is the one command boundary every reader shares: the -// raw command text must start with the bare `backscroll search` tokens. -// Absolute paths, env/shell wrappers, and other subcommands are not echoes. -func isDirectSearchCommand(command string) bool { - fields := strings.Fields(command) - return len(fields) >= 2 && fields[0] == "backscroll" && fields[1] == "search" + return directsearch.IsDirectSearchCommand(obj.Command) } func classifyText(text string) string { diff --git a/internal/readers/codex_reader.go b/internal/readers/codex_reader.go index 5848e7e..087d1e4 100644 --- a/internal/readers/codex_reader.go +++ b/internal/readers/codex_reader.go @@ -2,10 +2,10 @@ package readers import ( "encoding/json" - "path" "strings" "time" + "github.com/pablontiv/backscroll/internal/directsearch" "github.com/pablontiv/backscroll/internal/input_config" "github.com/pablontiv/backscroll/internal/models" "github.com/pablontiv/backscroll/internal/sync" @@ -125,37 +125,11 @@ func codexCallOf(item codexItem) codexCall { return codexCall{} } -// isCodexDirectSearchCall recognizes Codex's own direct shell invocations of -// `backscroll search`: an `exec_command` whose raw `cmd` starts with the bare -// tokens, or a `shell` call whose argv is exactly a shell, `-c`/`-lc`, and -// that same command string. The wrapper form is Codex-reader-local; the -// shared command boundary itself never widens. +// isCodexDirectSearchCall is now a thin wrapper around the shared predicate +// in internal/directsearch. The storage replay path uses the same function, +// so the two call sites cannot drift apart. func isCodexDirectSearchCall(tool, arguments string) bool { - switch tool { - case "exec_command": - var obj struct { - Cmd string `json:"cmd"` - } - if json.Unmarshal([]byte(arguments), &obj) != nil { - return false - } - return isDirectSearchCommand(obj.Cmd) - case "shell": - var obj struct { - Command []string `json:"command"` - } - if json.Unmarshal([]byte(arguments), &obj) != nil || len(obj.Command) != 3 { - return false - } - if !strings.HasSuffix(path.Base(obj.Command[0]), "sh") { - return false - } - if obj.Command[1] != "-c" && obj.Command[1] != "-lc" { - return false - } - return isDirectSearchCommand(obj.Command[2]) - } - return false + return directsearch.IsCodexDirectSearchCall(tool, arguments) } func codexMessage(item codexItem, ts time.Time, reasoning bool) (models.Message, bool) { diff --git a/internal/storage/queries.go b/internal/storage/queries.go index 17a1279..5b23653 100644 --- a/internal/storage/queries.go +++ b/internal/storage/queries.go @@ -3,12 +3,14 @@ package storage import ( "context" "database/sql" + "encoding/json" "fmt" "sort" "strings" "time" "github.com/pablontiv/backscroll/internal/categories" + "github.com/pablontiv/backscroll/internal/directsearch" "github.com/pablontiv/backscroll/internal/projects" "github.com/pablontiv/backscroll/internal/sequences" ) @@ -989,6 +991,16 @@ func (d *Database) StalePaths(currentVersion int) ([]string, error) { // It also requeues surviving sources whose tool rows were stored as search_echo=0 // with a serialized direct search call (pre-#80 Codex/OpenCode writes), so // identity pairing can mark the paired result. Output-only rows stay unmatched. +// +// The bash and exec_cmd cases are filtered in SQL — their stored text is the +// simple `name key=value` token form with no JSON encoding, so a GLOB prefix +// is faithful. The Codex shell case is filtered in Go (see +// filterShellEchoZeroPaths / pendingSearchEchoShellMatches): its arguments +// are JSON-encoded and the on-disk separator between 'backscroll' and +// 'search' can be any form strings.Fields accepts but the JSON encoder +// leaves in the wild (literal space, \t/\n/\f/\r, \uXXXX, or raw UTF-8 bytes +// for non-control whitespace). Trying to enumerate every byte sequence in +// SQL GLOB is provably unbounded; decoding the JSON argv in Go is not. func (d *Database) PendingSearchEchoPaths() ([]string, error) { return d.stalePaths(0, true) } @@ -1002,8 +1014,16 @@ func (d *Database) stalePaths(currentVersion int, echoOnly bool) ([]string, erro AND ((? AND (search_items.extraction_version IS NULL OR search_items.extraction_version < ?)) OR search_items.search_echo IS NULL` if echoOnly { + // Broad filter for shell candidates (any tool row whose text starts + // with the bare `shell` tool-name token). The strict + // isCodexDirectSearchCall check happens in Go via + // pendingSearchEchoShellMatches, which locates the `command=` token + // boundary inside the sorted key=value list (it may not be the first + // key — e.g. `additional_permissions` sorts before `command`) and + // decodes just the JSON array that follows. query += ` - OR (` + unmarkedDirectSearchCallSQL("search_items") + `)` + OR (` + unmarkedDirectSearchCallSQL("search_items") + `) + OR (search_items.content_type = 'tool' AND COALESCE(search_items.search_echo, 0) = 0 AND search_items.text LIKE 'shell %')` } query += `) ORDER BY indexed_files.last_indexed ASC, search_items.source_path ASC @@ -1028,9 +1048,134 @@ func (d *Database) stalePaths(currentVersion int, echoOnly bool) ([]string, erro return nil, fmt.Errorf("iterate stale paths: %w", err) } + if echoOnly { + filtered, err := d.filterShellEchoZeroPaths(paths) + if err != nil { + return nil, err + } + paths = filtered + } + return paths, nil } +// filterShellEchoZeroPaths drops paths whose only echo-zero shell candidate +// row is rejected by directsearch.IsCodexDirectSearchCall. A path survives if +// it has a v15 NULL search_echo row (those are always kept), a non-shell +// echo-zero candidate (bash / exec_cmd) that already passed the SQL filter, +// or at least one echo-zero shell row that the strict reader predicate +// accepts. +func (d *Database) filterShellEchoZeroPaths(paths []string) ([]string, error) { + if len(paths) == 0 { + return paths, nil + } + keep := make(map[string]bool, len(paths)) + for _, path := range paths { + // Reason 1: v15 NULL search_echo backlog — always keep. + var nullHit int + err := d.db.QueryRow(` + SELECT 1 FROM search_items + WHERE source_path = ? AND search_echo IS NULL + LIMIT 1 + `, path).Scan(&nullHit) + if err != nil && err != sql.ErrNoRows { + return nil, fmt.Errorf("check NULL backlog for %s: %w", path, err) + } + if nullHit == 1 { + keep[path] = true + continue + } + // Reason 2: non-shell echo-zero survivor (bash / exec_cmd GLOB). + var globHit int + err = d.db.QueryRow(` + SELECT 1 FROM search_items + WHERE source_path = ? + AND content_type = 'tool' + AND COALESCE(search_echo, 0) = 0 + AND text NOT LIKE 'shell %' + AND (`+unmarkedDirectSearchCallSQL("search_items")+`) + LIMIT 1 + `, path).Scan(&globHit) + if err != nil && err != sql.ErrNoRows { + return nil, fmt.Errorf("check non-shell echo-zero for %s: %w", path, err) + } + if globHit == 1 { + keep[path] = true + continue + } + // Reason 3: echo-zero shell candidate. Apply the strict reader + // predicate — keep the path if ANY row matches. + rows, err := d.db.Query(` + SELECT text FROM search_items + WHERE source_path = ? + AND content_type = 'tool' + AND COALESCE(search_echo, 0) = 0 + AND text LIKE 'shell %' + `, path) + if err != nil { + return nil, fmt.Errorf("query shell rows for %s: %w", path, err) + } + matched := false + for rows.Next() { + var text string + if err := rows.Scan(&text); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("scan shell row for %s: %w", path, err) + } + if pendingSearchEchoShellMatches(text) { + matched = true + break + } + } + _ = rows.Close() + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate shell rows for %s: %w", path, err) + } + if matched { + keep[path] = true + } + } + out := make([]string, 0, len(keep)) + for _, p := range paths { + if keep[p] { + out = append(out, p) + } + } + return out, nil +} + +// pendingSearchEchoShellMatches is the Go-side check for whether a stored +// Codex shell tool row is a direct `backscroll search` call. +// +// SerializeToolInput emits the rollout's `arguments` as a space-joined +// `key=value` token list with keys sorted alphabetically. Real Codex shell +// calls carry not just `command` but also `workdir`, `timeout_ms`, and +// potentially `additional_permissions` or `sandbox` — so `command=` is not +// guaranteed to be the first key. We locate the ` command=` token boundary +// anywhere in the text and feed only what follows to a json.Decoder, which +// stops after reading one complete JSON value (the array). Whatever +// (already-serialized, non-JSON) `key=value` text follows the array is +// ignored. The decoded array is then wrapped into the object shape +// directsearch.IsCodexDirectSearchCall expects and fed to the exact same +// predicate the reader uses at ingest time. +func pendingSearchEchoShellMatches(text string) bool { + const token = " command=" + idx := strings.Index(text, token) + if idx < 0 { + return false + } + remainder := text[idx+len(token):] + var commandArray []string + if err := json.NewDecoder(strings.NewReader(remainder)).Decode(&commandArray); err != nil { + return false + } + args, err := json.Marshal(map[string][]string{"command": commandArray}) + if err != nil { + return false + } + return directsearch.IsCodexDirectSearchCall("shell", string(args)) +} + // ReresolveProjects iterates all distinct source_paths where project='unknown' or project IS NULL, // calls the resolver function for each path, and updates ALL rows for that path with the returned project ID. // If resolver returns empty string or "unknown", the source_path is skipped and rows remain unchanged. diff --git a/internal/storage/search.go b/internal/storage/search.go index a7efe93..d91c724 100644 --- a/internal/storage/search.go +++ b/internal/storage/search.go @@ -434,6 +434,17 @@ func directBackscrollSearchEchoSQL(alias string) string { // readers wrote false as 0, so those rows are not in the v15 NULL backlog. // Requeueing the call's source file lets identity pairing mark the result; // output shape is never matched here. +// +// Only the bash and exec_cmd cases are matched in SQL: their stored text uses +// simple `name key=value` tokens with no JSON encoding, so a GLOB prefix is +// faithful. The Codex shell case has its own predicate in Go (see +// PendingSearchEchoPaths and directsearch.IsCodexDirectSearchCall) because +// its arguments are JSON-encoded and the on-disk separator between +// 'backscroll' and 'search' can be any form strings.Fields accepts but the +// JSON encoder leaves in the wild (literal space, \t/\n/\f/\r, \uXXXX, or +// raw UTF-8 bytes for non-control whitespace). Trying to enumerate every +// byte sequence in SQL GLOB is provably unbounded; decoding the JSON argv +// in Go is not. func unmarkedDirectSearchCallSQL(alias string) string { trimmed := "ltrim(" + alias + ".text, " + asciiWhitespaceSQL + ")" sep := "'[' || " + asciiWhitespaceSQL + " || ']'" diff --git a/internal/storage/search_echo_test.go b/internal/storage/search_echo_test.go index b98ce7a..953c16e 100644 --- a/internal/storage/search_echo_test.go +++ b/internal/storage/search_echo_test.go @@ -3,14 +3,56 @@ package storage import ( "context" "database/sql" + "encoding/json" + "os" "path/filepath" "reflect" + "strings" "testing" "github.com/pablontiv/backscroll/internal/compat" + "github.com/pablontiv/backscroll/internal/input_config" "github.com/pablontiv/backscroll/internal/models" + "github.com/pablontiv/backscroll/internal/readers" ) +// shellText serializes a Codex shell argv triple via a real json.Marshal +// round-trip into readers.SerializeToolInput, so the stored text mirrors what +// the CodexReader actually persists for an accepted argv shape. The earlier +// fmt.Sprintf+%q fixture used Go's strconv.Quote escaping, which escapes +// NBSP (U+00A0) as `\u00a0` text — but real JSON encoders (Go's encoding/json +// and Rust serde_json) only escape control chars (<0x20) and emit other +// unicode.IsSpace runes as their raw UTF-8 bytes. Building fixtures through +// a real json.Marshal round-trip catches encoding-divergence bugs the string +// quoting cannot. +func shellText(t *testing.T, shellBin, flag, argv2 string) string { + t.Helper() + args := struct { + Command []string `json:"command"` + }{Command: []string{shellBin, flag, argv2}} + raw, err := json.Marshal(args) + if err != nil { + t.Fatal(err) + } + return readers.SerializeToolInput("shell", raw) +} + +// shellTextN serializes a Codex shell call with an arbitrary argv length. Used +// for 4+ element fixtures the reader rejects via len(Command)==3, whose +// over-match guard must still hold for every JSON-escape separator form. +func shellTextN(t *testing.T, shellBin, flag string, argvRest ...string) string { + t.Helper() + argv := append([]string{shellBin, flag}, argvRest...) + args := struct { + Command []string `json:"command"` + }{Command: argv} + raw, err := json.Marshal(args) + if err != nil { + t.Fatal(err) + } + return readers.SerializeToolInput("shell", raw) +} + func TestV15EchoBackfillPreservesPerennialIdentity(t *testing.T) { path := createFixtureDatabase(t, "v14.sql") db, err := openWithoutSetup(path) @@ -133,6 +175,147 @@ func TestPendingSearchEchoPathsRequeuesZeroValuedDirectCalls(t *testing.T) { } } +func TestPendingSearchEchoPathsRequeuesZeroValuedCodexShellCall(t *testing.T) { + db, err := Open(filepath.Join(t.TempDir(), "index.db")) + if err != nil { + t.Fatal(err) + } + defer db.Close() + // Each fixture stores one tool row at search_echo=0 to simulate a pre-#80 + // Codex writer. The requeue expectation mirrors isCodexDirectSearchCall's + // argv shape: argv is exactly [, -c|-lc, "backscroll search ..."]. + files := []IndexedFile{ + // Requeued: bare 3-element -c shell direct search. + {Source: "session", SourcePath: "shell_bare_c.jsonl", Hash: "h1", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["sh","-c","backscroll search"]`, ContentType: "tool"}, + }}, + // Requeued: 3-element -c with extra args. + {Source: "session", SourcePath: "shell_args_c.jsonl", Hash: "h2", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["sh","-c","backscroll search --text orchard"]`, ContentType: "tool"}, + }}, + // Requeued: 3-element -lc with /bin/bash path. + {Source: "session", SourcePath: "shell_args_lc.jsonl", Hash: "h3", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["/bin/bash","-lc","backscroll search --text orchard"]`, ContentType: "tool"}, + }}, + // NOT requeued: different command (argv[2]="ls"). + {Source: "session", SourcePath: "shell_ls.jsonl", Hash: "h4", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["sh","-c","ls"]`, ContentType: "tool"}, + }}, + // NOT requeued: wrong flag (argv[1]="-x"). + {Source: "session", SourcePath: "shell_xflag.jsonl", Hash: "h5", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["sh","-x","backscroll search"]`, ContentType: "tool"}, + }}, + // NOT requeued: 4-element argv with trailing 4th element. + {Source: "session", SourcePath: "shell_four.jsonl", Hash: "h6", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["sh","-c","backscroll search","bar"]`, ContentType: "tool"}, + }}, + // NOT requeued: 4-element argv with trailing whitespace + 4th element (over-match guard). + {Source: "session", SourcePath: "shell_four_ws.jsonl", Hash: "h7", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["sh","-c","backscroll search ","bar"]`, ContentType: "tool"}, + }}, + // NOT requeued: 4-element argv with -lc flag. + {Source: "session", SourcePath: "shell_four_lc.jsonl", Hash: "h8", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: `shell command=["bash","-lc","backscroll search","bar"]`, ContentType: "tool"}, + }}, + } + if err := db.SyncFiles(files); err != nil { + t.Fatal(err) + } + // Force every row to search_echo=0 so only the new SQL clause can requeue them. + if _, err := db.db.Exec(`UPDATE search_items SET search_echo=0`); err != nil { + t.Fatal(err) + } + got, err := db.PendingSearchEchoPaths() + if err != nil { + t.Fatal(err) + } + want := []string{"shell_args_c.jsonl", "shell_args_lc.jsonl", "shell_bare_c.jsonl"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("pending=%v want %v", got, want) + } +} + +// TestPendingSearchEchoPathsRequeuesZeroValuedCodexShellSerializedWhitespace +// walks the reader's strings.Fields acceptance for every unicode.IsSpace +// separator between 'backscroll' and 'search' inside the shell argv's third +// element. Each fixture is built via a real json.Marshal round-trip into +// readers.SerializeToolInput, so the stored text carries whatever the JSON +// encoder actually produces — control chars (U+0009/000A/000D/000C) escape +// to \t/\n/\r/\f, but every other unicode.IsSpace rune (NBSP U+00A0, NEL +// U+0085, en/em spaces U+2002-2003, …) survives as raw UTF-8 bytes that no +// SQL GLOB can keep up with. The replay check must therefore do what the +// reader does — decode the JSON argv and re-run strings.Fields — and this +// test pins that behavior down. +func TestPendingSearchEchoPathsRequeuesZeroValuedCodexShellSerializedWhitespace(t *testing.T) { + db, err := Open(filepath.Join(t.TempDir(), "index.db")) + if err != nil { + t.Fatal(err) + } + defer db.Close() + files := []IndexedFile{ + // Requeued: literal space (control case). + {Source: "session", SourcePath: "sep_space.jsonl", Hash: "h1", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll search orchard"), ContentType: "tool"}, + }}, + // Requeued: JSON \t escape (U+0009 < 0x20 → escapes). + {Source: "session", SourcePath: "sep_tab.jsonl", Hash: "h2", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\tsearch orchard"), ContentType: "tool"}, + }}, + // Requeued: JSON \n escape (U+000A < 0x20). + {Source: "session", SourcePath: "sep_newline.jsonl", Hash: "h3", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\nsearch orchard"), ContentType: "tool"}, + }}, + // Requeued: JSON \r escape (U+000D < 0x20). + {Source: "session", SourcePath: "sep_cr.jsonl", Hash: "h4", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\rsearch orchard"), ContentType: "tool"}, + }}, + // Requeued: JSON \f escape (U+000C < 0x20). + {Source: "session", SourcePath: "sep_ff.jsonl", Hash: "h5", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\fsearch orchard"), ContentType: "tool"}, + }}, + // Requeued: NBSP (U+00A0) — raw UTF-8 bytes (0xc2 0xa0), no \uXXXX escape. + // This is the case the round-2 SQL GLOB missed. + {Source: "session", SourcePath: "sep_nbsp.jsonl", Hash: "h6", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\u00a0search orchard"), ContentType: "tool"}, + }}, + // Requeued: en space (U+2002) — raw UTF-8 bytes (0xe2 0x80 0x82). + {Source: "session", SourcePath: "sep_en_space.jsonl", Hash: "h7", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\u2002search orchard"), ContentType: "tool"}, + }}, + // Requeued: em space (U+2003) — raw UTF-8 bytes (0xe2 0x80 0x83). + {Source: "session", SourcePath: "sep_em_space.jsonl", Hash: "h8", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "sh", "-c", "backscroll\u2003search orchard"), ContentType: "tool"}, + }}, + // Requeued: NBSP with -lc flag and /bin/bash path (cross-flag sanity). + {Source: "session", SourcePath: "sep_nbsp_lc.jsonl", Hash: "h9", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellText(t, "/bin/bash", "-lc", "backscroll\u00a0search orchard"), ContentType: "tool"}, + }}, + // NOT requeued: 4-element argv with NBSP separator + 4th element + // (over-match guard holds for raw UTF-8 separators too). + {Source: "session", SourcePath: "sep_nbsp_four.jsonl", Hash: "h10", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellTextN(t, "sh", "-c", "backscroll\u00a0search ", "bar"), ContentType: "tool"}, + }}, + // NOT requeued: 4-element argv with tab separator + 4th element. + {Source: "session", SourcePath: "sep_tab_four.jsonl", Hash: "h11", Messages: []IndexedMessage{ + {Ordinal: 0, Role: "assistant", Text: shellTextN(t, "sh", "-c", "backscroll\tsearch ", "bar"), ContentType: "tool"}, + }}, + } + if err := db.SyncFiles(files); err != nil { + t.Fatal(err) + } + if _, err := db.db.Exec(`UPDATE search_items SET search_echo=0`); err != nil { + t.Fatal(err) + } + got, err := db.PendingSearchEchoPaths() + if err != nil { + t.Fatal(err) + } + want := []string{"sep_cr.jsonl", "sep_em_space.jsonl", "sep_en_space.jsonl", "sep_ff.jsonl", "sep_nbsp.jsonl", "sep_nbsp_lc.jsonl", "sep_newline.jsonl", "sep_space.jsonl", "sep_tab.jsonl"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("pending=%v want %v", got, want) + } +} + func TestEchoProvenanceDoesNotAffectProse(t *testing.T) { db, err := Open(filepath.Join(t.TempDir(), "index.db")) if err != nil { @@ -147,3 +330,93 @@ func TestEchoProvenanceDoesNotAffectProse(t *testing.T) { t.Fatalf("prose filtered: %v %v", got, err) } } + +// TestPendingSearchEchoPathsRequeuesCodexShellRoundTrip is the regression +// fixture for the round-3 reviewer finding. Real Codex shell calls carry +// not just `command` but also `workdir`, `timeout_ms`, and potentially +// `additional_permissions` — Codex's own ShellToolCallParams schema. +// SerializeToolInput sorts the keys alphabetically, so a call with +// `additional_permissions` serializes as +// `shell additional_permissions={...} command=[...] workdir=/tmp timeout_ms=10000`, +// not `shell command=[...] workdir=/tmp timeout_ms=10000`. The requeue path +// must accept both orderings (Bug B: broaden the admission past the literal +// `shell command=` prefix and locate the `command=` token boundary inside +// the sorted list) and must read only the JSON array that follows, +// ignoring the trailing key=value tokens (Bug A: don't try to wrap the +// entire remainder as a JSON object). +// +// The fixture is generated via a full CodexReader.Parse round-trip of an +// inline rollout JSONL — not via SerializeToolInput directly, not via a +// hand-written string — so it exercises the same storage path the production +// ingest uses. +func TestPendingSearchEchoPathsRequeuesCodexShellRoundTrip(t *testing.T) { + db, err := Open(filepath.Join(t.TempDir(), "index.db")) + if err != nil { + t.Fatal(err) + } + defer db.Close() + // Three shapes: command-only (control), command + workdir + timeout_ms, + // and the worst case — additional_permissions sorts before command, so + // the legacy `text LIKE 'shell command=[%'` prefilter would miss it. + cases := []struct { + name, argsJSON string + requeue bool + }{ + {"control", `{"command":["sh","-c","backscroll search --text orchard"]}`, true}, + {"workdir_timeout_ms", `{"command":["sh","-c","backscroll search"],"workdir":"/tmp","timeout_ms":10000}`, true}, + {"additional_permissions_first", `{"additional_permissions":{"network":false},"command":["bash","-lc","backscroll search --text orchard"]}`, true}, + {"workdir_first", `{"workdir":"/tmp","command":["sh","-c","backscroll search"]}`, true}, + {"wrong_command", `{"command":["sh","-c","ls"],"workdir":"/tmp","timeout_ms":10000}`, false}, + {"four_elements", `{"command":["sh","-c","backscroll search","bar"],"workdir":"/tmp"}`, false}, + } + for i, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + jsonl := "{\"ordinal\":0,\"timestamp\":\"2026-09-01T12:00:00Z\",\"type\":\"session_meta\",\"payload\":{\"cwd\":\"/synthetic/test\"}}\n" + + "{\"ordinal\":1,\"timestamp\":\"2026-09-01T12:00:01Z\",\"type\":\"response_item\",\"payload\":{\"type\":\"function_call\",\"name\":\"shell\",\"call_id\":\"test-call\",\"arguments\":\"" + strings.ReplaceAll(tc.argsJSON, "\"", "\\\"") + "\"}}\n" + dir := t.TempDir() + path := filepath.Join(dir, "codex.jsonl") + if err := os.WriteFile(path, []byte(jsonl), 0o644); err != nil { + t.Fatal(err) + } + parsed, err := (&readers.CodexReader{}).Parse(path, input_config.InputDefinition{}) + if err != nil { + t.Fatal(err) + } + var storedText string + for _, rec := range parsed.Records { + if rec.ContentType == "tool" && strings.HasPrefix(rec.Content, "shell ") { + storedText = rec.Content + break + } + } + if storedText == "" { + t.Fatal("shell record not parsed") + } + db2, err := Open(filepath.Join(t.TempDir(), "db-"+tc.name+".db")) + if err != nil { + t.Fatal(err) + } + defer db2.Close() + sourcePath := "shell_" + tc.name + ".jsonl" + if err := db2.SyncFiles([]IndexedFile{{Source: "session", SourcePath: sourcePath, Hash: "h", Messages: []IndexedMessage{{Ordinal: 0, Role: "assistant", Text: storedText, ContentType: "tool"}}}}); err != nil { + t.Fatal(err) + } + if _, err := db2.db.Exec(`UPDATE search_items SET search_echo=0`); err != nil { + t.Fatal(err) + } + got, err := db2.PendingSearchEchoPaths() + if err != nil { + t.Fatal(err) + } + if tc.requeue { + if !reflect.DeepEqual(got, []string{sourcePath}) { + t.Errorf("case %d (%s): pending=%v want [%s]", i, tc.name, got, sourcePath) + } + } else { + if len(got) != 0 { + t.Errorf("case %d (%s): pending=%v want []", i, tc.name, got) + } + } + }) + } +}