Skip to content

fix(deployments): rejected deployment polls back off, and the page needs Harper 5.1 - #1824

Merged
dawsontoth merged 13 commits into
claude/1603-offscreen-status-pollsfrom
claude/1602-deployments-poll-policy
Oct 12, 2026
Merged

dawsontoth merged 13 commits into
claude/1603-offscreen-status-pollsfrom
claude/1602-deployments-poll-policy

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #1821 (claude/1603-offscreen-status-polls), on #1817, on #1812; retarget to stage once those merge.

⊙ Problem

#1602 reported direct-to-instance (:9925) 400s, one page re-issuing a rejected operation every ~60s for half an hour. That shape is gone: over the ~15 days RUM still retains, there were 34 such 400s out of 54,062 direct-instance requests, in short bursts (largest: 12 in one session over 6 minutes, at irregular 15–30s gaps with simultaneous triples — user actions, not a timer), and no repeating cadence. The generic fixes since (#1601 no retries on a 400, #1772 sustained-400 back-off) cover every poller that uses the shared policy. What remained were the instance-operation pollers that bypassed it: the deployment history list (2s/10s), the deployment detail (2s until terminal) and the log auto-refresh (5s). For those a 403 never stopped the timer and a 400 or 5xx never slowed it — and the detail poll never ended on a failure at all, because "until terminal" needs a status a failed fetch doesn't have: a deployment id the instance rejects (No deployment found with id …, a 400 — Harper components/deploymentOperations.ts:100) was asked for every 2s for as long as the dialog stayed open. Separately, the deployments page is hidden from the nav below Harper 5.1, but a bookmark or back-navigation still mounted it and polled list_deployments against an instance that predates it (the page even anticipated this with "This instance may not support deployment history").

❓ Your call: the reported ~60s pattern isn't reproducible from current RUM, so this closes the remaining code paths rather than a measured flood. Close #1602 with it, or keep the issue open to watch for the original fingerprint?

💡 Solution

  • The three pollers go through pollUnlessForbidden. The two whose interval depends on the data compute the base interval per tick and hand it over — (query) => pollUnlessForbidden(base)(query) — which works because the back-off state is keyed by the query, not the wrapper. So: 403 stops; a sustained 400 or 5xx backs off to at most one request per 5 minutes; a success restores the cadence.
  • The deployments page reads the instance's Harper version first. Below 5.1 it says "Deployment history needs Harper 5.1 or later on this instance" and never sends list_deployments; while the version is still loading it shows the table's loading state rather than either outcome, and if the version cannot be read at all it shows the existing "Could not load deployments" line instead of spinning.
  • DESIGN.md records the convention: a poll on a query that can reject goes through the shared policy.

⚖️ Alternatives

  • Give pollUnlessForbidden an overload that accepts an interval function — cleaner call sites, but it changes the shared helper's signature for two callers when composing per tick already works with no API change.
  • Route-level gates for Certificates (4.6) and Secrets (5.2) too — neither page polls, so a bookmark there costs one rejected request, not a loop; left alone.
  • Rely on the back-off alone for a pre-5.1 instance — still a toast and a request every few minutes, and an error message instead of the reason.

🔧 Changes

✅ Verification

End-to-end route: not observable end-to-end here; unit tests drive the real query options and the real page component.

  • deploymentPolling.test.ts — detail: 2s while active, stops at terminal, stops on 403, backs a sustained 400 off to 5 min; list: 2s/10s by activity, stops on 403, backs off a sustained 400.
  • getReadLog.test.ts — 5s only with auto-refresh; stops on 403.
  • deployments/index.test.tsx — lists on 5.1.0; never enables list_deployments on 5.0.13 and shows the reason; while the version loads, shows neither the reason nor "No deployments yet."; when the version read fails, says it could not load.
  • Mutation-checked: reverting the four source files fails 7 of the new tests.
  • Mutation-checked the follow-up: dropping the version-read failure branch fails its test.
  • npx vitest run → 399 files / 3,713 tests passed (exit 0, pre-commit hook). npx tsc -b, npx oxlint --format stylish ., npx dprint check all exit 0.

Pre-push review (codex + gemini), one round. Fixed in the second commit: the page spun forever if the version read failed with nothing cached (gemini); the log view's off path allocated a wrapper that always answered false (codex). Declined: explicit .ts extensions on local test imports (studio imports are extensionless throughout, e.g. every existing from './…' in src/features/cluster/); the comment nits (each states a constraint — why a route needs its own gate, why a failed detail fetch needs the policy). The cursor leg failed on the 1Password SSH agent and the domain (Harper adjudication) leg on an expired OAuth session. The follow-up commit was not re-reviewed.

Closes #1602

🤖 Generated with Claude Code

Related PRs: #1821 overlaps (stacked base), #1817 overlaps (base of #1821), #1812 overlaps (base of #1817)
Complexity: easy

Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-composer(no-receipt),domain(auth); declined=cursor-grok,cursor-kimi,cursor-muse; rounds=1; full=1 @ 77ebdc2

