fix(n00n-lua): repair test build after caller field landed - #289
fix(n00n-lua): repair test build after caller field landed#289w0wl0lxd wants to merge 15 commits into
Conversation
- Update router test to load queries from fixture file - Add auto-detection patterns for search, skeleton, and trace intents - Expand fixture with 30 labeled queries for better coverage
…views' into fix/live-task-progress-and-tool-views # Conflicts: # n00n-ui/src/components/args_view.rs # n00n-ui/src/components/tool_display.rs
…s-and-tool-views # Conflicts: # n00n-ui/src/chat.rs
…n::Session/PendingAsyncTask The independently-landed session-caller feature added a `caller` field to UiAction::Session and `caller_session_id`/`caller_tool` to PendingAsyncTask, but the test modules were left referencing the old shapes, so `cargo test` /`cargo nextest` failed to compile the n00n-lua test target. Add the missing `caller: _` ignore patterns in session.rs tests and the `caller_session_id`/`caller_tool` fields in the runtime.rs test helper so the suite builds and the parity regression tests can run.
|
Warning Review limit reached
Next review available in: 1 minute 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 (26)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds workload admission controls, Fusion delegation authorization, caller-aware session orchestration, runtime task tracking, Fusion phase events, workflow journal limits, and OpenAI-compatible tool-call serialization updates. ChangesFusion orchestration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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-agent/src/agent/tool_dispatch.rs`:
- Around line 565-574: The tool dispatch admission logic currently charges every
MCP call against the process workload. Update the admission selection near
McpSession::is_tool_read_only(...) to use ToolWorkload::Cheap for read-only
tools and ToolWorkload::Process for mutating tools, while preserving the
existing cancellation and error handling.
In `@n00n-agent/src/fusion/mod.rs`:
- Around line 117-123: Update the derive attributes on FusionDispatchGuard to
remove both Copy and Clone, leaving its existing Debug, PartialEq, and Eq
behavior intact. Keep the single-use state enforced through the existing
authorize method and dispatched field.
- Around line 125-165: Update run_authorized and the FusionDelegateTool dispatch
path to carry and use FusionDispatchGuard instead of the
fusion_delegate_authorized boolean. Construct or pass the guard from
is_fusion_delegate, invoke FusionDispatchGuard::authorize() for each production
delegation using the actual invocation origin, and reject dispatch when
authorization returns an error; remove the boolean-only gate while preserving
the guard’s existing policy checks.
In `@n00n-agent/src/tools/admission.rs`:
- Around line 91-117: Prevent reentrant semaphore waits by adding an in-scope
permit marker or equivalent reentrant acquire behavior in
ToolAdmission::acquire, returning a no-op guard when the caller already holds
the workload permit, and add a nested-acquisition test. In
n00n-agent/src/agent/run.rs lines 487-499, pass the host tool’s in-scope agent
permit into the child agent so acquire_agent reuses it. In
n00n-agent/src/agent/tool_dispatch.rs lines 505-514, propagate that permit
through ToolContext while preserving its current position after
enforce_permission.
In `@n00n-config/src/lib.rs`:
- Around line 2302-2304: Replace the removed
fusion_compaction_sidekick_tier_is_rejected assertion with equivalent coverage
in the Config::validate tests, verifying that a Compaction sidekick tier returns
ConfigError::InvalidFusionSidekickTier { tier: Tier::Compaction }. Keep the
existing validation behavior unchanged and assert the structured error rather
than relying on an unrelated message string.
- Around line 2286-2291: Update the test assertion for
merged.agent.fusion.sidekick_tier to use an imported Tier type instead of the
fully qualified crate::providers::Tier path. Add the import at the top of the
test module, then reference Tier::Medium (or a clear alias) in the assertion
while preserving the existing comparison.
In `@n00n-lua/src/api/session.rs`:
- Around line 138-146: Update the focus extraction in the opts match within the
session handling function to propagate mlua conversion errors with the same `?`
handling used by prompt, title, and parent_id, rather than defaulting conversion
failures to false; preserve false only for the None options case.
In `@n00n-lua/src/runtime.rs`:
- Around line 614-623: Import SessionCaller alongside the existing command
imports, then update active_session_caller to use the short SessionCaller name
for its return type and both agent calls instead of the repeated fully qualified
path.
In `@n00n-lua/tests/plugin_host.rs`:
- Line 5006: Update a representative session test around the UiAction::Session
pattern to bind the caller field instead of discarding it, then assert that it
identifies the invoking session and tool. Keep the existing req and reply_tx
handling unchanged, and ensure the assertion covers both caller identity
components introduced by the session-caller feature.
In `@n00n-ui/src/chat.rs`:
- Around line 1096-1107: Update Chat::handle_event to handle AgentEvent::Retry
explicitly instead of reaching unreachable!, returning ChatEventResult::Continue
for direct or unexpected retry events. Preserve the existing retry behavior for
valid callers while removing the production panic path.
- Around line 182-197: Update the AgentEvent::FusionPhase handling to call the
existing push_fusion_phase helper instead of constructing and pushing
DisplayMessage directly. This must preserve the helper’s whitespace-only label
filtering and label-length limits while retaining the phase-specific display
behavior.
In `@n00n-ui/src/components/mod.rs`:
- Line 208: Import n00n_storage::id::n00nId alongside the module’s other types,
then update the Action::FocusSession variant to use the imported n00nId name
instead of the fully qualified path.
In `@n00n-ui/src/event_loop.rs`:
- Around line 1191-1212: Update the runtime-set mutation helpers remove_runtime
and push_runtime to call sync_runtime_tasks after removing or adding a session,
ensuring every existing runtime receives an up-to-date RuntimeTaskEntry
projection. Keep the existing sync_runtime_tasks call after session creation
unchanged.
- Around line 1128-1155: Replace repeated linear parent lookups in is_ancestor,
root_and_depth, and the callers that perform lineage checks with a per-operation
HashMap<n00nId, Option<n00nId>> parent index built once from the sessions
collection. Traverse parent relationships through the map while preserving the
existing cycle/length bounds and ancestry results, and reuse the same index
throughout sync_runtime_tasks and admit_child rather than rebuilding or scanning
sessions for each query.
- Around line 928-945: Update the session-list filtering in the caller-id
handling around caller_id_result so an unresolved agent caller is not treated
like the host: use caller.is_host() to allow an unfiltered list only for host
callers, while applying lineage_related to agent callers, including those with
no session ID. Leave SessionRequest::List and SessionRequest::Status unchanged
unless the surrounding design explicitly requires closing those read paths.
- Around line 253-258: The task_status assignment in the runtime status
construction must reflect the current session lifecycle, including child
sessions becoming idle. Update the logic around runtime_kind and task_status, or
the emit_status_changes flow, to set RuntimeTaskStatus::Done when the session
reaches SessionStatus::Idle while preserving Running for active sessions, so
descendant admission and task synchronization see completion.
In `@plugins/task/init.lua`:
- Line 90: Replace byte-based truncation in the title construction at
plugins/task/init.lua:90 and the corresponding Fusion “Executing:” construction
at plugins/fusion/init.lua:149 with UTF-8-safe fixed-length prefix handling.
Measure the prefix by characters or valid UTF-8 boundaries, then concatenate the
full result before passing it to n00n.session.new or returning it; add
regression tests covering emoji and CJK at each cutoff boundary.
In `@plugins/team/init.lua`:
- Around line 806-813: Update the title construction in the session
initialization flow to truncate `(input.goal or "background team")` using the
existing UTF-8-aware text helper instead of byte-based string.sub, preserving
the 80-character limit. Remove the redundant pcall-wrapped
n00n.session.set_title call after n00n.session.new, since the title is already
supplied during creation.
In `@plugins/workflow/init.lua`:
- Around line 284-286: Move the MAX_JOURNAL_BYTES check in the workflow journal
loading flow to run after compaction, applying it to the compacted text rather
than the original contents. Preserve the existing error return when the
compacted journal still exceeds the 4 MiB limit, while allowing oversized
journals reduced by compaction to load successfully.
🪄 Autofix (Beta)
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: 57cdc087-8b8c-40f1-a428-d4c5caefc7b9
📒 Files selected for processing (40)
changelog.d/fusion-beta-orchestration.added.mdn00n-agent/src/agent/run.rsn00n-agent/src/agent/tool_dispatch.rsn00n-agent/src/fusion/mod.rsn00n-agent/src/lib.rsn00n-agent/src/tools/admission.rsn00n-agent/src/tools/mod.rsn00n-agent/src/tools/registry.rsn00n-agent/src/types.rsn00n-config/src/lib.rsn00n-lua/src/api/agent.rsn00n-lua/src/api/session.rsn00n-lua/src/api/tool.rsn00n-lua/src/api/util/command.rsn00n-lua/src/lib.rsn00n-lua/src/loader.rsn00n-lua/src/runtime.rsn00n-lua/tests/plugin_host.rsn00n-lua/tests/real_plugins_restore.rsn00n-providers/src/providers/openai_compat.rsn00n-ui/src/app/mod.rsn00n-ui/src/app/tests.rsn00n-ui/src/app/view.rsn00n-ui/src/chat.rsn00n-ui/src/components/mod.rsn00n-ui/src/event_loop.rsplugins/agent_control/init.luaplugins/bash/init.luaplugins/batch/init.luaplugins/blackboard/init.luaplugins/code_execution/init.luaplugins/fusion/init.luaplugins/task/init.luaplugins/team/init.luaplugins/workflow/init.luasrc/cmd/mod.rssrc/cmd/tui_bridge.rssrc/print.rssrc/sdk_mode.rstests/fixtures/explore-queries.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
src/cmd/mod.rsn00n-ui/src/app/view.rsn00n-ui/src/components/mod.rsn00n-agent/src/types.rsn00n-lua/src/lib.rsn00n-agent/src/lib.rsn00n-providers/src/providers/openai_compat.rsn00n-lua/tests/real_plugins_restore.rsn00n-lua/src/api/session.rsn00n-ui/src/app/tests.rsn00n-lua/src/loader.rssrc/print.rssrc/sdk_mode.rsn00n-config/src/lib.rsn00n-agent/src/tools/mod.rsn00n-lua/src/api/util/command.rsn00n-lua/tests/plugin_host.rsn00n-agent/src/tools/admission.rssrc/cmd/tui_bridge.rsn00n-lua/src/api/agent.rsn00n-agent/src/tools/registry.rsn00n-ui/src/chat.rsn00n-agent/src/agent/tool_dispatch.rsn00n-lua/src/api/tool.rsn00n-lua/src/runtime.rsn00n-agent/src/agent/run.rsn00n-ui/src/app/mod.rsn00n-ui/src/event_loop.rsn00n-agent/src/fusion/mod.rs
🧠 Learnings (1)
📚 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/fusion-beta-orchestration.added.md
🪛 markdownlint-cli2 (0.23.1)
changelog.d/fusion-beta-orchestration.added.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (45)
changelog.d/fusion-beta-orchestration.added.md (1)
1-1: LGTM!n00n-lua/tests/real_plugins_restore.rs (1)
420-466: LGTM!n00n-providers/src/providers/openai_compat.rs (1)
639-643: LGTM!Also applies to: 1211-1234
src/cmd/mod.rs (1)
166-169: LGTM!tests/fixtures/explore-queries.json (1)
5-5: LGTM!Also applies to: 29-29
n00n-agent/src/tools/mod.rs (2)
8-19: LGTM!Also applies to: 582-582
314-314: 🔒 Security & PrivacyNo changes needed. Existing Rust constructors initialize
session_id, and no Rust logging path emits the field.n00n-agent/src/tools/registry.rs (2)
20-20: LGTM!Also applies to: 244-246, 264-264, 343-343, 422-422, 470-470, 537-537, 857-863
361-380: 🚀 Performance & ScalabilityNo change needed.
n00n-lua/tests/plugin_host.rs (1)
16-16: LGTM!Also applies to: 359-386, 5050-5050, 5090-5090, 5110-5110
plugins/team/init.lua (1)
1094-1094: LGTM!plugins/workflow/init.lua (1)
41-41: LGTM!Also applies to: 613-615, 1012-1012
n00n-agent/src/agent/run.rs (1)
22-23: LGTM!Also applies to: 889-889
n00n-agent/src/agent/tool_dispatch.rs (1)
16-16: LGTM!Also applies to: 1065-1136, 1156-1373, 2774-2829
n00n-agent/src/fusion/mod.rs (1)
6-10: LGTM!Also applies to: 177-177
n00n-agent/src/lib.rs (1)
22-23: 🗄️ Data Integrity & IntegrationNo consumer imports the removed short fusion paths.
n00n-lua/src/api/agent.rs (1)
117-126: LGTM!Also applies to: 517-519, 713-714, 724-724, 862-862, 1760-1769
n00n-lua/src/api/tool.rs (2)
23-23: LGTM!Also applies to: 129-129, 152-152, 219-221, 648-649, 690-691, 1054-1071, 1203-1203, 1247-1247, 1266-1276, 1672-1696, 1782-1782
1032-1034: 🔒 Security & PrivacyVerify the audience expansion before release.
This change makes every Lua tool registration without
audiencesvisible to every audience. Inventory existing registrations that omitaudiences. Add explicit audiences where broad access is not required. Add denial tests for restricted child-agent paths.As per coding guidelines, “Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.”
Source: Coding guidelines
plugins/agent_control/init.lua (1)
183-183: LGTM!Also applies to: 235-235, 500-500
plugins/bash/init.lua (1)
637-637: LGTM!plugins/batch/init.lua (1)
502-502: LGTM!plugins/blackboard/init.lua (1)
736-736: LGTM!plugins/code_execution/init.lua (1)
287-289: LGTM!plugins/fusion/init.lua (1)
152-156: LGTM!plugins/task/init.lua (1)
337-337: LGTM!n00n-ui/src/event_loop.rs (7)
194-217: The kind derived from the tool name is not persisted, soruntime_kindre-derives a different kind from the title after a restart. The impact is limited to the task picker label. Fixing it requires a persisted field in session metadata, which is out of scope for this change.
26-27: LGTM!Also applies to: 45-47, 71-73
828-833: LGTM!Also applies to: 881-881
986-1003: LGTM!Also applies to: 1020-1049
1113-1127: LGTM!
1157-1189: LGTM!Also applies to: 1488-1492
1004-1019: 🗄️ Data Integrity & IntegrationNo non-repo Lua
parent_idpath to verify.Current tracked Lua files do not pass
parent_idtosession.new, so this concern remains only relevant for external/bundled plugins.n00n-lua/src/api/session.rs (2)
27-36: LGTM!
273-273: LGTM!Also applies to: 294-294, 319-319, 357-357
n00n-lua/src/api/util/command.rs (2)
406-445: LGTM!
457-457: 🗄️ Data Integrity & IntegrationNo change needed.
The existing
UiAction::Sessionconstruction includescaller; the existingSessionRequest::Newconstruction includes bothpromptandtitle; and the non-test struct-variant patterns include the matching fields.n00n-lua/src/lib.rs (1)
18-19: LGTM!n00n-lua/src/runtime.rs (4)
25-25: LGTM!Also applies to: 295-296, 329-330
625-638: LGTM!Also applies to: 649-650, 815-816
1500-1500: LGTM!Also applies to: 2102-2102, 2114-2115, 2979-2980
918-923: 🗄️ Data Integrity & IntegrationNo change needed. The shared task metadata already includes
caller_session_idandcaller_tool, andrun_inline_tasksbuilds the inline scopes from the samePendingAsyncTaskfields.n00n-lua/src/loader.rs (1)
878-896: LGTM!src/cmd/tui_bridge.rs (1)
14-14: LGTM!Also applies to: 198-203
src/print.rs (1)
369-369: LGTM!
| impl FusionDispatchGuard { | ||
| #[must_use] | ||
| pub const fn new( | ||
| enabled: bool, | ||
| classification: DelegationKind, | ||
| audience: ToolAudience, | ||
| ) -> Self { | ||
| Self { | ||
| enabled, | ||
| classification, | ||
| audience, | ||
| dispatched: false, | ||
| } | ||
| } | ||
|
|
||
| /// Authorize one direct delegation from the main agent. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error when Fusion is disabled, policy does not delegate, the | ||
| /// caller is not the main agent, invocation is indirect, or the guard was consumed. | ||
| pub fn authorize(&mut self, origin: FusionInvocationOrigin) -> Result<(), FusionDispatchError> { | ||
| if !self.enabled { | ||
| return Err(FusionDispatchError::Disabled); | ||
| } | ||
| if self.classification != DelegationKind::Delegate { | ||
| return Err(FusionDispatchError::Ineligible); | ||
| } | ||
| if self.audience != ToolAudience::MAIN { | ||
| return Err(FusionDispatchError::InvalidAudience); | ||
| } | ||
| if origin != FusionInvocationOrigin::Direct { | ||
| return Err(FusionDispatchError::IndirectInvocation); | ||
| } | ||
| if self.dispatched { | ||
| return Err(FusionDispatchError::AlreadyDispatched); | ||
| } | ||
| self.dispatched = true; | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: find production uses of the Fusion dispatch guard and its enums.
set -euo pipefail
rg -n -C3 'FusionDispatchGuard|FusionInvocationOrigin|FusionDispatchError|FusionFailure' \
--glob '!**/tests/**'
# Show how dispatch currently authorizes delegation.
rg -n -C6 'fusion_delegate_authorized' n00n-agent/srcRepository: w0wl0lxd/n00n
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== matching files =="
git ls-files | rg '(^|/)fusion.*\.rs$|tool_dispatch\.rs$|lib\.rs$' || true
echo
echo "== candidate symbol occurrences, all tracked files =="
rg -n -C3 'FusionDispatchGuard|FusionInvocationOrigin|FusionDispatchError|FusionFailure|fusion_delegate_authorized|FusionDispatch' . --glob '!**/tests/**' --glob '!**/target/**' || true
echo
echo "== locate fusion/mod.rs and inspect relevant sections =="
if git ls-files 'n00n-agent/src/fusion/mod.rs' | grep -q .; then
wc -l n00n-agent/src/fusion/mod.rs
sed -n '1,240p' n00n-agent/src/fusion/mod.rs | cat -n
fi
echo
echo "== inspect tool_dispatch guard/delegation sections =="
sed -n '330,405p' n00n-agent/src/agent/tool_dispatch.rs | cat -n
sed -n '2740,2845p' n00n-agent/src/agent/tool_dispatch.rs | cat -n 2>/dev/null || true
echo
echo "== inspect lib exports =="
sed -n '1,240p' n00n-agent/src/lib.rs | cat -nRepository: w0wl0lxd/n00n
Length of output: 31801
Wire FusionDispatchGuard into run_authorized.
run_authorized still blocks fusion_delegate with only fusion_delegate_authorized: bool, and that flag is passed from is_fusion_delegate via FusionDelegateTool. The real guard is only constructed in tests, and FusionDispatchGuard::authorize() is not called in production dispatch. Use the guard for the delegation gate so the typed authorization affects runtime behavior.
🤖 Prompt for 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.
In `@n00n-agent/src/fusion/mod.rs` around lines 125 - 165, Update run_authorized
and the FusionDelegateTool dispatch path to carry and use FusionDispatchGuard
instead of the fusion_delegate_authorized boolean. Construct or pass the guard
from is_fusion_delegate, invoke FusionDispatchGuard::authorize() for each
production delegation using the actual invocation origin, and reject dispatch
when authorization returns an error; remove the boolean-only gate while
preserving the guard’s existing policy checks.
There was a problem hiding this comment.
Criterion
Details
| Benchmark suite | Current: 5fdf6ad | Previous: b36e929 | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6649160 ns/iter (± 221072) |
6455055 ns/iter (± 172769) |
1.03 |
fib/jit_watchdog |
2229829 ns/iter (± 42433) |
2494875 ns/iter (± 4312) |
0.89 |
fib/jit_none |
2234682 ns/iter (± 76432) |
2493442 ns/iter (± 15255) |
0.90 |
fib/interp_mlua_hook |
8189033 ns/iter (± 93775) |
7718671 ns/iter (± 72364) |
1.06 |
fib/interp_watchdog |
4365369 ns/iter (± 78050) |
4073914 ns/iter (± 16857) |
1.07 |
fib/interp_none |
4334219 ns/iter (± 23083) |
3956463 ns/iter (± 10333) |
1.10 |
buffer_rw/jit_mlua_hook |
581513 ns/iter (± 3335) |
554260 ns/iter (± 1328) |
1.05 |
buffer_rw/jit_watchdog |
191182 ns/iter (± 433) |
167782 ns/iter (± 628) |
1.14 |
buffer_rw/jit_none |
191287 ns/iter (± 781) |
167759 ns/iter (± 338) |
1.14 |
buffer_rw/interp_mlua_hook |
1050869 ns/iter (± 9455) |
1044457 ns/iter (± 12359) |
1.01 |
buffer_rw/interp_watchdog |
583136 ns/iter (± 9843) |
629318 ns/iter (± 4016) |
0.93 |
buffer_rw/interp_none |
582535 ns/iter (± 7851) |
639390 ns/iter (± 4827) |
0.91 |
splash_render_120x40 |
53160 ns/iter (± 2991) |
66065 ns/iter (± 4263) |
0.80 |
splash_render_200x60 |
194621 ns/iter (± 3327) |
149400 ns/iter (± 22243) |
1.30 |
This comment was automatically generated by workflow using github-action-benchmark.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fdf6ad20b
ℹ️ 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".
| SessionCaller::agent( | ||
| cell.caller_session_id.as_ref().map(ToString::to_string), | ||
| cell.caller_tool.as_ref().map(ToString::to_string), |
There was a problem hiding this comment.
Preserve host identity for detached Lua callbacks
When a Lua slash command such as /sessions or /rename calls n00n.session, RunCommand executes it inside TaskScope::detached, whose caller_session_id is unset. This constructs SessionCaller::agent(None, None), so validate_caller_identity rejects every request with "session request has no invoking session". Detached UI command and keybind callbacks need to retain or receive host identity rather than being classified as anonymous agents.
AGENTS.md reference: AGENTS.md:L78-L80
Useful? React with 👍 / 👎.
| let idx = self.push_runtime(self.ctx.spawn_runtime(session)); | ||
| self.sessions[idx].kind = runtime_kind_for_tool(caller.tool()); | ||
| self.sessions[idx].task_status = RuntimeTaskStatus::Running; |
There was a problem hiding this comment.
Initialize promptless child sessions as idle
When an agent calls n00n.session.new without a prompt, the new runtime remains idle but is unconditionally marked Running. Because its last_status also starts as idle, emit_status_changes never corrects the task status, leaving a permanent running entry that consumes one of the eight active-descendant slots; after enough promptless children, valid child creation is rejected. Set the initial task status from whether work was actually submitted, or force an initial status reconciliation.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Action performedReview finished.
|
- Remove Copy and Clone from FusionDispatchGuard (security: single-use guard) - Use ToolWorkload::Cheap for read-only MCP calls (performance) - Import Tier type in n00n-config test (code style) - Add regression test for Compaction sidekick tier (test coverage) - Propagate focus conversion errors instead of discarding (error handling) - Use UTF-8-aware truncate_text in task/fusion/team plugins (UTF-8 safety) - Remove redundant set_title call in team plugin (cleanup) - Move journal size check after compaction (data integrity)
…, and log fusion authorization failures
- Add is_agent() to SessionCaller and fix the session_id assertion. - Replace disallowed unwrap_or with unwrap_or_else. - Propagate plugin name as caller_tool through run_detached for command handlers.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9eb9be9201
ℹ️ 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".
| } | ||
| let rt = self.remove_runtime(i); | ||
| rt.handles.cancel(); | ||
| self.sync_runtime_tasks(); |
There was a problem hiding this comment.
Cancel or reject descendants before deleting their parent
When a host or ancestor deletes a non-focused session that still has live descendants, this removes and cancels only the selected runtime before rebuilding the task projections. Because LineageSnapshot derives ancestry exclusively from the remaining live parent map, those descendants become orphaned, disappear from the root's task panel and descendant counts, and can continue executing and incurring cost outside the new active-session limits. Either cascade deletion/cancellation through the live subtree or reject deletion until its descendants are gone.
Useful? React with 👍 / 👎.
| let kind = runtime_kind(&session); | ||
| let task_status = if session.meta.parent_id.is_some() { | ||
| RuntimeTaskStatus::Done | ||
| } else { | ||
| RuntimeTaskStatus::Running | ||
| }; |
There was a problem hiding this comment.
Refresh cached runtime metadata after in-place session loads
When focus_session reuses a blank idle runtime to load a stored child session, App::load_session replaces the session and its parent_id, but these newly cached kind and task_status values are only computed during spawn_runtime. The reused runtime can therefore remain labeled as main and Running; since both the old and loaded apps are idle, emit_status_changes may never correct the status. Recompute this metadata, reset last_status, and resync projections whenever a session is loaded in place.
Useful? React with 👍 / 👎.
|
This branch depends on SessionCaller infrastructure that has been removed from main. The caller-field feature is no longer present, so this PR is obsolete. |
Pull request was closed
Context
Parity pass against upstream
tontinton/maki(fork basef434c380d5, 2026-07-19). n00n is already aggressively syncing maki: the 22 maki-only commits between the fork base and maki/main are functionally already present in n00n's tree (sql/toml indexing, slug manifests, per-provider base-url overrides, mcp capabilities,tool_searchdefer, startup theme, lua mixed-table key preservation, DeepSeek cache-hit pricing,convert_messagescontent emission, Ctrl+T plan reopen, interpreterreset_clock, context-window sync, output cap, tool-output dedupe — all already in HEAD, so there was nothing to port there).Change
The one thing blocking
cargo nextestwas a pre-existing break in n00n-lua's test target: the independently-landed session-caller feature addedcallertoUiAction::Sessionandcaller_session_id/caller_tooltoPendingAsyncTask, but the test modules still used the old shapes. This PR adds the missingcaller: _ignore patterns (session.rstests) and thecaller_session_id/caller_toolfields (runtime.rs test helper) so the suite compiles and the parity regression tests run.This is test-only; no production code changed.
Verification
cargo nextest runfor the three parity regression tests now passes (3 passed, 3122 skipped).