Add TUI lifecycle smoke harness#255
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 38 minutes and 11 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new smoke:tui npm script and an end-to-end TUI smoke runner that builds a temp repo, drives Aimux through tmux, and captures artifacts. It also threads selected environment variables into dashboard startup and reconciles restart results after healthy verification. ChangesTUI smoke runner
Dashboard command environment
Runtime restart reconciliation
Sequence Diagram(s)TUI smoke runsequenceDiagram
participant SmokeTui as "smoke:tui"
participant TuiSmoke as "scripts/tui-smoke.mjs"
participant Tmux as tmux
participant Dashboard as dashboard
participant Agent as "smoke agent"
SmokeTui->>TuiSmoke: node scripts/tui-smoke.mjs
TuiSmoke->>Tmux: start detached dashboard session
TuiSmoke->>Dashboard: wait for stable markers
TuiSmoke->>Agent: spawn and read ready
TuiSmoke->>Agent: send prompt and verify echo
TuiSmoke->>Tmux: capture artifacts and clean up sessions
Restart reconciliationsequenceDiagram
participant Restart as restartAimuxControlPlane
participant Coherence as buildRuntimeCoherenceReport
participant Reconcile as reconcileProjectResultsWithVerification
participant Projects as projects
Restart->>Coherence: collect before and after reports
Coherence-->>Restart: verification
Restart->>Reconcile: apply healthy after-report to failed services
Reconcile-->>Projects: update service status and state
Restart->>Projects: count failures from reconciled statuses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/dashboard/command-spec.test.ts (1)
17-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a shell-escaping case for env values.
This test covers spaces and secret omission, but the new shell interpolation should also lock down values containing
'or shell metacharacters soshellQuoteregressions are caught.Suggested test extension
const spec = getDashboardCommandSpec("/tmp/repo", { - AIMUX_HOME: "/tmp/custom-home", + AIMUX_HOME: "/tmp/custom'home; echo unsafe", AIMUX_DAEMON_PORT: "43219", AIMUX_CLI_BIN: "/tmp/bin/aimux smoke", SECRET_TOKEN: "not-for-tmux", } as NodeJS.ProcessEnv); const command = spec.dashboardCommand.args[1] ?? ""; - expect(command).toContain("env AIMUX_HOME='/tmp/custom-home'"); + expect(command).toContain(`env AIMUX_HOME='/tmp/custom'"'"'home; echo unsafe'`);🤖 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 `@src/dashboard/command-spec.test.ts` around lines 17 - 29, Add an additional assertion case in getDashboardCommandSpec’s test to cover shell-escaping of environment values that include single quotes or shell metacharacters, so shellQuote regressions are detected. Extend the existing command string checks in command-spec.test.ts by setting one env var with a tricky value and verifying the generated dashboardCommand args still contain a safely escaped form, using the same getDashboardCommandSpec and command inspection pattern already in the test.
🤖 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 `@scripts/tui-smoke.mjs`:
- Around line 245-253: The tmux command construction in startDashboard is
serializing the entire inherited env object, which can leak secrets and clobber
tmux-managed variables. Change the env prefixing to include only the specific
dashboard-needed variables instead of spreading all entries, and keep the rest
of the command assembly intact. Apply the same selective env handling in the
other affected command builder referenced by the comment so both paths avoid
shell-prefixing the full parent environment.
- Line 1: The new tui-smoke script is failing lint because Node globals are
being used without being declared and one catch block is rethrowing a different
variable than the original error. Update the script’s top-level Node usage so
ESLint recognizes the runtime globals, and in the catch handling around the code
path near the existing error handler, preserve and rethrow the caught error
object directly instead of substituting another variable. Keep the fix within
scripts/tui-smoke.mjs so the file passes no-undef and preserve-caught-error.
- Around line 513-518: Honor allowFailure in the spawn error path for the child
process helper, not just in the close handler. Update the logic around
child.on("error", finish) in the smoke runner so the function can treat missing
executables the same way it treats nonzero exits when opts.allowFailure is true,
preserving the original failure context. Use the existing helper around child
process execution (the same code that handles stdout, stderr, and
child.on("close")) to route spawn errors through the same allowFailure decision
before rejecting.
In `@src/runtime-restart.ts`:
- Around line 258-271: The reconciliation logic in
reconcileProjectResultsWithVerification is fine, but this runtime behavior
change must be reflected in the built artifacts. After updating the source in
src/runtime-restart.ts, run the project build so dist/ is regenerated and the
shipped runtime restart logic matches the source change.
---
Nitpick comments:
In `@src/dashboard/command-spec.test.ts`:
- Around line 17-29: Add an additional assertion case in
getDashboardCommandSpec’s test to cover shell-escaping of environment values
that include single quotes or shell metacharacters, so shellQuote regressions
are detected. Extend the existing command string checks in command-spec.test.ts
by setting one env var with a tricky value and verifying the generated
dashboardCommand args still contain a safely escaped form, using the same
getDashboardCommandSpec and command inspection pattern already in the test.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71efcc08-a8a1-4ae6-a22e-6a430bfdef99
📒 Files selected for processing (6)
package.jsonscripts/tui-smoke.mjssrc/dashboard/command-spec.test.tssrc/dashboard/command-spec.tssrc/runtime-restart.test.tssrc/runtime-restart.ts
|
Independent sub-agent review found that dashboard build stamps did not include the env-dependent generated dashboard command. Fixed in 5f76191 by hashing the wrapped dashboard command into the stamp and adding a regression test that env changes invalidate the stamp. |
1 participant
Summary
Verification
Summary by CodeRabbit
New Features
smoke:tuicommand to run a build followed by an automated TUI smoke check.Bug Fixes
Tests