Review-Attention: read ~3m (raised: degraded review) @ 77ebdc2

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a polling utility pollUnlessForbidden to handle query refetching gracefully, stopping on 403 errors and backing off on 400 or 5xx errors. It integrates this utility into the deployment and read log query options. Additionally, it adds version gating to the deployments page, ensuring that deployment history is only polled and displayed on Harper 5.1 or later, and includes comprehensive tests for these changes. There are no review comments, so I have no feedback to provide.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 69.99% 11088 / 15842
🔵 Statements 70.15% 11830 / 16863
🔵 Functions 64.13% 2908 / 4534
🔵 Branches 65.35% 8586 / 13138
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/instance/applications/components/DeployProgress/DeployProgress.tsx 84.21% 76.54% 83.33% 84.21% 57, 106-138, 284
src/features/instance/applications/components/DeployProgress/describeDeployOutcome.ts 75% 65.21% 100% 75% 32, 63-67, 79, 83
src/features/instance/applications/components/DeployProgress/useDeploymentStream.ts 84.21% 72.22% 100% 82.85% 95-101, 105-110, 122
src/features/instance/applications/components/NewApplication/ImportInstructions.tsx 60% 70% 42.85% 60% 37, 95-132, 166-176
src/features/instance/applications/components/NewApplication/schema.ts 100% 100% 100% 100%
src/features/instance/applications/hooks/useDeploymentRecordAction.ts 16.66% 0% 25% 16.66% 14-19
src/features/instance/applications/hooks/useSupportsDeploymentSSE.ts 30.76% 66.66% 40% 30.76% 33-63
src/features/instance/applications/modals/reportDeployHealth.ts 84% 71.42% 100% 83.33% 62-66, 71-90
src/features/instance/config/routes.ts 67.39% 100% 51.61% 67.39% 19, 26, 34, 40, 47, 53, 60, 67, 73, 81, 88, 95, 101, 108, 114
src/features/instance/config/deployments/deploymentsSearch.ts 100% 100% 100% 100%
src/features/instance/config/deployments/index.tsx 85.45% 78.94% 70.58% 88.23% 89-90, 94, 95, 101, 106-109, 147, 186
src/features/instance/config/deployments/components/ActivateStagedRelease.tsx 100% 78.57% 100% 100%
src/features/instance/config/deployments/components/DeploymentDetail.tsx 0% 0% 0% 0% 22-230
src/features/instance/config/deployments/components/deploymentStatusBadge.tsx 100% 50% 100% 100%
src/features/instance/config/deployments/constants/tableDefinition.tsx 100% 83.33% 100% 100%
src/features/instance/config/deployments/functions/deploymentSource.ts 100% 100% 100% 100%
src/features/instance/config/deployments/functions/describePeerInstall.ts 100% 85.71% 100% 100%
src/integrations/api/instance/applications/deployComponent.ts 95.83% 100% 83.33% 95.83% 117-121
src/integrations/api/instance/applications/deployComponentStream.ts 84.21% 57.89% 100% 84.21% 66-70
src/integrations/api/instance/applications/deployOutcome.ts 100% 92.3% 100% 100%
src/integrations/api/instance/applications/restartJob.ts 100% 96.61% 100% 100%
src/integrations/api/instance/database/getJob.ts 100% 93.75% 100% 100%
src/integrations/api/instance/database/importData.ts 0% 0% 0% 0% 28-105
src/integrations/api/instance/deployments/getDeployment.ts 18.18% 40% 28.57% 18.18% 18-22, 28, 62-78
src/integrations/api/instance/deployments/listDeployments.ts 72.72% 100% 60% 70% 36-45, 52
src/integrations/api/instance/deployments/types.ts 100% 100% 100% 100%
src/integrations/api/instance/status/getReadLog.ts 76.92% 83.33% 60% 76.92% 67-68, 85
src/integrations/api/sse/errors.ts 100% 100% 100% 100%
src/integrations/api/sse/streamOperation.ts 79.36% 75% 66.66% 78.68% 129, 164-183
src/integrations/datadog/redactRelayedMessage.ts 93.75% 92.85% 100% 93.75% 69
Generated in workflow #2219 for commit 377d6b0 by the Vitest Coverage Report Action

@dawsontoth
dawsontoth marked this pull request as ready for review October 12, 2026 16:38
@dawsontoth
dawsontoth requested a review from a team as a code owner October 12, 2026 16:38
dawsontoth and others added 13 commits October 12, 2026 12:42
…eds Harper 5.1

The deployment history list (2s/10s), the deployment detail (2s until
terminal) and the log auto-refresh (5s) were the instance-operation pollers
left outside the shared poll policy, so a 403 never stopped them and a 400 or
5xx never slowed them. The detail poll was the worst: a failed fetch has no
status to end on, so a deployment id the instance rejects ("No deployment found
with id …", a 400) was asked for every 2s for as long as the dialog stayed
open. All three now go through pollUnlessForbidden; the two whose interval
depends on the data compute it per tick and hand it over.

The deployments page is hidden from the nav below Harper 5.1, but a bookmark or
back-navigation still mounted it and polled list_deployments against an
instance that doesn't have it. It now reads the instance's version first and
says the feature needs Harper 5.1 instead of polling.

DESIGN.md records the polling convention.

Refs #1602

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on can't be read

Review follow-up: if the registration_info read failed with nothing cached,
the deployments page waited on the version forever behind a spinner. It now
shows the existing "Could not load deployments" line instead. The log view's
auto-refresh off path stays a plain `false` rather than a wrapper that always
answers false.

Refs #1602

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…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>
@dawsontoth
dawsontoth force-pushed the claude/1602-deployments-poll-policy branch from 030efff to beda7f9 Compare October 12, 2026 16:45
@dawsontoth dawsontoth closed this Oct 12, 2026
@dawsontoth
dawsontoth merged commit 377d6b0 into stage Oct 12, 2026
6 checks passed
@dawsontoth
dawsontoth deleted the claude/1602-deployments-poll-policy branch October 12, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RUM] New 400s on the direct-to-instance operations API (:9925) — a rejected poll re-fires every ~60s for 30 minutes

1 participant