Skip to content

fix(filetools): fix JSON escaping, UTF-8 truncation, error handling, and channels - #152

Merged
w0wl0lxd merged 11 commits into
mainfrom
fix/filetools-correctness
Jul 28, 2026
Merged

w0wl0lxd merged 11 commits into
mainfrom
fix/filetools-correctness

Conversation

@w0wl0lxd

@w0wl0lxd w0wl0lxd commented Jul 27, 2026 •

Copy link
Copy Markdown
Owner

This PR implements Phase 1 and Phase 2 of the validated filetools review.

Changes

  1. Fix manual JSON construction in n00n-agent/src/tools/tool_search.rs

    • Replace format!-based JSON building with serde_json::json! macro
    • Properly escapes backslashes, quotes, and newlines in tool descriptions
    • Added unit test with special characters to verify JSON validity and round-trip
  2. Fix UTF-8 over-truncation in plugins/read/init.lua

    • Replace naive byte-based truncation with utf8_boundary function from tool_view.lua
    • Correctly handles multi-byte characters (e.g., emoji) at truncation boundaries
    • Added Lua test block to verify UTF-8-aware truncation
  3. Do not ignore grep search errors in n00n-agent/src/tools/grep.rs

    • Log warnings for search_path failures instead of discarding errors
    • Line 127: changed from let _ = searcher.search_path(...) to explicit error logging
  4. Warn on/propagate walk errors in n00n-agent/src/tools/grep.rs

    • Pattern match on walk entry errors and log warnings before continuing
    • Added path existence check before building walker
    • Returns error if search path does not exist
  5. Replace Arc<Mutex<Vec>> with flume channel in n00n-agent/src/tools/grep.rs

    • Use flume::unbounded::() for lock-free communication
    • Removes Mutex hot-path contention, aligns with HFT "channels over Mutex" guidance
    • Preserves final sort_by_cached_key and group-limit logic
  6. Fix n00n-lua/src/api/fs.rs glob

    • Add root validation: return error if path is not a directory
    • Log walk errors with tracing::warn! instead of silently flattening
    • Warn on non-UTF-8 paths instead of silently filtering
    • Use bounded sort for mtime: collect and sort only up to limit instead of entire tree
  7. Fix n00n-lua/src/api/fs.rs collect_dir_entries silent error swallowing

    • Explicit error logging for read_dir failures
    • Log per-entry errors instead of using flatten()
    • Log canonicalize errors during recursive traversal
  8. Fix n00n-lua/src/api/fs.rs dir silent default for depth

    • Use t.get::<Option>("depth") to distinguish missing key (default 1) from wrong type (error)
    • Returns (nil, err) for wrong type instead of silently defaulting

Tests Run

  • cargo nextest run -p n00n-lua glob - 11 tests passed
  • cargo nextest run -p n00n-agent -- grep - 16 tests passed
  • cargo clippy --all --tests -- -D warnings - no warnings

Follow-up

Phase 3 and Phase 4 items from the review are planned for a follow-up PR:

  • plugins/read/init.lua line-limited read / large-file memory use
  • plugins/lib/n00n/truncate.lua long-line dropping
  • plugins/grep/init.lua quote-stripping jank
  • plugins/grep/init.lua math.log line-number formatting
  • n00n-agent/src/tools/registry.rs linear-scan get/has

Note

Low Risk
CLI streaming behavior is slightly stricter on EOF, but the removed done_seen paths were redundant after breaking on Done; no auth or data-path changes in the shown diff.

Overview
Adds a changelog note for file tool fixes (tool-search JSON escaping, UTF-8-aware read truncation, grep error propagation, glob/fs error handling).

Separately, message_client no longer tracks a done_seen flag while reading the agent event stream. A zero-byte read (EOF) now always fails with agent stream closed without a Done event; success is only when a parsed ServerEvent::Done breaks the loop. Redundant post-loop checks tied to done_seen are removed.

Reviewed by Cursor Bugbot for commit 17b6587. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@w0wl0lxd, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9c4a47b7-6392-43d2-ad39-1e2ba7c5abdf

📥 Commits

Reviewing files that changed from the base of the PR and between bee0ace and 17b6587.

