Repository navigation
fix(grep): handle model-generated inputs better - #233
Conversation
- Reject unknown parameters like 'command' via additionalProperties=false - Allow 'path' to be either string or array of strings for multiple paths - Improve regex error messages with PCRE look-around guidance - Update tool description and prompt hint to guide model behavior Fixes common failures where models pass 'command' instead of 'pattern', use PCRE look-around syntax not supported by Rust regex, or pass multiple space-separated paths in a single string.
|
Warning Review limit reached
Next review available in: 6 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe grep tool now accepts one or more paths, searches each path, deduplicates results, preserves partial successes, and reports warnings. It adds file modification times and documents Rust regex limitations. ChangesGrep support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GrepCaller
participant plugins_grep_init_lua
participant fs_grep
participant ResultRenderer
GrepCaller->>plugins_grep_init_lua: Submit string or array path
plugins_grep_init_lua->>fs_grep: Search each path
fs_grep-->>plugins_grep_init_lua: Return entries and errors
plugins_grep_init_lua->>ResultRenderer: Deduplicate, limit, format, and render results
ResultRenderer-->>GrepCaller: Return results and warnings
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Criterion
Details
| Benchmark suite | Current: 3dd3cd1 | Previous: d4aa62d | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6423938 ns/iter (± 39034) |
5200508 ns/iter (± 134174) |
1.24 |
fib/jit_watchdog |
2440034 ns/iter (± 7032) |
1893840 ns/iter (± 15674) |
1.29 |
fib/jit_none |
2443771 ns/iter (± 84059) |
1898454 ns/iter (± 15979) |
1.29 |
fib/interp_mlua_hook |
7653880 ns/iter (± 47384) |
5958861 ns/iter (± 35213) |
1.28 |
fib/interp_watchdog |
3846937 ns/iter (± 25806) |
2991790 ns/iter (± 63779) |
1.29 |
fib/interp_none |
3751394 ns/iter (± 21226) |
2909064 ns/iter (± 8038) |
1.29 |
buffer_rw/jit_mlua_hook |
553613 ns/iter (± 3170) |
431415 ns/iter (± 3093) |
1.28 |
buffer_rw/jit_watchdog |
168083 ns/iter (± 309) |
130366 ns/iter (± 800) |
1.29 |
buffer_rw/jit_none |
167957 ns/iter (± 487) |
130285 ns/iter (± 476) |
1.29 |
buffer_rw/interp_mlua_hook |
1040948 ns/iter (± 14604) |
815214 ns/iter (± 14486) |
1.28 |
buffer_rw/interp_watchdog |
629972 ns/iter (± 3882) |
486231 ns/iter (± 3200) |
1.30 |
buffer_rw/interp_none |
629840 ns/iter (± 36830) |
484431 ns/iter (± 3111) |
1.30 |
splash_render_120x40 |
77438 ns/iter (± 1261) |
50403 ns/iter (± 4688) |
1.54 |
splash_render_200x60 |
108479 ns/iter (± 4441) |
89670 ns/iter (± 16933) |
1.21 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Pushed 1ab3972 with the generated docs and a regression fix for the new multi-path input. The grep header previously passed the array directly to shorten_path, so restoring a call with path=["src", "tests"] silently lost the header; it now renders each path safely. Validation: just gen-docs-check; cargo test -p n00n-lua --test real_plugins_restore (32 passed); cargo test -p n00n-agent --lib grep (19 passed); git diff --check. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ab3972273
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| plugins/grep/init.lua | Core plugin change: adds multi-path support and additionalProperties guard, but the nil-path branch is missing, causing every path-less grep call to return 'No files found' without searching. |
| n00n-agent/src/tools/grep.rs | Adds PCRE look-around guidance to both regex error paths; straightforward and correct. |
| n00n-lua/tests/real_plugins_restore.rs | New test covers multi-path header rendering (restore), but does not exercise the handler, so the nil-path regression is not caught. |
| site/docs/content/tools/_index.md | Updated docs correctly add PCRE note but label the path type as 'object' rather than 'string or array'. |
| changelog.d/222.fixed.md | Changelog entry accurately describes the intent of this PR. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[handler called] --> B{path type?}
B -->|string| C[search_single_path]
B -->|table| D[loop: search_single_path per item]
B -->|other non-nil| E[return type error]
B -->|nil - MISSING BRANCH| F[no search executed]
F --> G[all_entries empty]
G --> H[return NO_MATCHES - regression]
C --> I[accumulate entries and errors]
D --> I
I --> J{entries empty and errors?}
J -->|yes| K[return first error]
J -->|entries empty no errors| L[return NO_MATCHES]
J -->|has entries| M{errors also present?}
M -->|yes - silent drop| N[errors discarded]
M -->|no| O[return formatted results]
N --> O
Comments Outside Diff (1)
-
plugins/grep/init.lua, line 275-310 (link)limitis applied independently per pathlimitis passed unchanged into eachsearch_single_pathcall, son00n.fs.grepenforces it per-path. Withpath = ["src", "tests", "plugins"]andlimit = 10, up to 30 total results can be returned. The combinedall_entriesis never re-truncated. This may or may not be the intended UX, but is worth a deliberate decision and a comment.Prompt To Fix With AI
This is a comment left during a code review. Path: plugins/grep/init.lua Line: 275-310 Comment: **`limit` is applied independently per path** `limit` is passed unchanged into each `search_single_path` call, so `n00n.fs.grep` enforces it per-path. With `path = ["src", "tests", "plugins"]` and `limit = 10`, up to 30 total results can be returned. The combined `all_entries` is never re-truncated. This may or may not be the intended UX, but is worth a deliberate decision and a comment. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Prompt To Fix All With AI
### Issue 1
plugins/grep/init.lua:303-319
**Nil path case never executes a search**
When `input.path` is `nil` (the most common invocation — grep the whole project with no path restriction), `path_type` is the Lua string `"nil"`. None of the three branches match, so `search_single_path` is never called, `all_entries` stays empty, and the handler returns `NO_MATCHES` ("No files found") without ever touching the filesystem. This is a regression that breaks the default, path-less usage of the tool.
### Issue 2
plugins/grep/init.lua:303-311
When `path` is `nil` the current code falls through all branches and performs no search. Adding an explicit `nil` branch that calls `search_single_path(nil)` restores the pre-existing behaviour of searching from the project root.
```suggestion
if path == nil then
search_single_path(nil)
elseif path_type == "string" then
search_single_path(path)
elseif path_type == "table" then
for _, p in ipairs(path) do
search_single_path(p)
end
else
return { llm_output = "error: path must be a string or array of strings, got " .. path_type, is_error = true }
end
```
### Issue 3
plugins/grep/init.lua:313-316
**Partial path errors silently dropped**
When the model supplies `["valid/path", "nonexistent/path"]`, results from the valid path are returned with no indication that the other path was not searched. The model will believe it searched both directories. Consider appending a warning to `llm_output` when `#all_errors > 0` but results are still present so the model can correct the call.
### Issue 4
site/docs/content/tools/_index.md:122
**Path type label is incorrect**
The documentation now shows `path` as type `object`, but it accepts either a `string` or an `array of strings`. "object" is a JSON object (key/value pairs), which is never valid here. A label like `string | string[]` or `string or array` would accurately reflect the `oneOf` schema.
### Issue 5
plugins/grep/init.lua:275-310
**`limit` is applied independently per path**
`limit` is passed unchanged into each `search_single_path` call, so `n00n.fs.grep` enforces it per-path. With `path = ["src", "tests", "plugins"]` and `limit = 10`, up to 30 total results can be returned. The combined `all_entries` is never re-truncated. This may or may not be the intended UX, but is worth a deliberate decision and a comment.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(grep): render multi-path headers" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@cursor review verbose=true |
|
Bugbot request id: serverGenReqId_890a102c-7f55-409d-a8c1-c507423ca079 |
Bugbot rules debugNo rules were used for this review. https://cursor.com/docs/bugbot#team-rules Bugbot request id: serverGenReqId_890a102c-7f55-409d-a8c1-c507423ca079 |
Bugbot couldn't run - usage limit reachedBugbot 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_890a102c-7f55-409d-a8c1-c507423ca079) |
…le grep/fusion changes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/grep/init.lua`:
- Around line 292-298: Update the result aggregation around the entries loop and
the return logic at lines 311-314 to track whether any path completed
successfully, independently of whether all_entries contains matches. Return
NO_MATCHES when at least one path succeeds with zero entries and no matches are
collected, while preserving error reporting when every path fails.
- Around line 320-321: Update the merged-result flow in the grep initialization
logic to retain each entry’s modification-time metadata, then globally sort the
combined results by modification time before rendering rather than preserving
search/input-path order. Keep the existing per-path search behavior and ensure
the tool’s promised modification-time ordering is restored.
- Around line 301-309: Update the path-dispatch logic around search_single_path
so a nil or omitted path searches the default workspace path. Preserve the
existing string and table handling, and keep the invalid-type error response for
non-nil unsupported values.
In `@site/docs/content/tools/_index.md`:
- Around line 122-127: Rewrite the grep guidance before the parameter table into
a short, user-focused introduction that states the primary file-content search
use case first. Move escaping, multi-line matching, and unsupported look-around
details into concise bullets, using warm, simple wording while preserving the
existing behavior and constraints.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da9b379d-de59-401d-9261-d2f2bc2e961e
📒 Files selected for processing (5)
changelog.d/222.fixed.mdn00n-agent/src/tools/grep.rsn00n-lua/tests/real_plugins_restore.rsplugins/grep/init.luasite/docs/content/tools/_index.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Do not add unsafe code, FFI, global mutable state,static mut, or unchecked transmute-like behavior without written review, an explicit lint exception, and a SAFETY comment where applicable.
Do not useunwrap,expect,panic!,todo!,unimplemented!, ordbg!in production Rust code; tests are exempt from the unwrap/expect/panic restriction.
Do not silently discard failures withunwrap_or,unwrap_or_default,.ok()onResult, or equivalent defaults; return typed errors, reject the operation, or use an explicitly named fallback with sanitized structured logging.
Use idiomatic Rust, descriptive names, minimal state, and avoid unnecessary comments, bloat, and magic numbers or strings.
Import types at the top of the file and use short imported names; keep constants immediately after imports.
UseResult<T, E>and explicit error handling instead of panics; usethiserrorfor library/domain errors andcolor-eyreat binary edges.
Use#[derive(Copy)]only for structs containing one primitive field.
Prefer structured logging with useful fields and provide helpful, sanitized error messages.
Place unit tests in the same file inside#[cfg(test)]modules; use#[test_case]and snake_case test names.
Propagate typed errors with?,ok_or_else, andmap_err; library crates usethiserrorand binaries usecolor-eyre.
Treat LLM and provider output as untrusted input; validate schemas, domain constraints, and source evidence before persistence or action.
Do not log raw provider payloads, prompts, credentials, or user session data, and never commit credentials, API keys, tokens, cookies, or auth headers.
Validate and authorize HTTP, file, queue, configuration/environment, LLM, and provider-callback inputs before mutation or persistence.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Files:
n00n-agent/src/tools/grep.rsn00n-lua/tests/real_plugins_restore.rs
site/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
User documentation should be warm, simple, concise, easy for non-native English speakers, story-oriented, without em-dashes, emojis, or an AI tone.
Files:
site/docs/content/tools/_index.md
🧠 Learnings (2)
📚 Learning: 2026-07-31T05:40:20.137Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 203
File: changelog.d/203.fixed.md:1-2
Timestamp: 2026-07-31T05:40:20.137Z
Learning: Files in changelog.d/ whose names begin with a numeric fragment identifier are headingless changelog fragments. Treat their contents as entry bodies because generated release sections provide the headings; do not report Markdown MD041 or add an H1 heading to these fragments. This does not apply to changelog.d/README.md.
Applied to files:
changelog.d/222.fixed.md
📚 Learning: 2026-07-31T19:15:04.814Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 206
File: changelog.d/orchestration-hardening.fixed.md:1-1
Timestamp: 2026-07-31T19:15:04.814Z
Learning: Files in changelog.d are changelog fragments intended for user-facing release notes and may begin directly with summary prose. Do not flag a missing Markdown H1 or require an H1 solely because Markdownlint MD041 reports it in these fragment files.
Applied to files:
changelog.d/222.fixed.md
🪛 markdownlint-cli2 (0.23.1)
changelog.d/222.fixed.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (4)
plugins/grep/init.lua (1)
27-40: LGTM!Also applies to: 214-250
n00n-agent/src/tools/grep.rs (1)
76-84: LGTM!changelog.d/222.fixed.md (1)
1-1: LGTM!n00n-lua/tests/real_plugins_restore.rs (1)
21-21: LGTM!Also applies to: 172-223, 845-903
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91b3403b97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # n00n-lua/tests/real_plugins_restore.rs
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@n00n-lua/src/api/fs.rs`:
- Around line 1011-1014: Update the mtime conversion in the surrounding
filesystem result-building function to avoid silently mapping duration_since
failures to zero: propagate the conversion failure as a typed Lua error, or use
an explicitly named fallback accompanied by sanitized structured logging.
Preserve the successful duration.as_secs() behavior and ensure failures cannot
silently affect result ordering.
In `@n00n-lua/tests/real_plugins_restore.rs`:
- Around line 1214-1249: Update
grep_applies_one_limit_and_deduplicates_overlapping_roots to assert that the
project/new.rs: header appears before the project/shared.rs: header in output,
preserving the existing content and deduplication assertions.
In `@plugins/grep/init.lua`:
- Around line 358-360: Update the empty-result branch in the grep result
aggregation to append the existing warning text from all_errors when `#all_errors`
> 0 before returning NO_MATCHES, while preserving the current output for fully
successful empty searches. Add coverage for one empty successful path combined
with one failed path.
In `@site/docs/content/tools/_index.md`:
- Line 127: Update the grep.path parameter description in the tools
documentation to explicitly state that it accepts a string or an array of
strings, replacing the ambiguous “string/array” wording. Include an example such
as ["src", "tests"] while keeping the description warm, simple, and concise.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 66322319-67c1-4194-b3a6-aada1a195536
📒 Files selected for processing (4)
n00n-lua/src/api/fs.rsn00n-lua/tests/real_plugins_restore.rsplugins/grep/init.luasite/docs/content/tools/_index.md
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: Coverage
- GitHub Check: Rustdoc
- GitHub Check: Test (macOS)
- GitHub Check: Build
- GitHub Check: Test (Windows)
- GitHub Check: MSRV (1.97)
- GitHub Check: Lint (Windows)
- GitHub Check: Lint (macOS)
- GitHub Check: Build (Windows)
- GitHub Check: Test
- GitHub Check: Docs
- GitHub Check: Lint
- GitHub Check: Criterion
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (2)
site/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
User documentation should be warm, simple, concise, easy for non-native English speakers, story-oriented, without em-dashes, emojis, or an AI tone.
Files:
site/docs/content/tools/_index.md
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Do not add unsafe code, FFI, global mutable state,static mut, or unchecked transmute-like behavior without written review, an explicit lint exception, and a SAFETY comment where applicable.
Do not useunwrap,expect,panic!,todo!,unimplemented!, ordbg!in production Rust code; tests are exempt from the unwrap/expect/panic restriction.
Do not silently discard failures withunwrap_or,unwrap_or_default,.ok()onResult, or equivalent defaults; return typed errors, reject the operation, or use an explicitly named fallback with sanitized structured logging.
Use idiomatic Rust, descriptive names, minimal state, and avoid unnecessary comments, bloat, and magic numbers or strings.
Import types at the top of the file and use short imported names; keep constants immediately after imports.
UseResult<T, E>and explicit error handling instead of panics; usethiserrorfor library/domain errors andcolor-eyreat binary edges.
Use#[derive(Copy)]only for structs containing one primitive field.
Prefer structured logging with useful fields and provide helpful, sanitized error messages.
Place unit tests in the same file inside#[cfg(test)]modules; use#[test_case]and snake_case test names.
Propagate typed errors with?,ok_or_else, andmap_err; library crates usethiserrorand binaries usecolor-eyre.
Treat LLM and provider output as untrusted input; validate schemas, domain constraints, and source evidence before persistence or action.
Do not log raw provider payloads, prompts, credentials, or user session data, and never commit credentials, API keys, tokens, cookies, or auth headers.
Validate and authorize HTTP, file, queue, configuration/environment, LLM, and provider-callback inputs before mutation or persistence.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Files:
n00n-lua/src/api/fs.rsn00n-lua/tests/real_plugins_restore.rs
🧠 Learnings (9)
📚 Learning: 2026-08-02T22:13:53.026Z
Learnt from: CR
Repo: w0wl0lxd/n00n PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-02T22:13:53.026Z
Learning: Applies to site/**/*.md : User documentation should be warm, simple, concise, easy for non-native English speakers, story-oriented, without em-dashes, emojis, or an AI tone.
Applied to files:
site/docs/content/tools/_index.md
📚 Learning: 2026-07-31T19:15:07.579Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 206
File: changelog.d/orchestration-hardening.fixed.md:1-1
Timestamp: 2026-07-31T19:15:07.579Z
Learning: In this repository, user-facing files in `changelog.d` are changelog fragments and commonly start directly with summary prose. Do not require a Markdown H1 in a changelog fragment solely because Markdownlint MD041 reports a missing first-line heading.
Applied to files:
site/docs/content/tools/_index.md
📚 Learning: 2026-07-31T19:15:09.621Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 206
File: n00n-lua/tests/real_plugins_restore.rs:222-247
Timestamp: 2026-07-31T19:15:09.621Z
Learning: In `n00n-lua/tests/real_plugins_restore.rs`, values embedded in compile-time `#[test_case]` inputs and raw Lua fixture source may remain inline when extracting constants requires runtime templating and reduces integration fixture clarity without improving coverage.
Applied to files:
n00n-lua/tests/real_plugins_restore.rs
📚 Learning: 2026-08-04T02:36:32.766Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 320
File: n00n-lua/tests/plugin_host.rs:4465-4476
Timestamp: 2026-08-04T02:36:32.766Z
Learning: In `n00n-lua`, `PluginHost::unload` must preserve captured Lua plugin state. Tests in `n00n-lua/tests/plugin_host.rs` must verify that both root-scoped and session-scoped payloads remain available after plugin unload.
Applied to files:
n00n-lua/tests/real_plugins_restore.rs
📚 Learning: 2026-08-10T05:11:10.183Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 327
File: n00n-lua/src/runtime.rs:1443-1445
Timestamp: 2026-08-10T05:11:10.183Z
Learning: In `n00n-lua/src/runtime.rs`, `drain_runtime` must continue dispatching runtime tool requests with `nested_only` set to `false` while a lifecycle operation is active. Reentrant agents can dispatch Lua tool requests from another executor thread, so nested-origin markers are not reliable across threads. Restricting lifecycle draining to nested requests can deadlock lifecycle barriers.
Applied to files:
n00n-lua/tests/real_plugins_restore.rs
📚 Learning: 2026-08-02T22:13:53.026Z
Learnt from: CR
Repo: w0wl0lxd/n00n PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-02T22:13:53.026Z
Learning: Applies to **/*_test.rs : Tests must be meaningful rather than tautological, deterministic, and free of arbitrary sleeps; shared constant error/status messages should be asserted against shared constants.
Applied to files:
n00n-lua/tests/real_plugins_restore.rs
📚 Learning: 2026-07-31T07:38:38.628Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 201
File: n00n-providers/src/providers/openai/mod.rs:543-557
Timestamp: 2026-07-31T07:38:38.628Z
Learning: In `n00n-providers/src/providers/openai/mod.rs`, `Model::from_spec` intentionally accepts unknown qualified Codex model IDs such as `codex/unknown-id`. This preserves forward compatibility with future Codex model IDs. Tests must verify this acceptance rather than require rejection.
Applied to files:
n00n-lua/tests/real_plugins_restore.rs
📚 Learning: 2026-08-02T22:13:53.026Z
Learnt from: CR
Repo: w0wl0lxd/n00n PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-02T22:13:53.026Z
Learning: Applies to **/*.rs : Treat LLM and provider output as untrusted input; validate schemas, domain constraints, and source evidence before persistence or action.
Applied to files:
plugins/grep/init.lua
📚 Learning: 2026-07-21T16:54:32.671Z
Learnt from: CR
Repo: w0wl0lxd/n00n PR: 0
File: n00n-lua/src/api/AGENTS.md:0-0
Timestamp: 2026-07-21T16:54:32.671Z
Learning: Applies to n00n-lua/src/api/**/*.lua : Tool handlers must report failures as `{ llm_output = msg, is_error = true }`; a plain string is always a success, and only `is_error` marks the result as an error to the provider.
Applied to files:
plugins/grep/init.lua
🔇 Additional comments (3)
site/docs/content/tools/_index.md (1)
10-10: LGTM!Also applies to: 122-126, 230-249, 348-349, 434-434, 444-448
plugins/grep/init.lua (1)
220-220: LGTM!Also applies to: 230-232, 282-307, 317-323, 332-357, 362-369
n00n-lua/tests/real_plugins_restore.rs (1)
13-13: LGTM!Also applies to: 22-24, 36-38, 176-555, 859-891, 1145-1165, 1251-1300
|
@coderabbitai review\n\nAll currently unresolved review threads are resolved in 23b37fc. The prior Windows failure was code-related: the assertion assumed |
|
🧠 Learnings used
|
1 participant
Fixes common grep tool failures from model-generated inputs:
additionalProperties: falseto schema to reject parameters likecommandthat models sometimes pass instead ofpatternpathschema to accept either a string or an array of strings, handling the case where models pass multiple space-separated pathspattern(notcommand), avoid PCRE look-around, and use arrays for multiple pathsThese changes address the most common failure patterns seen in logs:
command: "rg -n ..."instead ofpattern(?<!...)or(?!...)which Rust regex does not supportNote
Low Risk
Search-tool behavior and validation only; no auth, persistence, or security-sensitive paths. Multi-path merge is additive with minor edge cases around cross-path sorting.
Overview
Hardens the grep tool against frequent model-generated mistakes so searches fail less often and errors are easier to fix.
The grep plugin schema now sets
additionalProperties: false(rejecting stray params likecommand) and allowspathas a string or string array. The handler runs one search per path, merges results, and validates path types; headers use a new formatter so multiple paths show clearly in the UI.The Rust grep backend appends a short note on invalid regex that PCRE look-around (
(?!...),(?<!...)) is unsupported and Rust regex syntax should be used.Docs and prompts (tool description, prompt hint, site tools reference) now tell models to use
patternnotcommand, avoid PCRE look-around, and pass multiple paths as a JSON array. A restore test covers multi-path headers; changelog entry #222 records the fix.Reviewed by Cursor Bugbot for commit 1ab3972. Bugbot is set up for automated code reviews on this repo. Configure here.