Repository navigation
fix(applications): explain a git deploy with no root package.json - #1841
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to intercept and explain deployment failures caused by a missing root package.json in a git repository, replacing raw Node ENOENT errors containing temporary clone paths with a user-friendly guidance message. This is integrated into the deployment detail view, event logs, and deployment submission handlers, accompanied by comprehensive tests. The review feedback suggests updating explainDeployFailureText to gracefully handle undefined inputs to prevent potential TypeScript compilation or coercion issues, and using object destructuring to omit keys rather than setting them to undefined when rewriting the Axios error response body.
| export function explainDeployFailureText(text: string): string { | ||
| return MISSING_ROOT_PACKAGE_JSON.test(text) ? MISSING_ROOT_PACKAGE_JSON_MESSAGE : text; | ||
| } |
There was a problem hiding this comment.
In DeploymentDetail.tsx, explainDeployFailureText is called with the result of JSON.stringify(entry.data), which can evaluate to undefined if entry.data is undefined. To prevent potential TypeScript compilation errors under strict null checks and avoid fragile implicit coercion of undefined to "undefined" in RegExp.prototype.test, update the function signature and implementation to handle undefined gracefully.
export function explainDeployFailureText(text: string): string;
export function explainDeployFailureText(text: undefined): undefined;
export function explainDeployFailureText(text: string | undefined): string | undefined {
if (text === undefined) {
return undefined;
}
return MISSING_ROOT_PACKAGE_JSON.test(text) ? MISSING_ROOT_PACKAGE_JSON_MESSAGE : text;
}There was a problem hiding this comment.
Declining. TypeScript types JSON.stringify as returning string (lib.es5.d.ts:1169), so strict null checks raise no error here and tsc -b is clean. At runtime an absent entry.data stringifies to undefined; RegExp.test coerces that to "undefined", which cannot match the ENOENT … harper-git-clone-…/package.json pattern, so the value comes back unchanged and React renders nothing. That is what stage already does (DeploymentDetail.tsx:191 renders JSON.stringify(entry.data) directly). An undefined overload would only restate that pass-through.
🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.
…1833-deploy-failure-inline Brings in #1841's port onto stage's deploy stack (DeployRequest bodies, staged releases, the rollout follow, DeployRolloutError, record links) and ports this PR's one-failure-display rule onto it. - useDeployComponentMutation runs deployAndFollowComponent, keeps its mutationKey, and always sets skipGlobalErrorToast with a mutation-level console.error. ActivateStagedRelease drops its own console.error and meta, which would now report every failed activation twice. - Import and redeploy keep mutateAsync and the stale-continuation guards through stage's new outcomes: a stage, an unfollowable rollout, a DeployRolloutError and the record action. Since a followed rollout keeps the mutation pending for up to 30 minutes, this is what lets an outcome that lands after the form or modal is gone still replace the loading toast and get reported (#1868's open question). - On screen, a rollout failure shows in the progress panel only; off screen it toasts "did not reach every node" with the record action. The buffered redeploy now always toasts its failure, since nothing global does. - An inline import failure carries its title (import or stage) and the deployment record it named, and DeployFailure links that record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deploying a git repository without a root package.json fails in Harper with Node's raw "ENOENT: no such file or directory, open '/tmp/harper-git-clone-XXXXXX/package.json'", because the reclone path reads the manifest unguarded (harper components/Application.ts:555). Studio showed that verbatim everywhere a deploy failure surfaces: the import and redeploy toasts, the global mutation toast, the redeploy progress panel, and Config > Deployments' error panel and activity log. It names a temp directory on the node and not the problem, and RUM shows users retrying it up to nine times. Recognize that failure and replace it with guidance. The deploy mutation swaps the error at its source, so every consumer of the rejection (the global toast included) gets the guidance; each error keeps its type and envelope (an SSEOperationError for RUM redaction, an AxiosError's status and config for the buffered path). Config > Deployments maps the stored record's error and its activity log the same way. The root fix is tracked in HarperFast/harper#3142. Closes #1699 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oy body The explained Axios body set message/title/detail to undefined, so the keys stayed present on the object a consumer receives. Omit them instead, and pin the body shape with toStrictEqual, which the old code fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t beside its form A failed deploy showed the same failure in two toasts: the deploy mutation's global one, and the caller's own. The import form's only failure display was a toast that faded, for failures the user has to act on and retry. The deploy mutation now opts out of the global toast and keeps its RUM report with a mutation-level console.error. The import form keeps the failure inline above the Import button until the next attempt, with the SSH-keys link the redeploy panel already offered. Redeploy shows a streamed failure in its progress panel, toasting only when the panel is gone, and the buffered path, which relied on the global toast, now says why it failed. Both callers await mutateAsync so a failure that settles after the form unmounts still reaches the user, and every failure display reads the server's reason through describeError rather than axios's generic message. Closes #1833 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…is open now Awaiting mutateAsync means a deploy's continuation now runs after its form or modal is gone. An import that finished after the user left the form moved the editor's selection to the new project, and a redeploy that succeeded after its modal was closed closed and reset whatever redeploy modal was open by then. The import still refreshes the tree and reports its result, but leaves the selection alone once the form has unmounted. The redeploy modal records which deploy its progress panel shows; any other deploy reports through toasts and never settles the panel or closes the modal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… teardown Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…omments that narrated it The cross-model round on the port found the new comments narrating framework mechanics and caller behavior, and two claims in DeployFailure's that no longer held (that nothing else reports an SSH failure, and that RUM drops both halves of its regex). The rule they were circling, that every deploy caller shows its own failure and awaits mutateAsync, now lives once in applications/DESIGN.md; the code points there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hat ran it The closing cross-model round found that switching to another instance's Applications page mid-import keeps useImportApplication mounted (same route component), so a mount check let instance A's failure land inline on instance B's form, with a record link built from B's route params. The hook now tracks which instance's form is on screen. A failure is shown inline only on the form of the instance that ran the import, and toasts with that import's own record action otherwise; the editor selection moves only for that instance too. Also drops the duplicated DESIGN.md pointers and the issue-number history from test notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
54008ef to
7233fb0
Compare
📦 This PR now carries #1871 too
Rebased into 7 linear, signed commits on current
stage, with no merge commits:package.jsonBoth were ported onto the deploy code that landed with the polling → deploy stack (rollout-job following,
deployRequestBody, staged releases, resend-only-on-4xx). The ported end state of each PR is exactly what was gated before the rebase.Also resolves #1868's open ❓: import and both redeploy paths now await
mutateAsync. So an outcome that settles after the form or modal is gone, including a followed rollout that is still pending, still replaces the loading toast and gets reported. The deploy mutation setsskipGlobalErrorToastand reports to RUM once, which also removes the double toast on rollout failures.Left out on purpose: during the port,
rememberedClusterDeepLink.test.tsgot a 30s timeout to get past a flaky hook. That file stays as onstage, where #1918 fixed the flake properly.Gate on the final tree:
tsc -b, oxlint,dprint check, vitest 462/462 files.⊙ Problem
Importing a git repository with no
package.jsonat its root fails with Node's rawENOENT: no such file or directory, open '/tmp/harper-git-clone-oXPS7c/package.json', and Studio showed that verbatim everywhere a deploy failure surfaces. That covers the import and redeploy toasts, the global mutation toast, the redeploy progress panel, and Config > Deployments' error panel and Activity log. The text names a temp directory on the Harper node that is deleted moments later, and says nothing about the actual problem. #1699 measured 19 of these across 4 sessions in 30 days, with one user retrying the same import 9 times.The root cause is in Harper.
packGitReferenceWithoutScriptsreads the clone'spackage.jsonwithout checking it exists (harpercomponents/Application.ts:555, still the case at v5.3.1), and the same text reaches the 500 body ({ error },operations.js:1083), the streamederrorevent (:1101, which the recorder also appends toevent_log), and the deployment record. I filed that as HarperFast/harper#3142 (P2) and changed nothing in harper.💡 Solution
A new
explainDeployFailuremodule recognizes the failure (ENOENTplus aharper-git-clone-*directory'spackage.json) and substitutes guidance: "This repository has no package.json in its root folder. Harper deploys a git repository as an npm package, which needs one there, so add a package.json to the root of the repository and deploy again."onDeployComponentSubmitrethrows every failure through it, so each consumer of the rejection gets the guidance: the import toast, the redeploy toast and progress panel, and the globalmutationErrorHandlertoast that the issue's triage didn't name. Each error keeps its type and envelope. A streamedSSEOperationErrorstays one, because RUM redaction keys on that name, and keeps its code, phase and deployment id. A bufferedAxiosErrorkeeps its status, config and body shape:errorcarries the guidance and the other text fields that could carry the raw message are dropped.⚖️ Alternatives
DeployProgressanduseImportApplication). Rejected: it misses the global mutation toast, which shows the same raw text a second time for every deploy failure, and it would repeat the call in four places.package.json.🔧 Changes
explainDeployFailure.ts(new): the matcher, the guidance,explainDeployFailureTextfor plain text, andexplainDeployFailurefor a thrown error.deployComponent.ts:onDeployComponentSubmitwraps the existing body (renameddeployComponent, unchanged) and rethrows throughexplainDeployFailure.DeploymentDetail.tsx: the error panel and the Activity log run their text throughexplainDeployFailureText.✅ Verification
explainDeployFailure.test.tscovers Linux, macOS and Windows clone paths, a wrapped and a multi-line message, an event serialized as JSON, and look-alikes it must leave alone (another file in the clone, apackage.jsonelsewhere, the SSH-access failure). It checks that a streamed error keeps its type and details, that a buffered error keeps its status, config and body shape across{ error },{ error, message }, Problem Details and string bodies, and that other errors pass through untouched.deployComponent.test.tschecks that a streamed and a buffered deploy reject with the guidance. It also drives a realQueryClientwith the app'smutationErrorHandlerand checks that the global toast shows the guidance.DeploymentDetail.test.tsxrenders a failed deployment whose record andevent_logcarry the raw text (mirroring what Harper writes), with Linux and Windows paths, and checks that the clone path appears nowhere.origin/stage'sdeployComponent.tsandDeploymentDetail.tsx, 4 tests fail showing the raw ENOENT /Request failed with status code 500/ the clone path in the DOM.messagefield, title/detail, the clone-directory anchor, the Windows separator, the doubled separator, dotAll, the AxiosError envelope, the body shape, title removal, and bothDeploymentDetailcall sites.tsc -b,oxlintanddprint check, all exit 0.Error, reading Problem Details fields, the doubled Windows separator, dotAll, and trimmed comments. Round 2: codex found nothing and ruled round 1 resolved. Gemini's point that rebuilding the body dropped Harper's other fields was taken. Its per-render regex "major" was rejected: a linear scan over a bounded log. Round 3: no findings from either leg. The Cursor legs could not fetch (1Password SSH agent refused) and the Harper domain adjudicator failed on an expired OAuth session, so no round was adjudicated.message/title/detailinstead of setting them toundefined, and the body-shape test usestoStrictEqual, which the old code fails (0efc440). Declined adding anundefinedoverload toexplainDeployFailureText:JSON.stringifyis typedstring, and an absententry.datapasses through unchanged and renders nothing, as onstage. The fix changes no consumer-visible output (describeErrorandJSON.stringifyread both shapes identically), so no further cross-model round ran.Related issues: HarperFast/harper#3142 (root cause), #1833 (duplicate and fading deploy-failure toasts).
Closes #1699
🤖 Generated with Claude Code
Related PRs: none found
Complexity: medium
Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-composer(no-receipt),domain(auth); declined=cursor-grok,cursor-kimi,cursor-muse; rounds=3; full=1 @ 0efc440
Review-Attention: read ~3m (raised: degraded review) @ 0efc440