Repository navigation
feat(embed): report lifecycle script output - #1284
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesLifecycle script output
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to Lifecycle output is routed through tagged events only for event-mode installs, while standalone and non-event installs retain inherited stdio; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
Greptile SummaryThe PR routes lifecycle-script stdout and stderr through structured install output events for embedding hosts while retaining inherited stdio outside event mode.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "fix(embed): bound lifecycle output recor..." | Re-trigger Greptile |
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 `@crates/aube-scripts/src/lib.rs`:
- Around line 1195-1208: Update the lifecycle output-reading loop around
BufReader::read_until so each record has a bounded maximum size before being
appended to buffer. When the limit is exceeded, either return a controlled error
or report the data as bounded chunks, while preserving normal newline trimming
and reporter.report behavior for records within the limit.
🪄 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: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e85005be-7216-4ba9-aa3b-a3b08d68156a
📒 Files selected for processing (5)
crates/aube-scripts/src/lib.rscrates/aube/src/commands/install/control.rscrates/aube/src/commands/install/mod.rscrates/aube/src/embed.rscrates/aube/tests/embed.rs
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
Summary
InstallEvent::Outputfor embedding hostsMotivation
Embedding hosts such as mise render their own progress UI. Lifecycle scripts currently bypass aube’s event reporter and write directly to the shared terminal, which corrupts those progress displays. Tagged output events let the host print lifecycle logs through its progress renderer without pausing all progress.
Tests
cargo testcargo clippy --all-targets -- -D warningscargo fmt --all -- --checkAI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Medium Risk
Changes lifecycle script I/O for event-mode installs (piping and async forwarding) while preserving inherited stdio elsewhere; incorrect piping or backpressure could affect long-running or verbose scripts.
Overview
Embedding hosts that use event output mode can now receive lifecycle script stdout/stderr instead of those scripts writing directly to the shared terminal.
aube-scriptsadds an optionalScriptOutputReporter: when set, lifecycle children are spawned with piped stdio, output is read line-by-line (with 64 KiB chunks for long lines without newlines), and each chunk is forwarded to the reporter. With no reporter, behavior stays inherited stdio.set_script_settings_with_path_ordernow updates settings fields in place so it does not clear an installed reporter.Install / embed: at install start,
set_output_reporteris wired fromInstallControlwhen output mode is Events. Lines becomeInstallEvent::Outputat info level with codeAUBE_LIFECYCLE_SCRIPT_OUTPUT. The constant is re-exported on the public embed API. An embed integration test asserts both stdout and stderr from a root dev-preinstall hook show up as tagged events.Reviewed by Cursor Bugbot for commit 240b758. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests