fix(plugins): publish live preview buffers for bash, task, workflow - #125
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (4)
📝 WalkthroughWalkthroughBash, task, and workflow tools now register execution buffers for live UI updates. New tests verify matching ChangesLive progress previews
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ToolTest
participant PluginHandler
participant PluginContext
participant AgentEvent
ToolTest->>PluginHandler: Execute tool input
PluginHandler->>PluginContext: Register live buffer
PluginContext-->>AgentEvent: Emit LiveToolBuf
AgentEvent-->>ToolTest: Verify preview id and content
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Criterion
Details
| Benchmark suite | Current: eb8fb94 | Previous: 9ccb16b | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6622692 ns/iter (± 5729) |
6855088 ns/iter (± 75157) |
0.97 |
fib/jit_watchdog |
1307396 ns/iter (± 5315) |
2223249 ns/iter (± 14481) |
0.59 |
fib/jit_none |
1305908 ns/iter (± 4472) |
2216333 ns/iter (± 38058) |
0.59 |
fib/interp_mlua_hook |
6632131 ns/iter (± 38953) |
7949331 ns/iter (± 84712) |
0.83 |
fib/interp_watchdog |
2423089 ns/iter (± 357867) |
4274359 ns/iter (± 10615) |
0.57 |
fib/interp_none |
2448599 ns/iter (± 121258) |
4271893 ns/iter (± 21683) |
0.57 |
buffer_rw/jit_mlua_hook |
593648 ns/iter (± 3273) |
579723 ns/iter (± 921) |
1.02 |
buffer_rw/jit_watchdog |
75496 ns/iter (± 342) |
191820 ns/iter (± 310) |
0.39 |
buffer_rw/jit_none |
75536 ns/iter (± 249) |
192178 ns/iter (± 1652) |
0.39 |
buffer_rw/interp_mlua_hook |
900187 ns/iter (± 1264) |
1039945 ns/iter (± 15439) |
0.87 |
buffer_rw/interp_watchdog |
401486 ns/iter (± 979) |
589561 ns/iter (± 1625) |
0.68 |
buffer_rw/interp_none |
401333 ns/iter (± 2080) |
588483 ns/iter (± 1264) |
0.68 |
splash_render_120x40 |
54441 ns/iter (± 10778) |
72259 ns/iter (± 5943) |
0.75 |
splash_render_200x60 |
94174 ns/iter (± 15498) |
176308 ns/iter (± 14428) |
0.53 |
This comment was automatically generated by workflow using github-action-benchmark.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98bb1ba0c6
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/real_plugins_restore.rs`:
- Around line 51-114: Extract the inline workflow tool name, live-preview ID,
event sequence value, and receive timeout into named constants used by
assert_publishes_live_buf and its test cases in
n00n-lua/tests/real_plugins_restore.rs:51-114. In
n00n-lua/tests/task_policy.rs:575-600, likewise define and use named constants
for the task preview ID and event sequence, replacing the inline magic values
without changing behavior.
🪄 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: 35bd3ff4-3811-429f-9047-33b939df5dbf
📒 Files selected for processing (6)
changelog.d/125.fixed.mdn00n-lua/tests/real_plugins_restore.rsn00n-lua/tests/task_policy.rsplugins/bash/init.luaplugins/task/init.luaplugins/workflow/init.lua
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Lint (macOS)
- GitHub Check: Test (macOS)
- GitHub Check: Rustdoc
- GitHub Check: Build (Windows)
- GitHub Check: MSRV (1.97)
- GitHub Check: Unused deps
- GitHub Check: Build
- GitHub Check: Test (Windows)
- GitHub Check: Coverage
- GitHub Check: Lint (Windows)
- GitHub Check: Test
- GitHub Check: Docs
- GitHub Check: Lint
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Criterion
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Non-trivial or multi-file changes should use a dedicated git worktree and new branch; do not modify unrelated user changes, force-push, or push to main.
Ship finished work with a clear Conventional Commit message, a pushed branch, and a draft pull request; never add AI-agent attribution to authored content.
Before investigating unfamiliar failures or third-party behavior, research documented behavior first and distinguish unrelated baseline failures from regressions in the touched surface.
Use structural tools before broad searches: prefercodegraphorarborfor cross-file relationships,indexbefore reading files, targeted reads, parallel calls, andcode_executionfor filtering large outputs.
Files:
changelog.d/125.fixed.mdplugins/bash/init.luan00n-lua/tests/task_policy.rsplugins/task/init.luaplugins/workflow/init.luan00n-lua/tests/real_plugins_restore.rs
plugins/**/*.lua
📄 CodeRabbit inference engine (AGENTS.md)
Built-in Lua plugins belong under
./pluginsand should use the repository's plugin tooling and conventions.
Files:
plugins/bash/init.luaplugins/task/init.luaplugins/workflow/init.lua
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Workspace Rust lint rules are mandatory: deny unsafe code, productionunwrap_used,expect_used,panic,todo!,unimplemented!,dbg!, wildcard imports, and silent-default error handling.
Do not add unsafe code, FFI, global mutable state,static mut, or unchecked transmute-like behavior without written review and an explicit crate-level lint exception.
Use explicit error handling withResult<T, E>rather than panics; propagate typed errors with?,ok_or_else, andmap_err.
Usethiserrorfor library and domain-specific errors, andcolor-eyreat binary edges.
Do not silently discard errors with.ok(),unwrap_or,unwrap_or_default, or equivalent defaults; return an error, reject the operation, or use an explicitly named fallback with sanitized structured logging.
Follow Rust idioms, use descriptive variable and function names, avoid unnecessary state and bloat, and keep each line justified.
Import types at the top of the file, use short imported names instead of inline qualified paths, and place constants immediately after imports.
Do not use inline magic numbers or strings; deriveCopyonly for structs with one primitive field.
Use structured logging with useful fields and provide helpful, sanitized error messages.
Do not commit credentials, API keys, tokens, cookies, authorization headers, or user data, and do not log raw provider payloads, prompts, credentials, or session data.
Validate and authorize HTTP, file, queue, configuration/environment, LLM output, and provider callbacks before mutation or persistence.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Write meaningful, non-tautological, non-flaky tests; avoid arbitrary sleeps, and define shared constant error/status messages for assertions.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
All crates must opt into the workspace lint configuration with
[lints] workspace = true; the root workspace lint configuration is authoritative.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
**/*.{rs,ron,json,toml,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Treat LLM and provider output as untrusted input; validate it against schemas, domain constraints, and source evidence before persistence or action.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
🪛 markdownlint-cli2 (0.23.0)
changelog.d/125.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 (5)
plugins/bash/init.lua (1)
611-611: LGTM!plugins/task/init.lua (1)
142-142: LGTM!Also applies to: 155-155
plugins/workflow/init.lua (1)
612-616: LGTM!Also applies to: 809-809
n00n-lua/tests/real_plugins_restore.rs (1)
25-25: LGTM!changelog.d/125.fixed.md (1)
1-1: LGTM!
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Criterion'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: 98bb1ba | Previous: 5883bbf | Ratio |
|---|---|---|---|
buffer_rw/jit_watchdog |
167958 ns/iter (± 207) |
75535 ns/iter (± 1409) |
2.22 |
buffer_rw/jit_none |
167837 ns/iter (± 388) |
76858 ns/iter (± 905) |
2.18 |
splash_render_120x40 |
77273 ns/iter (± 1120) |
32557 ns/iter (± 1739) |
2.37 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
n00n-lua/tests/real_plugins_restore.rs (1)
97-98: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCancel and join on every failure path.
A timeout,
expect, orassert!failure before these lines leaves the spawned execution—and potentially its child process—running. Use a cleanup guard or ensure cancellation and joining happen before propagating any failure.🤖 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-lua/tests/real_plugins_restore.rs` around lines 97 - 98, Update the test execution flow around cancel and execution.join so every failure path, including timeouts, expect failures, and assertions, first cancels the spawned execution and joins its handle. Use a cleanup guard or equivalent scope-based handling, while preserving the existing panic propagation and cancellation behavior.
🤖 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/tests/real_plugins_restore.rs`:
- Around line 80-83: Update the receive loop around rx.recv_timeout in the live
preview test to enforce a single overall deadline rather than resetting the
5-second timeout for each event. Create one Instant-based deadline before the
loop, calculate the remaining duration before every recv_timeout call, and
preserve the existing matching LiveToolBuf handling while failing once the total
wait expires.
---
Outside diff comments:
In `@n00n-lua/tests/real_plugins_restore.rs`:
- Around line 97-98: Update the test execution flow around cancel and
execution.join so every failure path, including timeouts, expect failures, and
assertions, first cancels the spawned execution and joins its handle. Use a
cleanup guard or equivalent scope-based handling, while preserving the existing
panic propagation and cancellation behavior.
🪄 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: 8f192863-5b4a-420e-8cb3-3c7c933ac2c1
📒 Files selected for processing (2)
n00n-lua/tests/real_plugins_restore.rsn00n-lua/tests/task_policy.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Test (Windows)
- GitHub Check: Lint (Windows)
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Workspace Rust lint rules are mandatory: deny unsafe code, productionunwrap_used,expect_used,panic,todo!,unimplemented!,dbg!, wildcard imports, and silent-default error handling.
Do not add unsafe code, FFI, global mutable state,static mut, or unchecked transmute-like behavior without written review and an explicit crate-level lint exception.
Use explicit error handling withResult<T, E>rather than panics; propagate typed errors with?,ok_or_else, andmap_err.
Usethiserrorfor library and domain-specific errors, andcolor-eyreat binary edges.
Do not silently discard errors with.ok(),unwrap_or,unwrap_or_default, or equivalent defaults; return an error, reject the operation, or use an explicitly named fallback with sanitized structured logging.
Follow Rust idioms, use descriptive variable and function names, avoid unnecessary state and bloat, and keep each line justified.
Import types at the top of the file, use short imported names instead of inline qualified paths, and place constants immediately after imports.
Do not use inline magic numbers or strings; deriveCopyonly for structs with one primitive field.
Use structured logging with useful fields and provide helpful, sanitized error messages.
Do not commit credentials, API keys, tokens, cookies, authorization headers, or user data, and do not log raw provider payloads, prompts, credentials, or session data.
Validate and authorize HTTP, file, queue, configuration/environment, LLM output, and provider callbacks before mutation or persistence.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Write meaningful, non-tautological, non-flaky tests; avoid arbitrary sleeps, and define shared constant error/status messages for assertions.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
All crates must opt into the workspace lint configuration with
[lints] workspace = true; the root workspace lint configuration is authoritative.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
**/*.{rs,ron,json,toml,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Treat LLM and provider output as untrusted input; validate it against schemas, domain constraints, and source evidence before persistence or action.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Non-trivial or multi-file changes should use a dedicated git worktree and new branch; do not modify unrelated user changes, force-push, or push to main.
Ship finished work with a clear Conventional Commit message, a pushed branch, and a draft pull request; never add AI-agent attribution to authored content.
Before investigating unfamiliar failures or third-party behavior, research documented behavior first and distinguish unrelated baseline failures from regressions in the touched surface.
Use structural tools before broad searches: prefercodegraphorarborfor cross-file relationships,indexbefore reading files, targeted reads, parallel calls, andcode_executionfor filtering large outputs.
Files:
n00n-lua/tests/task_policy.rsn00n-lua/tests/real_plugins_restore.rs
🔇 Additional comments (2)
n00n-lua/tests/real_plugins_restore.rs (1)
12-12: LGTM!Also applies to: 25-25, 39-42, 55-78, 84-95, 109-109
n00n-lua/tests/task_policy.rs (1)
40-41: LGTM!Also applies to: 577-602
Summary
The
bash,task, andworkflowplugins were building aToolViewpreview buffer during execution but never publishing it as a live buffer. As a result the main chat showed an empty/collapsed preview while the tool ran; progress was only visible after the tool finished (or by expanding after the fact).Publish each preview buffer with
ctx:live_buf(...)immediately after it is created, matching the pattern used bybatch,code_execution,explore_result, andactivity_preview. The UI can now poll the live buffer and render the header + rolling tail of output as it updates.Changes
plugins/bash/init.lua: publish the bash output view as a live buffer.plugins/task/init.lua: publish the subagent progress preview as a live buffer.plugins/workflow/init.lua: publish the workflow progress preview as a live buffer.Test plan
styluaon all touched Lua filescargo check -p n00n-luaandcargo check -p n00n-uicargo test -p n00n-lua --test spec --test real_plugins_restorepasscargo test -p n00n-lua --test code_execution_policystill fails with a pre-existinginterpreter_calls_advertised_tool_end_to_end("lua thread disconnected") failure unrelated to this change.