Fast-path host status through the daemon#302
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds a ChangesHost status text route
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Shim as installed-aimux-shim.sh
participant Daemon as daemon.ts
participant Paths as ensureProjectPaths
participant Renderer as renderCoreHostStatusLines
Shim->>Daemon: GET /core/host-status-text?project=<root>
Daemon->>Paths: ensureProjectPaths(project)
Daemon->>Daemon: hostStatusPayload()
Daemon->>Renderer: renderCoreHostStatusLines(payload, knownProject)
Renderer-->>Daemon: status lines
Daemon-->>Shim: text or JSON response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/daemon.ts (2)
680-689: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRoute handler wired correctly; confirm build step.
The handler correctly validates the required
projectparam, ensures paths, and delegates rendering to the shared helper — consistent withdaemon.test.tsexpectations. Since this is a daemon runtime behavior change, ensureyarn buildis run to refreshdist/prior to release.As per coding guidelines, "For aimux runtime or CLI behavior changes, run
yarn buildto updatedist/; source-level validation withyarn vitestandyarn typecheckis not enough."🤖 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/daemon.ts` around lines 680 - 689, The host status route in daemon.ts is already wired correctly, so no code change is needed in the GET handler for CORE_API_ROUTES.hostStatusText; instead, make sure to run yarn build so the updated daemon runtime behavior is reflected in dist/ before release. Keep the existing validation flow in the route handler (project check, ensureProjectPaths, hostStatusPayload, renderCoreHostStatusLines) unchanged and ensure the built artifacts are refreshed.Source: Coding guidelines
309-315: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the project-root helper
resolveProjectRootduplicates the samefindMainRepo+ fallback logic used insrc/core-cli.tsandsrc/main.ts; extract a shared helper so daemon and CLI behavior stay aligned.🤖 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/daemon.ts` around lines 309 - 315, The resolveProjectRoot method in Daemon duplicates the same findMainRepo-with-fallback logic used elsewhere, so extract that behavior into a shared helper and update Daemon, core-cli, and main to call it. Keep the helper responsible for trying findMainRepo and returning the original cwd on failure, and reference resolveProjectRoot plus the existing findMainRepo usage sites when wiring the shared implementation.src/core-cli.ts (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCLI delegation looks correct.
Payload shape at lines 59-67 matches
CoreHostStatusTextPayloadexactly, and delegating to the sharedrenderCoreHostStatusLinesremoves the duplicated pid-extraction helper. Since this changesaimux host statusCLI runtime behavior, remember to runyarn buildto refreshdist/— source-levelyarn vitest/yarn typecheckalone won't update the published CLI artifact.As per coding guidelines, "For aimux runtime or CLI behavior changes, run
yarn buildto updatedist/; source-level validation withyarn vitestandyarn typecheckis not enough."Also applies to: 49-49, 54-74
🤖 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/core-cli.ts` at line 7, The CLI delegation in core-cli.ts looks correct, but because this changes aimux host status runtime behavior and affects the published CLI artifact, make sure to run yarn build so dist/ is regenerated; symbolically, this applies around renderCoreHostStatusLines and the CoreHostStatusTextPayload handling, and source-only checks like yarn vitest or yarn typecheck are not enough.Source: Coding guidelines
src/installed-shim.test.ts (1)
280-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a fallback test for
host status --jsonwith stale health.Only the plain
host statusfallback path is tested. The--jsonbranch has separate dispatch logic (line 114 in the shim) and should get equivalent fallback coverage.✅ Suggested additional test
+ it("falls back to the Node launcher for host status --json when daemon health is stale", () => { + const fixture = makeFixture(); + writeFileSync(fixture.healthFile, `${health("old-build", 321)}\n`); + writeFileSync(fixture.textRouteFile, '{"projectRoot":"/repo"}\n'); + writeFileSync(fixture.daemonInfoPath, `${JSON.stringify({ pid: 321, port: 45678 })}\n`); + + const result = fixture.run(["host", "status", "--json"], { NODE_EXIT: "34" }); + + expect(result.status).toBe(34); + expect(readFileSync(fixture.nodeLog, "utf8")).toBe( + `${fixture.aimuxRoot}/dist/launcher-bin.js host status --json\n`, + ); + });🤖 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/installed-shim.test.ts` around lines 280 - 291, Add a fallback test in installed-shim.test that covers host status --json when daemon health is stale, since only the plain host status path is currently verified. Reuse the same stale-health fixture setup in makeFixture and the same launcher fallback assertions, but invoke fixture.run with host status --json so the separate dispatch in the shim is covered. Keep the expectation that the Node launcher is used by checking the nodeLog path and exit status, similar to the existing host status 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.
Nitpick comments:
In `@src/core-cli.ts`:
- Line 7: The CLI delegation in core-cli.ts looks correct, but because this
changes aimux host status runtime behavior and affects the published CLI
artifact, make sure to run yarn build so dist/ is regenerated; symbolically,
this applies around renderCoreHostStatusLines and the CoreHostStatusTextPayload
handling, and source-only checks like yarn vitest or yarn typecheck are not
enough.
In `@src/daemon.ts`:
- Around line 680-689: The host status route in daemon.ts is already wired
correctly, so no code change is needed in the GET handler for
CORE_API_ROUTES.hostStatusText; instead, make sure to run yarn build so the
updated daemon runtime behavior is reflected in dist/ before release. Keep the
existing validation flow in the route handler (project check,
ensureProjectPaths, hostStatusPayload, renderCoreHostStatusLines) unchanged and
ensure the built artifacts are refreshed.
- Around line 309-315: The resolveProjectRoot method in Daemon duplicates the
same findMainRepo-with-fallback logic used elsewhere, so extract that behavior
into a shared helper and update Daemon, core-cli, and main to call it. Keep the
helper responsible for trying findMainRepo and returning the original cwd on
failure, and reference resolveProjectRoot plus the existing findMainRepo usage
sites when wiring the shared implementation.
In `@src/installed-shim.test.ts`:
- Around line 280-291: Add a fallback test in installed-shim.test that covers
host status --json when daemon health is stale, since only the plain host status
path is currently verified. Reuse the same stale-health fixture setup in
makeFixture and the same launcher fallback assertions, but invoke fixture.run
with host status --json so the separate dispatch in the shim is covered. Keep
the expectation that the Node launcher is used by checking the nodeLog path and
exit status, similar to the existing host status test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66a5d1fd-5bef-437e-8456-7fc38d8bfb81
📒 Files selected for processing (9)
docs/command-ownership-inventory.mdscripts/installed-aimux-shim.shsrc/core-cli.tssrc/core-command-contract.tssrc/core-command-ownership.test.tssrc/core-text.tssrc/daemon.test.tssrc/daemon.tssrc/installed-shim.test.ts
Summary
/core/host-status-textroute with text and JSON output parityaimux host status [--json]through the daemon without launching Node on healthy installsVerification
Summary by CodeRabbit
New Features
aimux host statusin both plain text and--jsonoutput.Bug Fixes