chore: codify aimux launch contracts - #277
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe CLI launch helper is split into contract-specific exports for daemon, dashboard, and current-identity use cases. Daemon startup, dashboard command construction, runtime coherence reporting, and inventory checks now use those contracts, with tests updated to match. ChangesLaunch Command Contract Split
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Daemon as src/daemon.ts
participant Dashboard as src/dashboard/command-spec.ts
participant Runtime as src/runtime-coherence.ts
participant Launcher as src/cli-launcher.ts
Daemon->>Launcher: getAimuxDaemonLaunchCommand()
Launcher-->>Daemon: command/args/source
Dashboard->>Launcher: getAimuxDashboardLaunchCommand({ env, currentArgvEntry })
Launcher-->>Dashboard: command/args/source
Runtime->>Launcher: getAimuxCurrentCliIdentity()
Launcher-->>Runtime: cliLaunch identity
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli-launcher.test.ts (1)
45-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the full daemon contract args after the entry path.
Line 47 would still pass if
"daemon"were accidentally dropped. Tighten this to protect the named launch contract.Proposed test tightening
expect(launch.command).toBe(process.execPath); expect(launch.args[0]).toMatch(/main\.(js|ts)$/); - expect(launch.args.at(-1)).toBe("run"); + expect(launch.args.slice(1)).toEqual(["daemon", "run"]); expect(launch.source).toBe("current-entry");🤖 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/cli-launcher.test.ts` around lines 45 - 47, The launch contract assertion in cli-launcher.test.ts is too loose and can miss if the daemon command is dropped. Tighten the checks around launch.args in the test by asserting the full ordered daemon contract immediately after the entry path, using the existing launch and args expectations so the test verifies both the presence and placement of "daemon" before the trailing "run" argument.
🤖 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.
Nitpick comments:
In `@src/cli-launcher.test.ts`:
- Around line 45-47: The launch contract assertion in cli-launcher.test.ts is
too loose and can miss if the daemon command is dropped. Tighten the checks
around launch.args in the test by asserting the full ordered daemon contract
immediately after the entry path, using the existing launch and args
expectations so the test verifies both the presence and placement of "daemon"
before the trailing "run" argument.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff20506c-b773-44b1-86a2-7846780186ac
📒 Files selected for processing (7)
src/cli-launcher.test.tssrc/cli-launcher.tssrc/daemon.tssrc/dashboard/command-spec.tssrc/one-shot-node-inventory.test.tssrc/runtime-coherence.test.tssrc/runtime-coherence.ts
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes