Repository navigation
feat(deploy): follow a rolling deploy's restart job, and show canary certification - #1868
dawsontoth wants to merge 2 commits into
Conversation
…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>
![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 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.
1 participant
⊙ 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, 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, 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,777 tests, 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.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)
Complexity: complicated
Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt),domain(auth); declined=cursor-kimi,cursor-muse; rounds=2; full=1 @ 3b75aa0
Review-Attention: study ~12m (raised: degraded review) @ 3b75aa0