docs(roadmap): add grep zero limit gap

This commit is contained in:
Yeachan-Heo
2026-05-22 07:30:46 +00:00
parent eb12c3d1ef
commit c1bb355691

View File

@@ -6697,3 +6697,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
571. **`grep_search` silently drops unreadable or non-UTF8 files, so search results can look complete while binary/permission/encoding failures are hidden from the output contract** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 06:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@8632f90`. Active tmux session at probe time: `gajae-issue-326-backlog-zero-continuation`; no active claw-code implementation session. Channel context included Jobdori's glob-sort #576 finding, so this probe stayed in the adjacent file-ops search surface but chose a different contract gap. Code inspection: `rust/crates/runtime/src/file_ops.rs::grep_search_impl` iterates `collect_search_files`, applies workspace/filter checks, then does `let Ok(file_contents) = fs::read_to_string(&file_path) else { continue; };`. Any file that exists in the search set but cannot be decoded as UTF-8, is transiently unreadable, or hits another read error is silently skipped. The returned `GrepSearchOutput` has `num_files`, `filenames`, optional `content`, and `num_matches`, but no `skipped_files`, `read_errors`, `binary_files`, or `truncated_due_to_errors` field. This differs from `read_file`, which intentionally rejects binary files with a stable error, and it makes grep output appear authoritative even when a subset of candidate files was ignored. Existing grep tests cover happy-path content matches but not a directory containing one matching text file plus one unreadable/binary/non-UTF8 file. **Required fix shape:** (a) track skipped candidate files by reason (`binary_or_non_utf8`, `permission_denied`, `read_error`) without leaking file contents; (b) expose counts and optionally bounded path samples in `GrepSearchOutput`; (c) preserve best-effort search behavior if desired, but mark the result as partial/degraded when any candidate is skipped; (d) add regressions with delayed-non-UTF8/binary and permission-denied files proving the output reports skipped counts; (e) align binary/non-UTF8 classification with the `read_file` fix required by #570 so file tools share one text/binary contract. **Why this matters:** grep is an observability surface. If it silently ignores files, agents can conclude “no matches” or report an incomplete match set without any evidence that encoding or permission failures narrowed the search. Source: gaebal-gajae dogfood response to Clawhip message `1507269308277850223` on 2026-05-22.
572. **`grep_search` accepts unknown `output_mode` strings as filename-mode success, so typos like `contents` or `json` silently change the result contract instead of failing fast** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 07:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@ef67aa9`. Active tmux session at probe time: `gajae-pr-327-backlog-zero-review`; no active claw-code implementation session. Channel context included Jobdori's adjacent grep duplicate-context #577 finding; this probe stayed in `grep_search_impl` but checked argument-contract handling. Code inspection: `rust/crates/runtime/src/file_ops.rs::grep_search_impl` sets `output_mode = input.output_mode.clone().unwrap_or_else(|| "files_with_matches")`, then special-cases only `"count"` and `"content"`. Any other string falls through to the default filename-list path and is echoed back as `mode: Some(output_mode.clone())`, with `content: None` and `num_matches: None`. That means misspellings such as `output_mode:"contents"`, `"files_with_match"`, or unsupported values like `"json"` return a successful-looking response whose `mode` claims the unsupported value while the payload shape is actually files-with-matches. There is no enum validation, no `InvalidInput`, and no test for unsupported output modes. **Required fix shape:** (a) validate `output_mode` against an explicit enum (`files_with_matches`, `content`, `count`) before reading files; (b) return a typed `InvalidInput`/machine-readable error listing supported values for unknown modes; (c) make the returned `mode` always match the actual payload semantics; (d) add regressions for `contents`, `files_with_match`, and valid `content`/`count`/default behavior; (e) keep this aligned with CLI `--output-format` enum validation gaps so search/tool contracts fail fast on typos. **Why this matters:** grep output shape drives model/tool parsing. A typo should not silently downgrade from content/count mode into filename mode while preserving the bogus mode label; that creates false negative evidence and parser confusion with no visible error. Source: gaebal-gajae dogfood response to Clawhip message `1507276861917368421` on 2026-05-22.
573. **`grep_search` treats `head_limit:0` as unlimited, so callers cannot request an empty/page-metadata probe and may accidentally dump the full match set** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 07:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@eb12c3d`. Active tmux session at probe time: `gajae-pr-330-review-v2`; no active claw-code implementation session. Code inspection: `rust/crates/runtime/src/file_ops.rs::apply_limit` is used for grep filenames and content lines. It computes `explicit_limit = limit.unwrap_or(250)`, but if `explicit_limit == 0` it returns the full post-offset item vector with `appliedLimit: None` instead of truncating to zero or rejecting the value. Therefore a caller using `head_limit:0` as a common “no rows, just metadata/count/page preflight” convention gets every matching filename/content line after the offset, bypassing the default 250 cap and potentially injecting a large result into the model context. Existing grep tests pass `head_limit: Some(10)` and do not cover zero-limit semantics. This also makes `appliedLimit` misleading: an explicit limit was supplied, but the output reports no applied limit. **Required fix shape:** (a) define `head_limit` as a positive integer and reject zero with `InvalidInput`, or make zero return an empty result with `appliedLimit:0` consistently; (b) never let zero disable truncation unless a separately named `unlimited:true` escape hatch exists; (c) add regressions for `head_limit:0` in filename and content modes, positive limits, default 250 truncation, and offset-only behavior; (d) ensure `appliedLimit` reflects the caller-supplied limit when accepted; (e) document pagination semantics so wrappers do not accidentally turn metadata probes into full dumps. **Why this matters:** search pagination is a context-window safety control. A zero limit should be safe or invalid, not the one value that disables the cap and can flood the assistant with every match. Source: gaebal-gajae dogfood response to Clawhip message `1507284411995918427` on 2026-05-22.