📒 Files selected for processing (2)
  • changelog.d/152.fixed.md
  • src/cmd/agent.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/filetools-correctness

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@codecov

codecov Bot commented Jul 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Criterion

Details
Benchmark suite Current: 17b6587 Previous: bee0ace Ratio
fib/jit_mlua_hook 6860317 ns/iter (± 210788) 6741141 ns/iter (± 176540) 1.02
fib/jit_watchdog 2333610 ns/iter (± 7114) 2221593 ns/iter (± 6676) 1.05
fib/jit_none 2338723 ns/iter (± 88940) 2211674 ns/iter (± 9146) 1.06
fib/interp_mlua_hook 7991368 ns/iter (± 65719) 8103119 ns/iter (± 49257) 0.99
fib/interp_watchdog 4326674 ns/iter (± 15491) 4366110 ns/iter (± 37776) 0.99
fib/interp_none 4267634 ns/iter (± 12466) 4332836 ns/iter (± 44693) 0.98
buffer_rw/jit_mlua_hook 584822 ns/iter (± 2987) 580248 ns/iter (± 1781) 1.01
buffer_rw/jit_watchdog 191938 ns/iter (± 299) 191890 ns/iter (± 1703) 1.00
buffer_rw/jit_none 191857 ns/iter (± 390) 191920 ns/iter (± 301) 1.00
buffer_rw/interp_mlua_hook 1037888 ns/iter (± 5476) 1036936 ns/iter (± 15806) 1.00
buffer_rw/interp_watchdog 588356 ns/iter (± 2701) 589236 ns/iter (± 2379) 1.00
buffer_rw/interp_none 588011 ns/iter (± 2209) 588368 ns/iter (± 4125) 1.00
splash_render_120x40 81242 ns/iter (± 2438) 68970 ns/iter (± 4986) 1.18
splash_render_200x60 112627 ns/iter (± 3455) 159710 ns/iter (± 23830) 0.71

This comment was automatically generated by workflow using github-action-benchmark.

w0wl0lxd added 7 commits July 27, 2026 08:47
- Remove unused serde_json dependency from n00n-daemon/Cargo.toml
- Gate rustix::process imports with #[cfg(unix)] in lock.rs
- Add Windows stub for pid_alive that returns false (daemon not supported yet)
- Make check_unix_peer_uid #[cfg(unix)] with Linux/non-Linux variants
- Add # Errors documentation to non-Linux check_unix_peer_uid
- Gate auth and daemon_socket_in imports with #[cfg(unix)] in server.rs
- Gate daemon_socket_in import with #[cfg(unix)] in transport.rs
- Move Path import into #[cfg(unix)] block in worker.rs
- Gate JsonValueTrait import with #[cfg(unix)] in worker.rs

These changes allow n00n-daemon to compile and pass clippy on Windows and macOS
while keeping Linux behavior intact. Windows pid_alive is a stub since the
daemon is not supported there yet.
Merge main and apply n00n-daemon cross-platform cfg gates so Windows
and macOS lint/build pass alongside the filetools correctness fixes.
Gate unix-only imports and pid_alive, drop unused serde_json, and scope
worker Path/JsonValueTrait usage so cross-platform CI passes.
Background agent servers and direct control.sock clients compile only on
unix; Windows builds use the daemon control plane or return a clear error.
@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_ae03f327-ec76-492b-80a3-936f100b46e2)

@w0wl0lxd

Copy link
Copy Markdown
Owner Author

cursor review

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_81c9d605-85af-4f3c-a870-623c1a9d735a)

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_e7fb6f82-507a-4347-be37-6ec2ad6cc8f6)

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_30f11866-fa99-4ee1-a265-13a0898d0e96)

@w0wl0lxd
w0wl0lxd marked this pull request as ready for review July 28, 2026 08:22
@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_1413bdc9-7c8e-4bed-a195-3c55b87e4827)

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_87ebcb43-fd15-4312-8d36-71448792170b)

@w0wl0lxd
w0wl0lxd merged commit d464812 into main Jul 28, 2026
33 checks passed
@linear-code

linear-code Bot commented Jul 28, 2026

Copy link
Copy Markdown

N00N-91

@w0wl0lxd
w0wl0lxd deleted the fix/filetools-correctness branch July 29, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant