fix(agent,ui,lua,plugins): live task progress and tool views - #274
fix(agent,ui,lua,plugins): live task progress and tool views#274w0wl0lxd wants to merge 35 commits into
Conversation
w0wl0lxd
commented
Aug 3, 2026
w0wl0lxd
commented
- 96baf72 Merge pull request fix(providers): generate idempotency keys for provider POSTs #234 from w0wl0lxd/fix/provider-idempotency
- f5be3f3 Merge pull request fix(providers): handle empty Devin tool arguments #248 from w0wl0lxd/hotfix/devin-empty-tool-args
- fbdb608 fix(session): preserve recursive state on resume (fix(session): preserve recursive state on resume #257)
- de42a54 feat(storage): add versioned session state snapshots (feat(storage): add versioned session state snapshots #260)
- a12a180 Merge branch 'main' into fix/provider-token-usage
- 8bac102 fix(providers,ui): demote expected agent and provider log noise (fix(providers,ui): demote expected agent and provider log noise #220)
- 75efd70 Merge branch 'main' into fix/provider-token-usage
- b39f755 Merge pull request fix(providers): derive non-cached input from Devin usage #223 from w0wl0lxd/fix/provider-token-usage
- 6878099 fix(storage): merge main's write-side record limiting with branch's read-side decode limiting (fix(runtime): harden session reads and process shutdown #224)
- b36e929 fix(n00n-lua): demote glob permission-denied walk errors to debug (fix(n00n-lua): demote glob permission-denied walk errors to debug #232)
- cd94311 Merge remote-tracking branch 'origin/main' into fix/live-task-progress-and-tool-views
- 683e57a fix(agent,ui,lua,plugins): live task progress and tool views
- 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
|
Warning Review limit reached
Next review available in: 5 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 (22)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates Fusion delegation, tool dispatch authorization, workload admission, session provenance, runtime task tracking, UI task navigation, event naming, provider serialization, workflow journals, and tool registration metadata. ChangesFusion orchestration and dispatch
Tool admission and registration
Session provenance and runtime tasks
Protocol and UI updates
Workflow and guidance
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 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: 683e57a | Previous: b36e929 | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6523996 ns/iter (± 48191) |
6455055 ns/iter (± 172769) |
1.01 |
fib/jit_watchdog |
2443466 ns/iter (± 4618) |
2494875 ns/iter (± 4312) |
0.98 |
fib/jit_none |
2459213 ns/iter (± 59750) |
2493442 ns/iter (± 15255) |
0.99 |
fib/interp_mlua_hook |
7796479 ns/iter (± 22475) |
7718671 ns/iter (± 72364) |
1.01 |
fib/interp_watchdog |
3856867 ns/iter (± 15343) |
4073914 ns/iter (± 16857) |
0.95 |
fib/interp_none |
3767609 ns/iter (± 11844) |
3956463 ns/iter (± 10333) |
0.95 |
buffer_rw/jit_mlua_hook |
554686 ns/iter (± 1406) |
554260 ns/iter (± 1328) |
1.00 |
buffer_rw/jit_watchdog |
168014 ns/iter (± 422) |
167782 ns/iter (± 628) |
1.00 |
buffer_rw/jit_none |
168115 ns/iter (± 235) |
167759 ns/iter (± 338) |
1.00 |
buffer_rw/interp_mlua_hook |
1040902 ns/iter (± 3737) |
1044457 ns/iter (± 12359) |
1.00 |
buffer_rw/interp_watchdog |
646780 ns/iter (± 4118) |
629318 ns/iter (± 4016) |
1.03 |
buffer_rw/interp_none |
647637 ns/iter (± 5931) |
639390 ns/iter (± 4827) |
1.01 |
splash_render_120x40 |
49793 ns/iter (± 3339) |
66065 ns/iter (± 4263) |
0.75 |
splash_render_200x60 |
164597 ns/iter (± 16536) |
149400 ns/iter (± 22243) |
1.10 |
This comment was automatically generated by workflow using github-action-benchmark.
Resolved conflicts: - n00n-agent/src/agent/run.rs: kept SessionIdentity import, kept parameter name 'enabled' - n00n-agent/src/agent/tool_dispatch.rs: removed leftover conflict marker - n00n-agent/src/fusion/mod.rs: accepted origin/main structure, added back FusionContinuation enum and continuation methods from PR - n00n-agent/src/lib.rs: merged FusionContinuation, FusionFailure, FusionRequestDecision exports - n00n-agent/src/types.rs: removed duplicate FusionPhase variant, kept FusionPhaseChanged - n00n-lua/tests/plugin_host.rs: merged SessionIdentity and ToolWorkload imports - plugins/fusion/init.lua: accepted origin/main UTF-8 safe header implementation - src/print.rs: kept FusionPhaseChanged event handler - src/sdk_mode.rs: kept FusionPhaseChanged event handler
…assification - Integrate new Fusion orchestration changes from main - Add FUSION_DELEGATE_BLOCKED constant and request_kind tracking - Update FusionPhase to include Idle state - Add MAX_DELEGATIONS_BEFORE_LEAD_LOCK constant - Integrate tool filter changes with fusion_delegate filtering - Update fusion plugin header to use UTF-8 safe truncation - Use sanitize_error in fusion plugin error handling - Keep branch's classification behavior while integrating main's orchestration
- Remove duplicate FusionPhase variant in AgentEvent - Update AgentParams.identity to session_id (main API change) - Remove unused with_dynamic_mcp_tools method - Add missing ContentBlock::File match arm - Clone RequestOptions and ToolFilter where needed - Add Default impl for DelegationKind - Remove unused SessionIdentity imports
…task-router-classification
- Add missing fields to ImageSource struct in test - Add caller field to UiAction::Session pattern matches - Add missing fields to PendingAsyncTask in test - Use clone_from for clippy compliance - Allow dead_code for RuntimeTaskStatus::Error - Replace unwrap_or with match for typed error handling
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
n00n-agent/src/agent/run.rs (1)
532-545: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDistinguish cancellation and channel closure from an explicit denial.
.ok()on the cancellation race andResult::okonrecv_asyncdiscard both failure reasons. A cancelled run and a disconnected channel both surface asAgentError::Config { "Full-history replay was not approved." }. Two problems follow:
- A cancelled run no longer reports
AgentError::Cancelled, soAgent::runskips the cancellation branch that marks Fusion state cancelled and sanitizes history.- Operators cannot tell denial from cancellation or transport failure.
The fail-closed default is correct. Name the fallback and log the reason.
🛠️ Proposed fix
- let response = self.cancel.race(response_rx.recv_async()).await; - drop(response_rx); - let approved = response - .ok() - .and_then(Result::ok) - .and_then(|answer| PermissionAnswer::decode(&answer)) - .is_some_and(|answer| answer.is_allow()); - if approved { - Ok(()) - } else { - Err(AgentError::Config { - message: format!("Full-history replay was not approved. {scope}"), - }) - } + let response = self.cancel.race(response_rx.recv_async()).await; + drop(response_rx); + let answer = match response { + Err(_) => return Err(AgentError::Cancelled), + Ok(Err(error)) => { + warn!(?error, "history replay: approval channel closed"); + return Err(AgentError::Config { + message: format!("Full-history replay approval channel closed. {scope}"), + }); + } + Ok(Ok(answer)) => answer, + }; + if PermissionAnswer::decode(&answer).is_some_and(|answer| answer.is_allow()) { + Ok(()) + } else { + Err(AgentError::Config { + message: format!("Full-history replay was not approved. {scope}"), + }) + }Based on learnings from the coding guidelines: "Do not silently discard failures with
unwrap_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."🤖 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/agent/run.rs` around lines 532 - 545, Update the response handling in the cancellation race around PermissionAnswer::decode to preserve and distinguish cancellation, channel-receive failure, and explicit denial instead of collapsing them through ok(). Return AgentError::Cancelled for cancellation, propagate or map the receive failure to an appropriate typed error, and use an explicitly named fail-closed denial fallback with sanitized structured logging that records the reason; keep approval returning Ok(()) and denial returning the existing configuration error.Source: Coding guidelines
🤖 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 `@changelog.d/286.fixed.md`:
- Line 1: Replace the merge and conflict-resolution text in the changelog
fragment with a concise user-facing description of the live task progress or
tool-dispatch behavior fixed by this change, removing internal branch names and
merge details.
In `@n00n-agent/src/agent/run.rs`:
- Around line 383-386: Update the request initialization flow around
self.fusion_classification to reuse the classification already computed by
crate::fusion::classify_delegation(&input.message). In the later
state.set_request_kind call, pass self.fusion_classification instead of invoking
classify_delegation again, preserving the existing classification value as the
single source of truth.
- Around line 2160-2183: Update the cancellation path so `FusionState::cancel`
or the corresponding cancellation handling in `Agent::run` emits
`AgentEvent::FusionPhaseChanged` with `FusionPhase::Cancelled` after
transitioning the state. Preserve the existing `Err(AgentError::Cancelled)`
result and ensure the event is emitted once as the terminal fusion phase.
- Line 1240: Update the ContentBlock::File handling in estimate_message_tokens
to detect inline FileSource.file_data and add its token estimate before falling
back to IMAGE_TOKEN_ESTIMATE; preserve the existing fixed estimate for files
without inline data and the current ContentBlock::Image behavior.
In `@n00n-agent/src/agent/tool_dispatch.rs`:
- Around line 2275-2289: Pass DelegationKind enum values directly to the test
cases in n00n-agent/src/agent/tool_dispatch.rs lines 2275-2289, remove the
string parameter and match conversion, and retain the existing authorization
assertion. In n00n-agent/src/fusion/mod.rs lines 625-645, change the expected
test parameter to DelegationKind and compare classify_delegation(prompt)
directly against it instead of formatting it for string comparison.
In `@n00n-agent/src/fusion/mod.rs`:
- Around line 571-585: Remove the unused lowercase summary and mutation scan in
the fusion routing block, including MUTATION_SIGNALS if it has no other
consumers. Reconcile the Lead/Delegate route with Agent::apply_fusion_route: if
lead-lane retention is intended, delete the
FusionRoute::Switch(FusionLane::Sidekick) branch and the associated
MAX_DELEGATIONS_BEFORE_LEAD_LOCK constant; otherwise update apply_fusion_route
to honor the Switch payload and adjust the existing test accordingly.
- Around line 423-426: Replace the inline terminal-phase match in the transition
logic around FusionPhase with a call to FusionPhase::is_terminal(). Keep the
existing transition behavior while ensuring enter_terminal and run.rs use the
same terminal-state definition.
- Around line 16-31: Replace the duplicated FUSION_DENIAL_MESSAGES literals with
values derived from every applicable FusionDispatchError variant, keeping
is_guard_denial behavior unchanged. Add an exhaustive match-based test that
constructs each denial variant and compares its to_string() output, so adding a
new variant requires updating the denial list and test.
In `@n00n-agent/src/prompts/general.md`:
- Line 27: Insert a blank line immediately after the “# Conventions” heading in
general.md, before the first bullet, to satisfy the Markdown heading-spacing
requirement. Do not change the surrounding convention content.
In `@n00n-agent/src/tools/admission.rs`:
- Around line 174-196: Replace the body of process_permit_blocks_until_release
with the spawned-waiter pattern used by agent_permit_blocks_until_release,
holding the sole process permit while asserting a second acquire remains
pending, then dropping the first permit and verifying the waiter succeeds. Move
the already-cancelled token assertion into a separate test so cancellation
behavior is tested independently.
- Around line 91-117: Update Admission::acquire around semaphore.acquire_arc()
to race cancellation with ctx.deadline, ensuring permit acquisition cannot wait
indefinitely when the caller has no cancellation. Measure the wait duration and
emit a warning when it exceeds the configured threshold, while preserving the
existing granted debug log and cancellation error behavior.
In `@n00n-agent/src/tools/mod.rs`:
- Line 348: Restore the session identity in the tool-call context flow: update
Agent::tool_context() to populate ToolContext::identity for main-agent contexts,
and ensure AgentContext::from plus AgentContext::to_tool_context() preserve it
through n00n.agent.session(). Alternatively, explicitly make session_id
sufficient for child-session creation while retaining the existing behavior for
contexts with identity.
In `@n00n-lua/src/api/session.rs`:
- Line 142: Update the focus option handling in session.new to distinguish
absent or nil values from conversion failures: default only missing or nil focus
to false, while propagating a typed error for invalid values such as strings.
Remove the unwrap_or_else fallback that suppresses conversion errors and
preserve the existing typed conversion/error path.
In `@n00n-lua/src/runtime.rs`:
- Around line 2447-2448: Update the warm-cache TaskCell construction to copy the
original caller_session_id and caller_tool provenance from the live task,
alongside root_buf. Ensure click callbacks using n00n.session.* retain the
originating session and tool instead of defaulting to SessionCaller::agent(None,
None).
In `@n00n-lua/tests/real_plugins_restore.rs`:
- Around line 341-363: Update the replay test around the n00n.fs.write mock so
compaction succeeds while capturing the written journal content in a Lua
upvalue. Keep the n00n.agent.session guard, then assert the script’s agent call
returns the cached “new” value and that the captured compacted content excludes
the stale “old” value, ensuring replay is actually exercised.
In `@n00n-ui/src/event_loop.rs`:
- Around line 1194-1215: Update sync_runtime_tasks and the lifecycle paths that
handle terminal agent events, runtime removal, restore, and initial runtime
creation so runtime task projections are refreshed after each status or
hierarchy change. Ensure child runtimes transition from Running to Done or Error
based on their terminal result before rebuilding RuntimeTaskEntry values, so
completed tasks disappear from the picker and no longer count toward
MAX_ACTIVE_ROOT_DESCENDANTS.
In `@plugins/task/init.lua`:
- Around line 90-91: Update the title construction before n00n.session.new to
truncate the selected description or prompt by UTF-8 characters rather than
bytes, ensuring the resulting title is always valid UTF-8 while retaining the
existing 80-character limit and fallback text.
In `@plugins/team/init.lua`:
- Line 806: Update the title construction near n00n.session.new to use
n00n.ui.truncate_text with the 80-character limit and select its head result,
preserving the existing fallback to "background team" while ensuring multi-byte
UTF-8 goals are not truncated mid-codepoint.
- Around line 811-813: Remove the redundant pcall block invoking
n00n.session.set_title after n00n.session.new already receives title. Preserve
the creation flow and do not add a replacement call unless a specific
requirement establishes that the second update is necessary.
In `@plugins/workflow/init.lua`:
- Around line 612-615: Update the append handling around next_text so exceeding
MAX_JOURNAL_BYTES returns the already-built out result and skips the journal
write instead of raising an error, allowing subsequent agent calls to continue.
Use the configured limit wording or existing limit symbol rather than hardcoding
“4 MiB” in the message.
- Around line 284-286: Derive the journal-size limit text from MAX_JOURNAL_BYTES
instead of hardcoding “4 MiB” in both rejection messages. Update the
read-rejection path in plugins/workflow/init.lua:284-286 and the
append-rejection path in plugins/workflow/init.lua:612-615 so both messages
consistently reflect the shared constant.
- Around line 308-326: Guard the compaction write in load_journal with
journal.lock: acquire the lock, reread the current journal contents, rebuild the
compacted representation from that latest data, write it only when needed, and
release the lock on every success or failure path. Keep make_agent’s append
behavior and journal.text synchronization consistent with the locked rewrite.
---
Outside diff comments:
In `@n00n-agent/src/agent/run.rs`:
- Around line 532-545: Update the response handling in the cancellation race
around PermissionAnswer::decode to preserve and distinguish cancellation,
channel-receive failure, and explicit denial instead of collapsing them through
ok(). Return AgentError::Cancelled for cancellation, propagate or map the
receive failure to an appropriate typed error, and use an explicitly named
fail-closed denial fallback with sanitized structured logging that records the
reason; keep approval returning Ok(()) and denial returning the existing
configuration error.
🪄 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: 93116d22-2ef0-4e67-82f5-5f15f47f9744
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
changelog.d/286.fixed.mdchangelog.d/311.fixed.mdchangelog.d/fusion-beta-orchestration.added.mdn00n-agent/src/agent/run.rsn00n-agent/src/agent/tool_dispatch.rsn00n-agent/src/fusion/mod.rsn00n-agent/src/headless.rsn00n-agent/src/prompts/general.mdn00n-agent/src/prompts/system.mdn00n-agent/src/tools/admission.rsn00n-agent/src/tools/mod.rsn00n-agent/src/tools/registry.rsn00n-agent/src/types.rsn00n-lua/src/api/agent.rsn00n-lua/src/api/session.rsn00n-lua/src/api/tool.rsn00n-lua/src/api/util/command.rsn00n-lua/src/api/util/ctx.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/agent/agent_loop.rsn00n-ui/src/app/mod.rsn00n-ui/src/app/tests.rsn00n-ui/src/app/view.rsn00n-ui/src/chat.rsn00n-ui/src/components/args_view.rsn00n-ui/src/components/mod.rsn00n-ui/src/components/tool_display.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/tui_bridge.rssrc/print.rssrc/sdk_mode.rs
💤 Files with no reviewable changes (1)
- changelog.d/311.fixed.md
📜 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:
n00n-lua/src/loader.rsn00n-lua/src/api/util/ctx.rsn00n-agent/src/types.rssrc/sdk_mode.rsn00n-providers/src/providers/openai_compat.rsn00n-ui/src/components/mod.rsn00n-ui/src/app/view.rsn00n-ui/src/app/tests.rsn00n-lua/src/api/util/command.rsn00n-lua/src/lib.rsn00n-lua/tests/real_plugins_restore.rsn00n-agent/src/headless.rssrc/print.rsn00n-ui/src/chat.rsn00n-agent/src/tools/mod.rsn00n-ui/src/components/tool_display.rsn00n-lua/tests/plugin_host.rssrc/cmd/tui_bridge.rsn00n-lua/src/api/agent.rsn00n-agent/src/tools/admission.rsn00n-lua/src/api/session.rsn00n-ui/src/components/args_view.rsn00n-agent/src/tools/registry.rsn00n-lua/src/runtime.rsn00n-lua/src/api/tool.rsn00n-ui/src/agent/agent_loop.rsn00n-ui/src/app/mod.rsn00n-ui/src/event_loop.rsn00n-agent/src/agent/tool_dispatch.rsn00n-agent/src/fusion/mod.rsn00n-agent/src/agent/run.rs
🧠 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/286.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/286.fixed.mdchangelog.d/fusion-beta-orchestration.added.md
🪛 markdownlint-cli2 (0.23.2)
changelog.d/286.fixed.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
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)
n00n-agent/src/prompts/general.md
[warning] 26-26: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (62)
changelog.d/fusion-beta-orchestration.added.md (1)
1-1: LGTM!n00n-agent/src/prompts/general.md (1)
29-29: 🎯 Functional CorrectnessAlign the commit policy across both prompt files.
n00n-agent/src/prompts/general.md:29forbids all commits, whilen00n-agent/src/prompts/system.md:32permits commit, push, force-push, and amend when asked. If both prompts are loaded, this is a cross-file instruction conflict. Use one explicit policy in both files and confirm that prompt assembly preserves it.
n00n-agent/src/prompts/general.md#L29-L29: replace the absolute prohibition or remove the exception, according to the intended policy.n00n-agent/src/prompts/system.md#L32-L32: use the same wording and identify the authorizing user if the exception remains.n00n-lua/src/api/agent.rs (2)
117-127: LGTM!Also applies to: 517-519, 656-718, 775-775, 868-868, 1377-1377, 1556-1556, 1762-1772
856-856: 🗄️ Data Integrity & IntegrationNo change needed. Nested
n00n.agent.session()requests already use the live child session as caller, soEventLoop::caller_id_result()does not reject them.n00n-lua/src/api/util/ctx.rs (1)
77-87: LGTM!Also applies to: 628-643
n00n-agent/src/headless.rs (1)
392-392: LGTM!Also applies to: 654-654
n00n-ui/src/agent/agent_loop.rs (1)
339-369: LGTM!n00n-ui/src/event_loop.rs (3)
26-73: LGTM!Also applies to: 194-217, 250-308, 830-835, 931-957, 989-1052, 1492-1496
903-929: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuthorization Bypass (CWE-862): Missing Authorization
Reachability path
● Entry n00n-agent/src/agent/tool_dispatch.rs:747 process_tool_calls │ ▼ ● Sink n00n-ui/src/event_loop.rsAuthorize cross-session mutation requests.
Delete,Prompt,Cancel,Focus, andSetTitleresolve caller-selected session IDs without usingcaller. A non-host caller can control, rename, delete, or redirect focus to an unrelated session. Add one target-authorization helper and apply it before each operation.As per coding guidelines, “Validate and authorize ... inputs before mutation or persistence.”
#!/bin/bash set -euo pipefail rg -n -C 8 'SessionRequest::(Delete|Prompt|Cancel|Focus|SetTitle)|caller_id_result|lineage_related' \ n00n-ui/src/event_loop.rs n00n-lua/src/api/session.rs n00n-lua/src/api/util/command.rsAlso applies to: 1058-1112
958-983: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuthorization Bypass (CWE-862): Missing Authorization
Reachability path
● Entry n00n-agent/src/agent/tool_dispatch.rs:747 process_tool_calls │ ▼ ● Sink n00n-ui/src/event_loop.rsAuthorize
Statusbefore returning session data.
SessionRequest::Status { id }parses the requested ID and returns session output, cwd, and paused-team data without checkingcaller. A non-host caller can select an unrelated live session ID and read its data. Apply a caller-to-target authorization check before reading the runtime.As per coding guidelines, “Validate and authorize ... inputs before mutation or persistence.”
#!/bin/bash set -euo pipefail rg -n -C 8 'SessionRequest::(Live|Status)|caller_id_result|lineage_related' \ n00n-ui/src/event_loop.rs n00n-lua/src/api/session.rs n00n-lua/src/api/util/command.rssrc/cmd/tui_bridge.rs (1)
14-14: LGTM!Also applies to: 196-203, 287-291, 364-368, 410-414
n00n-ui/src/app/mod.rs (1)
64-64: LGTM!Also applies to: 132-191, 275-275, 394-394, 683-741, 928-935, 1007-1017, 1717-1717
n00n-ui/src/components/mod.rs (1)
195-225: LGTM!n00n-ui/src/app/view.rs (1)
133-142: LGTM!n00n-ui/src/chat.rs (1)
181-196: LGTM!Also applies to: 1012-1025, 1040-1050
n00n-lua/src/api/session.rs (1)
27-35: LGTM!Also applies to: 274-274, 295-295, 320-320, 358-358
n00n-lua/src/api/util/command.rs (1)
406-445: LGTM!Also applies to: 457-457, 502-502
n00n-lua/src/lib.rs (1)
19-20: LGTM!n00n-ui/src/app/tests.rs (1)
1442-1467: LGTM!Also applies to: 2274-2274, 2291-2291, 3884-3910
n00n-agent/src/types.rs (1)
943-943: LGTM!n00n-providers/src/providers/openai_compat.rs (1)
639-643: LGTM!Also applies to: 1211-1233
n00n-ui/src/components/args_view.rs (1)
27-35: LGTM!Also applies to: 152-152, 291-307, 448-466
n00n-ui/src/components/tool_display.rs (1)
448-455: LGTM!Also applies to: 1178-1204
src/print.rs (1)
372-372: LGTM!src/sdk_mode.rs (1)
1060-1060: LGTM!n00n-lua/src/loader.rs (1)
1024-1043: LGTM!n00n-agent/src/fusion/mod.rs (3)
701-727: This test hardcodes the same literals asFUSION_DENIAL_MESSAGES, so it cannot detect drift fromFusionDispatchError. Already covered in the comment on lines 16-31.
124-170: LGTM!
261-304: LGTM!n00n-agent/src/agent/run.rs (8)
57-78: LGTM!
552-561: LGTM!
801-858: LGTM!
888-920: LGTM!
743-743: LGTM!
1083-1107: LGTM!
1467-1519: LGTM!Also applies to: 1528-1539, 1603-1641
1851-1922: LGTM!n00n-agent/src/agent/tool_dispatch.rs (4)
330-339: LGTM!
813-822: LGTM!Also applies to: 845-854
2192-2260: LGTM!Also applies to: 2296-2296, 2306-2306
771-776: 🔒 Security & PrivacySensitive Data Exposure (CWE-532): Insertion of Sensitive Information into Log File
Reachability: Internal
Reachability path
● Entry n00n-agent/src/agent/run.rs:183 Agent │ ▼ ● Hop n00n-agent/src/fusion/mod.rs:134 new │ ▼ ● Sink n00n-agent/src/agent/tool_dispatch.rsConfirm that
schema::previewsanitizes tool input before logging.This
debug!emitsinput_previewderived from the raw tool-call input. Tool input can carry user file contents, shell commands, or prompt text. Confirm thatcrate::tools::schema::previewredacts sensitive values rather than only truncating them.As per coding guidelines: "Do not log raw provider payloads, prompts, credentials, or user session data."
#!/bin/bash # Description: Inspect the preview helper used for tool-input logging. set -euo pipefail fd -t f 'schema.rs' n00n-agent/src/tools --exec ast-grep outline {} --match preview --view expanded \; rg -n -C 12 'pub fn preview' n00n-agent/src/toolsn00n-agent/src/tools/admission.rs (4)
13-52: LGTM!
74-86: LGTM!
119-148: LGTM!
198-277: LGTM!n00n-agent/src/tools/mod.rs (2)
8-19: LGTM!
623-623: LGTM!Also applies to: 644-645
plugins/team/init.lua (1)
1094-1094: LGTM!plugins/workflow/init.lua (1)
1012-1012: LGTM!plugins/agent_control/init.lua (1)
183-183: LGTM!Also applies to: 235-235, 500-500
plugins/bash/init.lua (1)
637-637: LGTM!n00n-lua/tests/real_plugins_restore.rs (1)
826-830: LGTM!n00n-agent/src/tools/registry.rs (1)
20-20: LGTM!Also applies to: 244-246, 264-264, 343-343, 361-380, 422-422, 470-470, 537-537, 857-863
n00n-lua/src/api/tool.rs (2)
23-23: LGTM!Also applies to: 129-129, 152-152, 219-222, 648-649, 690-691, 1054-1072, 1203-1203, 1247-1247, 1266-1276, 1672-1696, 1782-1782
1032-1037: 🔒 Security & PrivacyAuthorization Bypass (CWE-284)
Reachability: External
Reachability path
● Entry n00n-lua/src/api/session.rs:19 roundtrip │ ▼ ● Hop n00n-lua/src/runtime.rs │ ▼ ● Hop n00n-lua/src/api/util/ctx.rs:629 agent_context_to_tool_context_drops_tool_use_id_and_sink │ ▼ ● Sink n00n-lua/src/api/tool.rsVerify the audience expansion before release.
parse_audience(None)now exposes every omitted-audience Lua tool to all audiences. This includes delegated subagents. Verify that each registration withoutaudiencesis safe forresearch_subandgeneral_sub, or keep the prior restrictive default.As per coding guidelines, “Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.”
#!/bin/bash set -euo pipefail # Inspect every Lua tool registration and identify registrations that omit # an explicit audience scope. rg -n -C 18 'n00n\.api\.register_tool\s*\(' --glob '*.lua' # Inspect delegated-audience construction and filtering paths. rg -n -C 8 'research_sub|general_sub|ToolAudience::all|parse_audience|definitions_active' \ n00n-lua n00n-agentn00n-lua/src/runtime.rs (1)
25-25: LGTM!Also applies to: 325-326, 359-360, 644-680, 879-880, 982-987, 1827-1827, 2435-2435, 3399-3400
n00n-lua/tests/plugin_host.rs (1)
17-19: LGTM!Also applies to: 362-389, 707-742, 5760-5760, 5804-5804, 5844-5844, 5864-5864
plugins/fusion/init.lua (1)
39-55: LGTM!Also applies to: 90-109, 144-148
plugins/task/init.lua (1)
337-337: LGTM!plugins/batch/init.lua (1)
509-509: LGTM!plugins/blackboard/init.lua (1)
736-736: LGTM!plugins/code_execution/init.lua (1)
287-289: LGTM!
| @@ -0,0 +1 @@ | |||
| fixed: Merge origin/main into fix/live-task-router-classification, resolving conflicts in fusion orchestration and tool dispatch | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the merge-log text with a user-facing change description.
Line [1] describes a branch merge and conflict resolution. It does not describe the behavior fixed for users. Replace the internal branch name and merge details with the live task progress or tool-dispatch behavior delivered by this change.
Based on learnings, changelog.d fragments are user-facing release-note entries, so this line should describe the fix rather than the merge process.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 `@changelog.d/286.fixed.md` at line 1, Replace the merge and
conflict-resolution text in the changelog fragment with a concise user-facing
description of the live task progress or tool-dispatch behavior fixed by this
change, removing internal branch names and merge details.
Source: Learnings
| self.fusion_classification = crate::fusion::classify_delegation(&input.message); | ||
| if self.config.fusion.enabled { | ||
| self.fusion_state = Some(FusionState::new_lead()); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Reuse the stored classification instead of classifying twice.
Line 383 stores classify_delegation(&input.message) in self.fusion_classification. Line 428 calls classify_delegation(&input.message) again for the same input. The second call repeats a lowercase allocation and ~45 substring scans, and it creates a second source of truth for the same value.
Pass self.fusion_classification to state.set_request_kind instead.
🤖 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/agent/run.rs` around lines 383 - 386, Update the request
initialization flow around self.fusion_classification to reuse the
classification already computed by
crate::fusion::classify_delegation(&input.message). In the later
state.set_request_kind call, pass self.fusion_classification instead of invoking
classify_delegation again, preserving the existing classification value as the
single source of truth.
| .map_or(IMAGE_TOKEN_ESTIMATE, |data| { | ||
| count_tokens_with_tokenizer(tokenizer, data) | ||
| }), | ||
| ContentBlock::Image { .. } | ContentBlock::File { .. } => IMAGE_TOKEN_ESTIMATE, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the ContentBlock::File variant and how it is constructed.
set -euo pipefail
fd -t f -e rs . n00n-providers/src --exec rg -n -C 6 'File\s*\{' {} \;
rg -n -C 4 'ContentBlock::File' --type=rustRepository: w0wl0lxd/n00n
Length of output: 8776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File definitions and source implementations =="
sed -n '480,545p' n00n-providers/src/types.rs
rg -n -C 4 'impl FileSource|fn file_id|fn file_data|file_data|source\.file_data|ContentBlock::File' n00n-providers/src --type=rust
echo
echo "== Token estimator around image/file =="
sed -n '1220,1250p' n00n-agent/src/agent/run.rs
echo
echo "== Estimate message definition/usages =="
rg -n -C 5 'fn estimate_message_tokens|context_size|try_auto_compact|IMAGE_TOKEN_ESTIMATE|IMAGE_TOKEN' --type=rustRepository: w0wl0lxd/n00n
Length of output: 17683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FileSource identifier and constructor members =="
rg -n -C 8 'pub fn identifier|fn file_data|FileSource::file_data|file_data:' n00n-providers/src/types.rs n00n-agent/src --type=rust
echo
echo "== Search for all FileSource construction =="
rg -n -C 4 'FileSource::|file_data\s*:\s*Some|file_url\s*:\s*Some\(' --type=rust .
echo
echo "== Run.rs token constants and estimator context =="
sed -n '1,80p' n00n-agent/src/agent/run.rs
sed -n '1200,1275p' n00n-agent/src/agent/run.rs
sed -n '1,80p' n00n-agent/src/agent/run.rs | rg -n 'IMAGE_TOKEN_ESTIMATE|IMAGE_TOKEN|context_size|try_auto_compact|estimate_message_tokens' -C 3 || trueRepository: w0wl0lxd/n00n
Length of output: 13130
Count ContentBlock::File``file_data when estimating context.
FileSource.file_data is part of the openai provider payload, but estimate_message_tokens maps every ContentBlock::File to the fixed IMAGE_TOKEN_ESTIMATE. If inline file data reaches compactable context, context_size under-counts and try_auto_compact can trigger late. Add a token estimate for FileSource.file_data before falling back to the image estimate.
🤖 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/agent/run.rs` at line 1240, Update the ContentBlock::File
handling in estimate_message_tokens to detect inline FileSource.file_data and
add its token estimate before falling back to IMAGE_TOKEN_ESTIMATE; preserve the
existing fixed estimate for files without inline data and the current
ContentBlock::Image behavior.
|
|
||
| #[test] | ||
| fn fusion_cancellation_emits_cancelled_terminal_phase() { | ||
| smol::block_on(async { | ||
| let provider = MockProvider::cancel_on_request(Vec::new(), 0); | ||
| let mut history = History::new(Vec::new()); | ||
| let (mut agent, event_rx) = | ||
| make_agent_with_config(provider, &mut history, fusion_enabled_config()); | ||
| let mut input = default_input(); | ||
| input.message = "grep for TODO markers".into(); | ||
|
|
||
| let result = agent.run(input).await; | ||
|
|
||
| assert!(matches!(result, Err(AgentError::Cancelled))); | ||
| let phases: Vec<_> = drain_events(&event_rx) | ||
| .into_iter() | ||
| .filter_map(|envelope| match envelope.event { | ||
| AgentEvent::FusionPhaseChanged { phase, .. } => Some(phase), | ||
| _ => None, | ||
| }) | ||
| .collect(); | ||
| assert_eq!(phases, [FusionPhase::Planning, FusionPhase::Cancelled]); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate every emitter of the Cancelled Fusion phase event.
set -euo pipefail
rg -n -C 6 'FusionPhaseChanged' --type=rust
rg -n -C 6 'emit_fusion_phase' --type=rustRepository: w0wl0lxd/n00n
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Inspect referenced file and nearby implementation without failing on no matches.
set -euo pipefail
echo "== file exists =="
test -f n00n-agent/src/agent/run.rs && echo yes || echo no
echo "== relevant snippets =="
sed -n '400,470p;2140,2195p' n00n-agent/src/agent/run.rs | cat -n
echo "== broader searches =="
rg -n -C 4 'FusionPhaseChanged|emit_fusion_phase|Cancelled|AgentEvent::Fusion|FusionPhase' n00n-agent/src/agent/run.rs || true
echo "== call sites for cancel =="
rg -n -C 5 '\.cancel\(\)|AgentError::Cancelled|state\.cancel' n00n-agent/src/agent/run.rs || trueRepository: w0wl0lxd/n00n
Length of output: 21752
🏁 Script executed:
#!/bin/bash
# Inspect where fusion state can acquire the Cancelled phase and where it is emitted.
set -euo pipefail
echo "== fusion files =="
fd -p '.*fusion.*' . -t f | sed 's#^\./##'
echo "== declarations and Cancelled references =="
rg -n -C 4 'enum FusionPhase|Cancelled|transition|fn cancel|fn fail|FusionPhase::Cancelled|phase_mut|set_phase|emit_fusion_phase' --type=rust
echo "== run cancellation branches and completion emission path =="
sed -n '520,710p;790,880p;1010,1045p;1060,1090p' n00n-agent/src/agent/run.rs | cat -nRepository: w0wl0lxd/n00n
Length of output: 344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== n00n-agent/src/fusion/mod.rs relevant definitions =="
sed -n '1,260p' n00n-agent/src/fusion/mod.rs | cat -n
echo "== FusionState::cancel/fail implementations in repository =="
rg -n -C 8 'impl FusionState|struct FusionState|fn cancel|fn fail|FusionPhase::Cancelled|Cancelled' n00n-agent/src/fusion/mod.rs n00n-agent/src n00n-agent -g '*.rs' || true
echo "== all FusionPhaseChanged occurrences outside run.rs =="
rg -n -C 4 'FusionPhaseChanged' n00n-agent/src n00n-agent src -g '*.rs' || trueRepository: w0wl0lxd/n00n
Length of output: 50370
Emit the Cancelled Fusion phase from FusionState::cancel.
FusionState::cancel() transitions the internal phase to Cancelled, but Agent::run only calls state.cancel() and returns Err(AgentError::Cancelled). The current AgentEvent::FusionPhaseChanged emitters only emit Planning, Executing, LeadFallback, Reviewing, and Complete, so this test expects an event that is never sent. Make state.cancel() emit the terminal Cancelled phase, or make Agent::run emit it after the cancellation handling.
🤖 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/agent/run.rs` around lines 2160 - 2183, Update the
cancellation path so `FusionState::cancel` or the corresponding cancellation
handling in `Agent::run` emits `AgentEvent::FusionPhaseChanged` with
`FusionPhase::Cancelled` after transitioning the state. Preserve the existing
`Err(AgentError::Cancelled)` result and ensure the event is emitted once as the
terminal fusion phase.
| #[test_case(false, "Delegate" ; "disabled")] | ||
| #[test_case(true, "Bypass" ; "bypass")] | ||
| #[test_case(true, "LeadOnly" ; "lead only")] | ||
| fn fusion_dispatch_guard_denies_non_delegate_policy(enabled: bool, policy: &str) { | ||
| use crate::fusion::{DelegationKind, FusionDispatchGuard, FusionInvocationOrigin}; | ||
|
|
||
| let mut guard = | ||
| let classification = match policy { | ||
| "Delegate" => DelegationKind::Delegate, | ||
| "Bypass" => DelegationKind::Bypass, | ||
| _ => DelegationKind::LeadOnly, | ||
| }; | ||
| let guard = | ||
| FusionDispatchGuard::new(enabled, classification, crate::tools::ToolAudience::MAIN); | ||
| assert!(guard.authorize(FusionInvocationOrigin::Direct).is_err()); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Pass domain enums to #[test_case] instead of strings. Both tests take a &str parameter and convert it to a DelegationKind inside the body. The conversion is the shared root cause: it moves a type error into runtime string matching, so a typo in a case label degrades silently instead of failing to compile. n00n-agent/src/agent/tool_dispatch.rs already passes FusionInvocationOrigin values directly at lines 2291-2292, so the pattern is supported.
n00n-agent/src/agent/tool_dispatch.rs#L2275-L2289: replace the"Delegate"/"Bypass"/"LeadOnly"string arguments withDelegationKindvalues and delete thematchblock, whose_ => DelegationKind::LeadOnlyarm currently absorbs typos into a still-passing case.n00n-agent/src/fusion/mod.rs#L625-L645: change the expected parameter toDelegationKindand replaceassert_eq!(format!("{:?}", classify_delegation(prompt)), expected)with a direct value comparison.
📍 Affects 2 files
n00n-agent/src/agent/tool_dispatch.rs#L2275-L2289(this comment)n00n-agent/src/fusion/mod.rs#L625-L645
🤖 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/agent/tool_dispatch.rs` around lines 2275 - 2289, Pass
DelegationKind enum values directly to the test cases in
n00n-agent/src/agent/tool_dispatch.rs lines 2275-2289, remove the string
parameter and match conversion, and retain the existing authorization assertion.
In n00n-agent/src/fusion/mod.rs lines 625-645, change the expected test
parameter to DelegationKind and compare classify_delegation(prompt) directly
against it instead of formatting it for string comparison.
| forwarded.background = false | ||
| local prompt = "Use the team tool now. Do not only describe this request.\n\n" .. n00n.json.encode(forwarded) | ||
| local id, err = n00n.session.new({ prompt = prompt, focus = false }) | ||
| local title = (input.goal or "background team"):sub(1, 80) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the UTF-8-safe truncation helper instead of :sub(1, 80).
Lua string.sub indexes bytes. A goal containing multi-byte UTF-8 can be cut mid-codepoint, so title becomes invalid UTF-8. That title is passed to n00n.session.new at line 807, stored, and rendered in the UI.
n00n.ui.truncate_text is the established helper for this. plugins/workflow/init.lua line 506 already uses n00n.ui.truncate_text(aopts.prompt, NAME_LABEL_MAX).head.
🐛 Proposed fix
- local title = (input.goal or "background team"):sub(1, 80)
+ local title = n00n.ui.truncate_text(input.goal or "background team", 80).head📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local title = (input.goal or "background team"):sub(1, 80) | |
| local title = n00n.ui.truncate_text(input.goal or "background team", 80).head |
🤖 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 `@plugins/team/init.lua` at line 806, Update the title construction near
n00n.session.new to use n00n.ui.truncate_text with the 80-character limit and
select its head result, preserving the existing fallback to "background team"
while ensuring multi-byte UTF-8 goals are not truncated mid-codepoint.
| pcall(function() | ||
| n00n.session.set_title({ id = id, title = title }) | ||
| end) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
The redundant set_title call is still present.
The change description states that the post-creation title update was removed because title is now passed to n00n.session.new at line 807. This pcall block still calls n00n.session.set_title with the same title. It repeats work already done at creation, and the pcall discards any error without logging.
Remove the block, or state why the second call is required.
🧹 Proposed cleanup
- pcall(function()
- n00n.session.set_title({ id = id, title = title })
- end)
return n00n.json.encode({ agent_id = id, status = "started", title = title })🤖 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 `@plugins/team/init.lua` around lines 811 - 813, Remove the redundant pcall
block invoking n00n.session.set_title after n00n.session.new already receives
title. Preserve the creation flow and do not add a replacement call unless a
specific requirement establishes that the second update is necessary.
| -- Older runs could append the same key more than once. Keep the last value | ||
| -- for replay, but rewrite the file once so retention is bounded by unique | ||
| -- agent calls rather than by every retry. | ||
| local lines = {} | ||
| for _, key in ipairs(order) do | ||
| local encoded, encode_err = n00n.json.encode({ k = key, v = cache[key] }) | ||
| if not encoded then | ||
| return nil, path, nil, "failed to encode compact workflow journal: " .. tostring(encode_err) | ||
| end | ||
| lines[#lines + 1] = encoded | ||
| end | ||
| local compact_text = table.concat(lines, "\n") .. "\n" | ||
| if compact_text ~= text then | ||
| local write_ok, write_err = n00n.fs.write(path, compact_text) | ||
| if not write_ok then | ||
| return nil, path, nil, "failed to compact workflow journal: " .. tostring(write_err) | ||
| end | ||
| end | ||
| return cache, path, compact_text |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether load_journal callers hold a lock and whether n00n.fs.write is atomic.
set -euo pipefail
rg -n -C 12 'load_journal' plugins/workflow/init.lua
rg -n -C 8 'journal.lock' plugins/workflow/init.lua
rg -n -C 10 'fn .*fs_write|"write"' n00n-lua/src/apiRepository: w0wl0lxd/n00n
Length of output: 11627
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map compact relevant sections and lock/runtime definitions.
wc -l plugins/workflow/init.lua
sed -n '260,340p' plugins/workflow/init.lua
sed -n '830,885p' plugins/workflow/init.lua
sed -n '496,626p' plugins/workflow/init.lua
echo '--- fs write definitions/usages ---'
rg -n -C 20 'fn write|write\(' n00n-lua/src/api/fs.rs
echo '--- workflow concurrency/concurrent dispatch terms ---'
rg -n -C 4 'workflow|resume|run_id|batch|team|concurrent|dispatch|semaphore|lock|workflow_tool|workflow ' plugins/workflow/init.luaRepository: w0wl0lxd/n00n
Length of output: 50371
Guard load_journal compaction with the journal lock.
load_journal can overwrite the journal via n00n.fs.write without holding journal.lock, while make_agent appends under that mutex and replaces journal.text with its full contents. Concurrent resumes for the same run_id can therefore have one process overwrite an entry appended by another. Add a lock acquire/read-compact-write/release for compacted journals, or make the rewrite an exclusive replace operation that appends do not race against.
🤖 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 `@plugins/workflow/init.lua` around lines 308 - 326, Guard the compaction write
in load_journal with journal.lock: acquire the lock, reread the current journal
contents, rebuild the compacted representation from that latest data, write it
only when needed, and release the lock on every success or failure path. Keep
make_agent’s append behavior and journal.text synchronization consistent with
the locked rewrite.
| local next_text = (journal.text or "") .. line .. "\n" | ||
| if #next_text > MAX_JOURNAL_BYTES then | ||
| error("workflow journal exceeds the 4 MiB limit", 0) | ||
| end |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
A rejected append discards an agent result that was already paid for.
The size check runs after subagent.launch completed at lines 554-562 and after out was built at lines 583-594. When the journal reaches the limit, this raises an error, so the caller loses the completed result and the tokens spent on it. Every later agent call in the run fails the same way with no recovery path.
Consider returning out and skipping the journal write when the limit is reached, so the run continues and only replay caching is lost.
The message also hardcodes "4 MiB"; see the consolidated comment on the limit text.
🤖 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 `@plugins/workflow/init.lua` around lines 612 - 615, Update the append handling
around next_text so exceeding MAX_JOURNAL_BYTES returns the already-built out
result and skips the journal write instead of raising an error, allowing
subsequent agent calls to continue. Use the configured limit wording or existing
limit symbol rather than hardcoding “4 MiB” in the message.
… state persistence
- Add caller authorization checks to session operations (Delete, Prompt, Cancel, Focus, SetTitle, Status) in event_loop.rs to prevent unauthorized cross-session mutations - Remove sensitive input_preview from tool dispatch debug log to avoid logging potentially sensitive tool input - Change default tool audience from ToolAudience::all() to ToolAudience::MAIN to prevent unauthorized tool exposure to subagents - Add explicit audiences to tools that should be available to subagents (glob, grep, read, webfetch, bash, todo_write)
|
I have addressed the CodeRabbit security review findings:
Please re-review these changes. |
|
@coderabbitai review |
|
|
@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: 904534129a
ℹ️ 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".
| }, | ||
| self.audience, | ||
| ))), | ||
| identity: None, |
There was a problem hiding this comment.
Setting identity to None makes every Lua tool receive an AgentContext without an identity. n00n.agent.session() rejects that state with session identity is unavailable, so foreground task, team, workflow, and Fusion paths that use subagent.launch() cannot start child agents even when the parent has a valid session_id.
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.
A newly spawned child is marked Running, but no AgentEvent::Done, AgentEvent::Error, or cancellation path ever changes this field or calls sync_runtime_tasks() afterward. Completed children therefore remain shown as running forever, and after eight historical children the MAX_ACTIVE_ROOT_DESCENDANTS check rejects all subsequent background agents.
Useful? React with 👍 / 👎.
| if caller_id.is_some_and(|caller_id| !self.lineage_related(caller_id, id)) { | ||
| return Err("caller is not authorized to prompt this session".into()); | ||
| } |
There was a problem hiding this comment.
When a tool calls n00n.session.prompt() without an explicit session id, the None branch targets whichever session is currently focused and bypasses the lineage check applied only in the Some(id) branch. If the user changes focus while a background tool is running, that tool can inject a prompt into an unrelated root session; resolve the focused id first and apply the same authorization check before mutation.
AGENTS.md reference: AGENTS.md:L78-L80
Useful? React with 👍 / 👎.
| auto_tier = auto_tier, | ||
| audience = "general_sub", |
There was a problem hiding this comment.
This launch no longer supplies the previously enforced trusted prompt-injection appendix, include_mcp = false, or the orchestration-tool denylist. Because subagent.launch() defaults MCP inclusion to true, a Fusion sidekick processing hostile repository or provider content now receives configured MCP capabilities and loses the explicit instruction that treated that content as data; restore the trusted system appendix and scoped allowlist/denylist on this call.
AGENTS.md reference: AGENTS.md:L75-L80
Useful? React with 👍 / 👎.
| description = input.description, | ||
| prompt = prompt, | ||
| subagent_type = subagent_type, | ||
| model_spec = input.model, |
There was a problem hiding this comment.
Any model-resolution, session-setup, provider, or budget error returned by this subagent.launch() reaches the following sanitize_error(err) call, but this commit deletes the only definition of sanitize_error. The intended error result is therefore replaced by a Lua attempt to call a nil value handler failure instead of a sanitized Fusion failure.
Useful? React with 👍 / 👎.
|
Closing as superseded by the live-progress work already on main. This stale aggregate is conflicting and retains unresolved identity, lifecycle, authorization, and trust-boundary defects. |
Pull request was closed
1 participant
