Ship np and nodepilot-mcp with both installers - #219
Merged
Conversation
The server artifact contained exactly one executable, NodePilot.Api.exe. Neither operator client was ever packaged: Build-Artifact.ps1 ran a single dotnet publish, the Inno script took only the payload and deploy folders, and the documentation told operators to build the clients themselves. `np` is the documented way to drive an installation from a script and nodepilot-mcp is the only way to point an AI agent at it, so "not shipped" meant "not really available" -- a lab install running 1.2.6 had neither binary anywhere on disk. Both builds now publish the clients into their own directories under tools\, kept separate from the API so no publish decides another's shared assembly versions. The server build publishes framework-dependent, matching the API; the desktop build publishes self-contained, because that package promises zero prerequisites and a framework-dependent apphost cannot borrow the runtime sitting next to the API. Install and update append <install>\tools\np to the machine PATH so operators can type `np` without knowing where setup landed; uninstall removes it before deleting the directory it points at. Update carries the same step because an installation predating this change has no entry to keep. nodepilot-mcp is deliberately left off PATH: an MCP client is configured with an absolute path in .mcp.json and gains nothing from it. The PATH string surgery lives in deploy/MachinePath.ps1 as pure functions rather than three copies. Its tests cover what a manual run never reproduces: re-installing must not grow PATH (which has a real length limit), and "C:\NP\tools\np\" and "c:\np\TOOLS\np" name the same directory while a naive compare disagrees -- a mismatch there leaves a dead entry pointing at a deleted directory. Writing those tests immediately caught a real defect: PowerShell unwraps a single-element array on return, so appending to a one-entry PATH concatenated strings and produced one unusable entry with no separator.
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The server artifact contained exactly one executable —
NodePilot.Api.exe. Neither operator client was ever packaged:Build-Artifact.ps1ran a singledotnet publish(the API).NodePilotServer.iss[Files]took only the payload anddeploy\.README.mdtold operators todotnet publish src/NodePilot.Clithemselves, and the project tree called both "shipped via dotnet publish".npis the documented way to drive an installation from a script, andnodepilot-mcpis the only way to point an AI agent at one. "Not shipped" therefore meant "not really available" — a lab install running 1.2.6 had neither binary anywhere on disk.(To be clear about what this did not break: NodePilot's own AI features are unaffected. The workflow assistant and
/ai-chatrun in-process inNodePilot.Ai; nothing referencesNodePilot.Mcp. The missing piece was the opposite direction — an external agent connecting in.)Change
Packaging. Both builds publish the clients into their own directories under
tools\:tools\np\np.exetools\mcp\nodepilot-mcp.exeSeparate directories per client on purpose: they carry their own copies of shared dependencies, and merging publishes into one folder lets whichever ran last decide the assembly versions the service then loads. Desktop is self-contained because that package promises zero prerequisites — a framework-dependent apphost cannot borrow the runtime sitting next to the API.
Size: server ZIP +~10 MB. The desktop installer grows more (two self-contained publishes); worth flagging against its current 176 MB.
PATH. Install and update append
<install>\tools\npto the machine PATH; uninstall removes it before deleting the directory it points at. Update carries the same step because an installation predating this change has no entry to keep.nodepilot-mcpis deliberately left off PATH — an MCP client is configured with an absolute path in.mcp.jsonand gains nothing from it.Assert-NodePilotExtractedFilesis unaffected: the clients are inside the signed ZIP, so "exactly the signed contents" still holds.Tests
deploy/Test-MachinePath.ps1(new, wired into CI in both PS 5.1 and PS 7 lanes) — 20 assertions over the PATH helpers, covering what a manual run never reproduces: re-installing must not grow PATH (it has a real length limit and install/upgrade cycles repeat), andC:\NP\tools\np\vsc:\np\TOOLS\npname the same directory while a naive compare disagrees — a mismatch there leaves a dead entry pointing at a deleted directory.Writing them immediately caught a real defect: PowerShell unwraps a single-element array on return, so appending to a one-entry PATH concatenated strings and produced
C:\WindowsC:\...\tools\np— one unusable entry with no separator.deploy/Test-DeploymentTemplates.ps1— pins the packaging contract: both builds publish both clients, the desktop clients are self-contained, the Inno script shipstools\*, and install/update/uninstall all go through the one shared helper instead of three copies of the same string surgery.Ran:
Test-DeploymentTemplates.ps1,Test-MachinePath.ps1,Test-SetupAdapter.ps1(213 assertions),Test-ArtifactSecurity.ps1— all pass.Docs
README.md(both client sections + project tree),CLAUDE.md,docs/claude-reference.md,docs/mcp-server.md,src/NodePilot.Cli/CLAUDE.md,deploy/README.md, and the docs site (content/cli.md,content/mcp-server.md) now say the clients ship with the installers, withdotnet publishdemoted to the source-checkout case.