Repository navigation
feat(deploy): follow a rolling deploy's restart job, and show canary certification - #1868
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive tracking and UI visualization for application deployment rollouts and canary certifications. It updates the deployment flow to follow rolling restart jobs, parse streamed warnings, and display detailed per-node outcomes or canary rejection decisions, backed by new unit tests and updated error redaction. Feedback on the implementation highlights a potential bug in the polling loop of followRestartJob where transient network or server errors do not reset the missingSince tracker, which could prematurely trigger a 'not-found' state. It is recommended to reset this tracker in the catch block and bound consecutive failures to handle persistent issues gracefully.
| } catch (error) { | ||
| signal?.throwIfAborted(); | ||
| // Anything else — a dropped connection, a 5xx while that node's own workers restart — passes. | ||
| if (isDeterministicRejection(error)) { | ||
| return unfollowable('refused'); | ||
| } | ||
| } |
There was a problem hiding this comment.
During rolling restarts, transient network errors or server-side 5xx errors (e.g., 502 Bad Gateway or 503 Service Unavailable) are common as nodes go up and down. Currently, if a non-deterministic error is caught, the loop continues but missingSince is not reset. This means the time spent during network/server downtime still counts towards the NOT_FOUND_GRACE_MS (20 seconds) limit. If the downtime exceeds 20 seconds, the very next successful poll that returns undefined (not found) will immediately trigger a premature unfollowable('not-found') state, even though we haven't actually had 20 seconds of successful "not found" responses.
Resetting missingSince to undefined in the catch block for non-deterministic errors ensures that the grace period only accumulates during consecutive successful polls where the job is genuinely missing.
Additionally, when implementing polling mechanisms that tolerate transient network or API errors, we should bound the number of consecutive allowed failures (e.g., to 5) so that persistent issues (such as revoked tokens or unreachable servers) are surfaced quickly rather than waiting for the entire timeout period.
| } catch (error) { | |
| signal?.throwIfAborted(); | |
| // Anything else — a dropped connection, a 5xx while that node's own workers restart — passes. | |
| if (isDeterministicRejection(error)) { | |
| return unfollowable('refused'); | |
| } | |
| } | |
| } catch (error) { | |
| signal?.throwIfAborted(); | |
| missingSince = undefined; | |
| consecutiveFailures++; | |
| if (consecutiveFailures > 5) { | |
| throw error; | |
| } | |
| if (isDeterministicRejection(error)) { | |
| return unfollowable('refused'); | |
| } | |
| } |
References
- When implementing polling mechanisms that tolerate transient network or API errors, bound the number of consecutive allowed failures (e.g., to 5) so that persistent issues (such as revoked tokens or unreachable servers) are surfaced quickly rather than waiting for the entire timeout period.
There was a problem hiding this comment.
Fixed the first half in a8dc6c8: a failed poll now resets missingSince (restartJob.ts:176), so the 20s grace only counts an unbroken run of misses. The scenario is real: the job's node restarts its own workers before its peers (harper bin/restart.ts:336-355), so a cluster address reaching another node can read one miss, then errors, and end a running job as not-found. New test: "does not count the time no node answered toward a missing job" (fails without the fix).
Declining the failure cap. A rolling restart is exactly when consecutive failures are expected, and 5 polls at 2s is 10s, shorter than a node restart. The function's contract is to reject only on abort (restartJob.ts:132); callers read an unfollowable rollout rather than catch a throw. The persistent cases it targets are already bounded: a 400/403 stops at once as refused (:172, pollUnlessForbidden.ts:33), and anything else ends timed-out (:180) with polling slowed to 15s after 5 minutes.
🤖 Generated by Claude Code (Anthropic Claude Opus 5.5); posted via @dawsontoth.
…certification A rolling deploy answers once the node that received it is done and hands the other nodes to a restart_service job (restartJobId). Studio never followed it, so a node that rejected the release read as a successful deploy. The deploy mutation now polls get_job until the job ends: a failed job rejects with DeployRolloutError naming each node, and a job Studio cannot follow resolves as unfollowable, which the callers never report as success. Also reads Harper 5.4's canary certification on a deploy's result and its rejection decision on a failure, keeps the deploy's streamed warning events, and links the progress panel to the deployment's record. Refs #1785 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n success A finished restart job with a result in neither shape Harper writes is now unknown rather than complete. The buffered redeploy and import paths report through the same rollout policy as the streamed one, so a node the legacy job could not reach, or a rollout Studio could not follow, is a warning there too, and their toasts link the deployment's record. Following the job aborts promptly when its signal fires mid-poll, and an unknown rollout settles the progress panel as inconclusive, not success. Refs #1785 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-found grace followRestartJob gave up on a job once it had been missing for 20s, but measured that from the first miss even when every poll in between failed. The receiving node restarts its own workers before its peers (harper bin/restart.ts:336-355), so a cluster address that reached another node could read one miss, then errors while the job's node restarted, and end the rollout as not-found though the job was running. A miss now only counts across an unbroken run of misses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… how each node installed The list filters by component and status, pages by the server's total, and shows each record's id, source (package, payload hash, or the release an activation made live) and user. A staged record (Harper 5.3+) is terminal and reads as waiting to be activated, so the list no longer polls it every 2s. The detail view shows the origin's install source, the activation's source release, and each peer's install comparison (5.3.1+), and drops the Restorable and Rollback-of fields no Harper version sets. The editor's Application menu links to the component's deployments. Refs #1786 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ilter where staging is absent The list no longer keeps the previous page's rows on screen, clickable, while the next page loads. A saved ?status=staged link opened against an instance older than 5.3 no longer asks for a status it cannot have. The component filter applies on Enter only, so leaving the field to click Next does not reset the page under the click, and it follows an outside change without remounting the field. Refs #1786 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the URL A saved ?status=staged link opened against Harper older than 5.3 left the URL claiming a filter while the list showed every status. Once the version is known, the page replaces the URL without it. Refs #1786 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Redeploy and Import offer Stage only where the instance reads 5.3.0 or later: it sends activate: false and no restart, and reports the staged deployment id without a health check. A staged record on the Deployments page has an Activate action, which sends deploy_component with its deployment_id and a rolling restart through the same mutation, so it is followed like any other rolling deploy; the progress panel ends a stage at Staged. All three request shapes come from one deployRequestBody, and the editor's Application menu links to a component's staged releases. Refs #1787 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sent twice Activate awaits the mutation instead of passing it callbacks, which React Query drops once the detail unmounts, so the report and the refresh still happen for an activation someone closed the dialog on. The mutation is keyed by entity and deployment, so a detail reopened mid-rollout shows it pending and offers no second activation. A stream that ends early says the outcome is unknown instead of reporting a failure. Refs #1787 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The activation reports every outcome itself, so it opts out of the global mutation toast, which would have called an early-ended stream a failure alongside the local 'may not be activated'. It logs the error with console.error as that handler did, so Datadog still sees it. Refs #1787 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…after it may have started When the SSE deploy got no event stream, onDeployComponentSubmit fell back to the buffered deploy_component on any SSEUnsupportedError: a gateway 502/503/504, a 2xx that was not a stream, a dropped connection, or the 120s idle watchdog firing before headers. Harper starts an SSE operation as soon as it accepts it and answers with a 200 stream, so each of those can follow a deploy that is already running, and the fallback deployed the same package a second time (a second install and rolling restart, or two deploys of one project racing). SSEUnsupportedError now carries the response status. The deploy falls back only on a 4xx, which Harper answers for a request it refused before running it (authentication, permissions, validation). Anything else becomes SSEInconclusiveError, which the import, redeploy and activate callers already answer by checking instead of resending. Closes #1889 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
deployComponentStream's contract still told every caller to retry on SSEUnsupportedError, which is the resend this branch removes. Also trims two comments that restated the design note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a8dc6c8 to
034dd9f
Compare
…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>
Ports the Jobs tab and the CSV import job toast onto stage, which now has #1868's structured job messages and #1884's grant bands. - waitForJob keeps both behaviors: every poll still reaches onUpdate, the ERROR one included, and a structured job message is read as text (errorText) for the rejection and for the import's success message. A test pins the two together. - #1884 removed isGrantGateInert. canListJobs now takes the instance version and honours a listed search_jobs_by_start_date only where gateVerdict(name, resolveGrantBand(version)) === 'grants', so from 5.3.0-beta.3 on a 5.3 allowlist role can list jobs and a 5.2 one cannot. JobsTab reads the version (already loaded by the instance route) only for a role that lists the grant. If a refetch of it fails, the request is tried instead of refused outright, and a 403 shows the same explanation. - The uploaded-CSV path's progress wiring gets a test; only the URL load had one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
⊙ Problem
Stacked on #1824 (
claude/1602-deployments-poll-policy), which stacks on #1821 → #1817 → #1812. Review and merge those first; this diff is only the two commits above #1824. The PRs for #1786 and #1787 stack on this one.Studio deploys with
restart: 'rolling'. The node that receives the deploy answers once its own part is done and hands the other nodes to arestart_servicejob, named in the response'srestartJobId. Studio never followed that job, so a node that rejected or failed the release read as a successful deploy: the post-deploy health check reads the one node Studio reaches, and the deployment record is written only by the receiving node, so it sayssuccesswhile the job runs and after it fails.From Harper 5.4 (HarperFast/harper#2981, merged in fa9b6f857, not yet tagged) a restarting deploy is also certified in a canary worker, and Studio read none of it: not the
certificationoutcome, not a rejection's decision, and not thewarningevents a deploy streams when a peer installed something different (5.3.1).💡 Solution
The deploy mutation now follows the restart job before it settles. When the result carries
restartJobId, it pollsget_jobuntil the job ends: a failed job rejects with a newDeployRolloutErrornaming each node, and a job Studio could not follow resolves asunfollowable, which no caller reports as success.What I verified in Harper's source:
restartJobIdexists since 4.6 (release_4.6.9:components/operations.js:449), so following applies to every version that returns it. A response without one behaves exactly as before: no poll, no timer, no event.hdb_jobis inNON_REPLICATING_SYSTEM_TABLES(harperresources/databases.ts:337), so only the receiving node can answerget_job. Central manager's cluster proxy sends every operation to the first running instance (central-managersrc/resources/cluster/Cluster.js:376-379), so over the proxy the deploy and its polls reach the same node. A direct connection to a cluster's address can reach another node, which answers[]; a row missing for 20 seconds counts asnot-found.server/jobs/jobProcess.ts:72-87), so every node's outcome arrives at once. "Show each node as it lands" is not possible by polling;harper deploy restart=rollingexits 0 before the other nodes have taken the release harper#3058 weighs streaming it.result.activated: [{node, certification, message}], or fails withmessage: {error, activated}(harperbin/restart.ts:232,:235,:258,:260). The plain restart job (4.6–5.3, and 5.4 when no canary was armed) recordsresult.replicated, where a node the job could not reach is listed with its error and the job still completes (:363,:385,:397), so that node reads "restart not confirmed". A finished job in neither shape isunrecognized, never a success.certificationon success iscertified | uncertified | unavailable | not-requested(harpercomponents/operations.js:817,:1073); a rejection fails the deploy with the decision object on both the streamederrorevent and the buffered body (:1094,:1101-1109, built bycertificationFailureat:1222-1247). The copy for an unchecked release comes from Harper's own reference ("Certifying a release in a canary worker").warningevents ship in 5.3.1 (v5.3.1:components/operations.js:945).Gating is by what the server sends, not by a version read: certification renders only when the response carries it (only 5.4 does), and following runs only when
restartJobIdis present. Aregistration_infogate on the deploy path would hide both from roles with an operations allowlist, which Harper denies that operation (AGENTS.md, "Browse — removing an attribute").⚖️ Alternatives
successeven when another node rejects the release harper#3060), or have Harper stream the job (harper deploy restart=rollingexits 0 before the other nodes have taken the release harper#3058). Both are open, and every 4.6–5.4 cluster in the field answers only throughget_job.get_jobto the receiving node explicitly. The deploy response doesn't name that node, and Studio has no contract mapping a Harper node name to an instance. Detect-and-degrade covers the direct case without one.🔧 Changes
restartJob.ts(new):readRestartJobnormalizes both job shapes;followRestartJobpolls with back-off (2s for a minute, then 5s, then 15s, 30-minute ceiling), stops on a 400/403, restarts its not-found grace after a failed poll (the receiving node restarts its own workers before its peers, harperbin/restart.ts:336-355), and passes and rechecks its abort signal;DeployRolloutError.deployComponent.ts:deployAndFollowComponentis the mutation function.onDeployComponentSubmitis unchanged, since fix(applications): explain a git deploy with no root package.json #1841 wraps it.deployOutcome.ts(new): the certification types anddeployFailureDetails, which reads the record id and decision from a streamed, buffered or rollout failure.deployComponentStream.ts: the new result fields,warningevents kept on the result, and one sharedtoDeployStreamEvent, whichgetDeployment.tsnow uses too.streamOperation.tspasses the decision toSSEOperationError.certificationinerrors.ts.useDeploymentStream.ts: warnings and the rollout as events, and the certification, decision and record id from what the deploy answered or failed with.DeployProgress.tsxwithdescribeDeployOutcome.ts(new): the certification line, a rejection's decision (copy) in place of the raw text, warnings, each other node's outcome, an unknown rollout, a link to the deployment's record, and no "verifying" badge over a rollout that already said so.reportDeployHealth.ts:deployNotes, andreportDeployResultfor a caller that does not health-check, so both callers on both transports share one policy.RedeployApplicationModal.tsx: an unknown rollout settles the panel inconclusive and the modal stays open when there is something to read; a rollout failure says it did not reach every node; the buffered path reports through the shared policy with a record link.useImportApplication.ts: the loading toast says it is rolling out, a followed rollout is health-checked on either transport, and a rollout failure is named as one.useDeploymentRecordAction.ts(new): the toast action that opens a deployment's record.redactRelayedMessage.ts: Datadog withholdsDeployRolloutError's message and stack span, which quote each node's error.getJob.ts:messageisunknownand gainsresult, because from 5.3.0 a job can record a report object as its message (harperserver/jobs/jobProcess.ts:86);getJobtakes a signal.src/features/instance/applications/DESIGN.md(new), indexed fromDESIGN.md: when a deploy is done, and why an unknown rollout is never a success.Also fixed: a CSV import whose load job Harper refused with its permission report showed
[object Object]; it now shows the report's text, andimportData.tsreads the message the same way. The type change above forced it.✅ Verification
restartJob.test.ts(both shapes, unconfirmed peers, malformed rows) and its follower tests (back-off, not-found grace and recovery, a miss run broken by failed polls, 400/403, timeout, abort before and during a poll);deployComponent.test.ts(no job means no poll and no timer; success waits for the job; a failed job rejects without deploying again; unknown resolves as unknown; streamed deploys follow the same way);deployOutcome.test.ts(the same decision off the streamed and buffered transports; warnings collected);DeployProgress.test.tsx;useDeploymentStream.test.ts;reportDeployHealth.test.ts;getJob.test.ts;redactRelayedMessage.test.ts.npx vitest run(403 files, 3,786 tests at a8dc6c8, exit 0; the pre-commit hook ran it again on each commit),npx tsc -b(exit 0),npx oxlint --format stylish .(exit 0),npx dprint check(exit 0).DeployToastOptionsexport, un-exported in the Stage a release now and activate it later #1787 PR) and a comment nit. The Cursor legs could not run (the 1Password SSH agent refused theirgit fetch), and the Harper domain leg failed authentication.mutate()'s ownonSuccess/onErrorwhile the caller is mounted. Leaving the applications page mid-rollout therefore leaves the "Redeploying…" / "Rolling out…" loading toast up and drops the outcome report (RedeployApplicationModal.tsx:155,useImportApplication.ts:86-112). The suggested fix is reporting frommutateAsync(...).then/catchor from callbacks set onuseMutation.skipGlobalErrorToast(overlaps Show a failed app import or redeploy once, and keep a failed import beside its form #1871).filter(isRecord)drops non-object entries of a recognizedactivated/replicatedarray, so the job could read as complete. Harper does not write that shape today.DeployToastOptionsexport, narrative comments, and a pre-existing modal-reopen race.gemini-code-assist feedback. Fixed: a failed poll did not reset
missingSince, so a node restart could end a running job as not-found (a8dc6c8, with a regression test). Declined: capping consecutive failures at 5 and throwing. It would break the reject-only-on-abort contract, and 10s is shorter than a node restart. A 400/403 already stops asrefused, and anything else endstimed-out.Closes #1785
🤖 Generated with Claude Code
Related PRs: #1824 overlaps (base of this stack), #1841 overlaps (wraps onDeployComponentSubmit; same deploy error path), #1806 overlaps (adds another relayed error type to the same Datadog list), #1871 overlaps (the same double-toast on a failed redeploy or import), #1897 overlaps (same deploy-outcome toasts in RedeployApplicationModal and useImportApplication), #1874 overlaps (stacked on this branch), #1876 overlaps (stacked; extends the same outcome modules), #1898 overlaps (stacked; changes when a deploy is resent)
Complexity: complicated
Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt); declined=cursor-kimi,cursor-muse; rounds=3; full=1 @ a8dc6c8
Review-Attention: study ~9m (decisions: do-less-alternative, mutation-spans-rollout, any-400-is-refused, unknown-resolves, modal-stays-open-on-warning) @ a8dc6c8