Explain why read_files blocks gitignored files - #1196
Conversation
4c5fab2 to
ed92e6e
Compare
|
Good diagnosis:
One thing I couldn't verify from the diff: Minor nit: the generated message is quite long and packs several facts into one sentence (rule sources, OS-permission disclaimer, existence, glob/code_search behavior, remediation, and the negation caveat). Consider whether the model actually needs all of that every time, or whether some of it belongs in the tool's static description instead of a per-call string. Not a blocker, just something a maintainer may want to trim. Overall: focused, tested, and addresses a real usability gap. Worth porting once the import is confirmed. |
Summary
read_filesreturns a bare[BLOCKED]when a file is excluded by ignore rules, with no reason and no remediation — the model can't tell the user why, and often can't even tell the file exists (glob/code_search omit it too). This makes the ignore-rule block self-describing, following the existingFILE_TOO_LARGEprecedent of sentinel + trailing explanation. Addresses #1151 (the interactive permission prompt the reporter also asks for is host-app side; see Out of scope).Root cause
sdk/src/tools/read-files.tsreturnsFILE_READ_STATUS.IGNORED('[BLOCKED]') verbatim from three distinct block paths: the.envpolicy, a hostfileFilter, and the gitignore/.codebuffignore/built-in-defaults check. Only the third is the silent, surprising one users hit — and it gave the model nothing to work with. The sentinel is consumed viastartsWith(HIDDEN_FILE_READ_STATUS/toOptionalFileincommon/src/constants/paths.ts) and rendered pass-through byagent-runtime, so a trailing explanation is safe;TOO_LARGEalready ships one.Changes
sdk/src/tools/read-files.ts— in theisFileIgnoredbranch only, the result becomes[BLOCKED]: <reason> <remediation>:.gitignore,.codebuffignore, built-in defaults) and states it is not an OS permission issue.fs.statconfirms it (the ignore check itself never touches the file, so an ignored-and-deleted path must not get the existence claim).isFileIgnoredcan fail closed on an unreadable ignore file without any rule matching (common/src/project-file-tree.ts,throwOnReadError: allowEnvTemplate), the wording hedges to "blocked by ignore-rule checking" instead of asserting a match..codebuffignore, with the explicit caveat that a file-level negation cannot re-include a path under an excluded directory (isIgnoredByIgnoreChainreturns on the first ignored parent prefix, so!build/out.jsis inert whenbuild/itself is ignored)..env-policy and custom-fileFilterblocks stay bare[BLOCKED]— those are deliberate security/host decisions that shouldn't leak reasons.sdk/src/tools/read-files.ts— the explanatory suffix is suppressed for sensitive env paths even on the internal-edit path (enforceEnvPolicy: false, which skips the top env gate), so a secret blocked via built-in ignore defaults still gets a bare[BLOCKED].sdk/README.md— documents that ignore-rule blocks carry a trailing reason and that.codebuffignore(.gitignoresyntax, checked alongside) is the project-level escape hatch; previously undocumented repo-wide.sdk/src/__tests__/read-files.test.ts— gitignore-path assertions switched from exacttoBeto sentinel-prefixstartsWith(env-policy/filter-block assertions remain exact, guarding the opacity); new regression test asserts the reason text, the directory-negation caveat, the omitted existence claim for an ignored-and-deleted file, and that the.envpolicy block stays opaque.sdk/src/__tests__/run-file-filter.test.ts— one gitignore-path assertion switched to prefix match.Out of scope: (a) an interactive permission prompt — no request-permission primitive exists in the public tree (contracts, client action, and the desktop UI are all private-side); this PR makes the tool layer say why so a host (or the model, in prose) can offer the
.codebuffignoreescape the reporter found manually. (b) changing glob/code_search output — separate surface, separate PR. The issue's "agent cannot detect the file exists" complaint is addressed here at the tool layer: the block message now states the file was omitted by discovery for the same reason, and that it exists on disk when a stat confirms it.Tests
read-files.test.ts,run-file-filter.test.ts): 44 pass / 0 fail.ignore-rule block explains reason and unblock path...→ red); with it applied, green. The new test also covers the ignored-and-deleted case (no existence claim) and the internal-edit secret gate.Validation
On mirror
0d2d7a085(branch rebased onto latestmain; branch basef5adf610ftouched no PR file —git diff f5adf610f..0d2d7a085 --name-only -- sdk/lists onlysdk/test/setup-env.ts, re-gated after rebase):cd sdk && CI=true bun run test— 537 pass / 0 fail (sdk is clean on main; unchanged).cd sdk && bun run typecheck— 0 errors (baseline 0).cd cli && CI=true bun run test— 2848 pass / 13 fail / 11 errors of 2871; failure set identical to the known-red baseline (3 ×* release wrapper contains only product configuration and package loading+ unloadablepackages/internal/tarfiles). No new failures.bun x prettier --checkon the touched files (incl.sdk/README.md):read-files.ts,run-file-filter.test.ts, andREADME.mdclean;read-files.test.tshas exactly one dirty line, the pre-existingbigFilewrap at ~line 841 that this diff does not touch — no new warnings from changed lines.bun run build:sdkandbun run build:freebuff— both succeed.=== '[BLOCKED]'comparisons exist anywhere in the public tree (grep for the literal finds only the constant definition);toOptionalFile/HIDDEN_FILE_READ_STATUSusestartsWith;agent-runtime(render-read-files-result.ts,getPreviouslyReadFiles) passes content through unmodified.1.3.14(.bun-version) viabunx bun@1.3.14— sdk suite, typecheck, prettier, and the cli failure set match the numbers above, which were first measured on local Bun 1.4.0.