Correct the deploy_component response, and document the restart a deploy does not do - #673
Conversation
Two things salvaged from #599, whose two-phase/revert content documents harper#1849 and harper#2301 — both closed without merging. The deploy_component response example still shows "Component deployed successfully". That string is not in core. The example requests a rolling restart, so the real response also carries restartJobId, and the message names the component. Deploying without a restart can flag a restart as needed, which nothing in reference/ said. A component that had no directory before the deploy has never been loaded, so its routes cannot serve until Harper restarts (v5.2.0); a redeploy whose package metadata changed invalidates loaded code that no plugin glob watches (v5.2.1). get_status reports it, per node. The route-miss 404 that names the component is super_user-only, because the difference between it and the generic 404 reports which component directories exist on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the operations API documentation to detail the behavior of deploying without a restart, including the introduction of the restartJobId field and how the restartRequired flag is evaluated. The review feedback suggests improving readability and scannability by replacing a mid-sentence <VersionBadge> component with plain text and splitting a sentence to highlight a critical security warning regarding information disclosure.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-673 This preview will update automatically when you push new commits. |
…d alone A mid-sentence VersionBadge reads as an interruption; the plain (v5.2.1) is what this page already uses for a version noted inside running text. The reason the fuller 404 is super_user-only is a security claim, not an aside on the sentence that precedes it.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-673 This preview will update automatically when you push new commits. |
Both from @kriszyp's review. The 5.1 release note carried the same invented message this PR removed from the reference. `Component deployed successfully` was never emitted at v5.1.0 either — `git log -S` over components/operations.js finds the string in no commit on any branch — so this is not a historical record of an older response, and the note now shows what v5.1.0 actually returned. get_status was documented only as the status-value store, which left the restartRequired flag this PR points readers at undiscoverable. The no-id form returns an instance summary instead of a status value: systemStatus, componentStatus, restartRequired, and middlewareChains when the request asks for it (v5.2.0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-673 This preview will update automatically when you push new commits. |
Every other table on this page notes a version as plain (v5.x.y) in the cell; the middlewareChains row I added reached for the badge instead, so one table rendered both ways.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-673 This preview will update automatically when you push new commits. |
🧹 Preview CleanupThe preview deployment for this PR has been removed. |
3 participants
Two corrections to
deploy_component, salvaged from #599 before closing it. Both are against shipped behavior inharpermain — nothing here waits on an upstream merge.The response example is wrong
reference/operations-api/operations.mdshows:{ "deployment_id": "a3f8c2d1...", "message": "Component deployed successfully" }That string does not exist in core.
git grep "Component deployed successfully"onharpermain returns nothing. The example's request asks for"restart": "rolling", which is the one path that hands the restart to a separate job, so the real response also carries arestartJobId, and the message names the component:{ "deployment_id": "a3f8c2d1...", "restartJobId": "b7d41e09...", "message": "Successfully deployed: my-app, restarting Harper" }The three messages are set in
components/operations.js:Successfully deployed: <name>, restarting Harperfor bothrestart: true(inline, no job id) and"rolling"(with one), andSuccessfully deployed: <name>when the deploy does not restart. The new note says which of the three you get.Nothing documented
restartRequiredA deploy that does not restart can flag a restart as needed, and
reference/said nothing about it — so an operator who deploys a new component with"restart": falsegets a 404 on its routes with no documented explanation.requestRestartAfterDeploy(components/requestRestart.ts) sets the flag when:097527c22, first tagged v5.2.0.84201118e, first tagged v5.2.1.An ordinary redeploy sets nothing. The flag is evaluated per node, since whether the component was already active can differ from node to node.
The route-miss 404 is documented with the gate that makes it correct:
server/REST.tsreturns the message naming the component only to an authenticatedsuper_user, because the difference between that response and the generic 404 is otherwise an oracle for which component directories exist on disk.getComponentsalready treats that assuper_user-only information.A third condition — an isolation flip, from harper#2524 — is deliberately left out. That PR is unreleased and dedicated-worker isolation is not documented here yet, so it belongs with whatever documents the feature.
Why not in #599
#599 had a draft of the restart-required note, but it claimed the 404 unconditionally and said a redeploy of a live component never sets the flag, which the v5.2.1 package-metadata condition contradicts. Both were re-derived from core rather than carried over. Everything else in #599 documents harper#1849 and harper#2301, closed without merging; the staged-deploy half came back as harper#2605 and is already on
mainin a more accurate form.Verification
npm run buildandnpm run format:checkare clean.🤖 Generated with Claude Code