fix(restart): say a restart Harper accepted is not back yet, never that it failed

[dawsontoth]

⊙ Problem

A restart Harper accepted is reported as failed when the target is slow to answer again: restartInstance sends the restart, then watches for the target with user_info, and both steps rejected the same promise. So the Restart button toasts "Failed to restart instance.", the cluster restart and rolling config update count the member as not restarted, and the chat tools return success: false. Each one invites a second restart of a target that already went down once (#1895).

❓ Your call: is this the right size for the fix? Chosen: classify at the producer, keep watching for a second probe schedule, and stop the one-at-a-time loops at a member that is not back. The smaller alternative is a copy-only change ("Failed" → "couldn't confirm") on every caller, which would drop the refused/accepted distinction a user needs (a refused restart is safe to retry; an accepted one is not). Reversible by reverting this PR.

💡 Solution

Harper's answer to the restart request is its only verdict on the restart. restart returns RESTART_RESPONSE on the main thread and schedules the teardown 50ms later "so we give the HTTP thread a chance to return a response" (harper bin/restart.ts:116-188; a non-object result is wrapped as { message } at server/serverHelpers/serverUtilities.ts:163-165). restart_service runs through executeJob and answers { message: 'Restarting http', job_id } once its job thread has been launched (serverUtilities.ts:590-617, server/jobs/jobRunner.ts:92-94); a bad service is a 400 from jobs.ts:130-133. So a 2xx means accepted, and the probe that follows only tells "back" from "not back yet". (Read-only harper checkout at v5.3.1-158-g16e5c663c.)

• restartInstance now rejects with the restart request's own error when that request failed (refused, or unanswered, which fix(errors): say a write's outcome is unknown when it got no verdict, not that it failed #1897's "outcome unknown" handles), and wraps only a failure after the POST resolved in the new RestartUnconfirmedError (cause: the last probe failure; its message keeps the probe's text so shouldKeepEvent filters it as before).
• The watch keeps going past the usual recovery time: one 10s settle, then two schedules of 12 probes, calling onSlowToReturn once between them so callers say "Still restarting" instead of giving up. The probes skip the gateway replay, the restart POST gets an explicit 60s timeout, and RESTART_MARK_TTL_MS is derived from the same constants, so no restart mark lapses mid-watch.
• describeError maps the new error to Not back yet: "Harper accepted the restart, but Studio couldn't confirm it came back. It may still be starting, so check its status before you restart it again." shouldToastOwnFailure keeps the button from adding "Failed to restart" beside it.
• The cluster restart and rolling config update stop at a member that never comes back rather than restart the next one, and report the members they left alone.
• The chat tools keep success: true (their action was carried out) with copy telling the agent not to restart, or create the app, again.

Also fixed: both chat tools now replace their loading toast on any failure; before, "Restarting HTTP service..." / "Creating from template..." kept spinning, since a loading toast never dismisses itself.

⚠️ Look hardest: the producer boundary, where only a failure after the POST resolved becomes RestartUnconfirmedError, and the shared TTL that has to outlast the whole watch.

⚖️ Alternatives

The planning review (codex) returned better-alternative-exists; its variant was adopted: keep the producer-owned typed error, but stop the rolling loops on it, keep watching inside one deadline, and give the chat tools a non-contradictory result.

• Classify in each caller by inspecting which request failed: five call sites re-deriving it from serialized request data, and the global mutation toast could not know the error came from a restart at all.
• Resolve instead of reject ({ data, answeredAgain: false }): fails open, since any caller not updated, and the mutation's global handler, would report an unconfirmed restart as "restarted!".
• Just lengthen the window: moves the edge (compaction on start, bin/restart.ts:135-137, is unbounded) without changing what the edge says.

❓ Your call: the loops now stop at a member that accepted the restart and never came back (cluster restart, rolling config; adopted from the planning review's blocker: carrying on can leave a two-member cluster with no member serving). The cost: re-clicking Restart Cluster restarts every member again, including those already done. The alternative is the old carry-on. Reversible by deleting the two breaks.

❓ Your call: the watch is now about twice as long (≈3.5 → ≈7 min before "Not back yet"), with a "Still restarting" notice at the halfway point, and the Restart button stays disabled throughout. Shorter means more false "not back" stops in the loops; longer means a longer spinner.

❓ Your call: not adopted, against both review rounds: counting a probe answered 401/403 as the target being back. Through central manager's proxy a 403 is CM refusing the caller (notAllowed() in central-manager src/resources/hdbInstance/HDBInstance.js:415-422) before it contacts the instance, so it says nothing about the target (recorded in DESIGN.md). The copy says Studio "couldn't confirm it came back", which stays true for any failed probe. Treating a direct client's 401/403 as back would need a direct-vs-proxied split in the probe.

❓ Your call: an unanswered restart (#1897's outcome-unknown case) still lets the loops carry on to the next member. It may also have taken its member down, but its POST got no verdict, so there is no watch to stop on; left as #1897 decided.

🔧 Changes

• src/integrations/api/instance/status/restartUnconfirmed.ts (new): the error class, the Not back yet copy, and describeUncertainRollingOutcome, which both loops use for their final toast when an outcome is unknown: confirmed count, members that failed, unanswered writes (fix(errors): say a write's outcome is unknown when it got no verdict, not that it failed #1897's sentence), and the member not back plus the ones left alone.
• src/integrations/api/instance/status/restartInstance.ts: the typed rejection, the two-schedule watch (a callback that throws is logged and the watch carries on), probes without gateway replay, the explicit request timeout, and the derived mark TTL.
• src/react-query/queryClient.ts: describeError and shouldToastOwnFailure recognise the new error.
• src/hooks/useRestartInstanceClick.ts: marks with the shared TTL and turns its toast into "Still restarting" halfway through the watch.
• src/hooks/useRestartClusterClick.tsx: shared TTL for both marks, a "Waiting On Instance i of n" toast, the stop, a failed count, and an uncertain outcome that outranks Cancel, which would otherwise hide the warning not to restart again.
• src/hooks/useRollingConfigUpdate.tsx: the same waiting toast, stop and Cancel precedence; it marks nothing, so there is no TTL to change.
• Chat tools: describeToolError.ts adds toastToolFailure; restartHTTPService/execute.ts and createApp/execute.ts return success: true with no-retry copy for an unconfirmed restart, and replace their progress toast on every failure, including the app-creation one.
• src/features/instance/operations/index.tsx: the Operations console's restart mark uses RESTART_MARK_TTL_MS instead of its own 5 minutes, since its waitUntilAnsweringAfterRestart watch is now longer; its test's module mock gains the constant.
• DESIGN.md: a new section recording the verdict rule, the shared deadline, the successful-probe-only rule, the loop stop, and the chat-tool contract.

✅ Verification

End-to-end route: integration-style tests through the real restartInstance, a real instance client (getInstanceClient with a fake adapter), and the real React Query mutation error handler; the loops are exercised with restartInstance mocked to reach each outcome. Not run against a live instance.

• Fails on base, passes here: the Restart-button test on base a35e2bed0 showed the bug's signature, two toasts Error / timeout of 3000ms exceeded and Error / Failed to restart instance. for a restart Harper answered 200; with the fix it shows one Not back yet toast and dismisses the loading toast. A second test holds the probe open past the first schedule and checks the mark is held, the button pending, "Still restarting" shown, then success once it answers.
• restartInstance.test.ts drives a real instance client: an accepted restart that never comes back rejects with RestartUnconfirmedError after exactly 24 probes on the 10s + 23×15s schedule, a slow target is still watched, refused / gateway / no-answer POSTs keep their own error, and RUM filtering is unchanged for wrapped timeouts and network errors.
• useRestartClusterClick.unconfirmed.test.tsx and useRollingConfigUpdate.unconfirmed.test.tsx: both loops stop at a member that never comes back, still carry on past a refusal, and keep the warning when Cancel is clicked; the cluster file also checks that a slow member that answers lets the loop go on, and that refused members are named. restartTools.test.ts pins both chat-tool results and toasts; queryClient.test.ts pins the shared copy.
• useRestartClusterClick.test.tsx adds RESTART_MARK_TTL_MS to its restartInstance mock, because the hook now imports it.
• Mutation check: 18 guards each reverted locally, and each turned at least one test red (wrapping the probe failure, the slow callback and its guard, the probe's gateway-replay opt-out, the second probe schedule, describeError, shouldToastOwnFailure, both loop stops and their Cancel precedence, the failed count, both chat-tool branches, the tool toast, the button's TTL and toast dismissal).
• Gates: npx vitest run 411 files / 3871 passed (also run by the pre-commit hook on every commit), npx tsc -b exit 0, npx oxlint --format stylish . exit 0, npx dprint check exit 0.
• Stack merge at 06dd4020: merged fix(errors): say a write's outcome is unknown when it got no verdict, not that it failed #1897, which carries fix(api): replay a gateway error only for reads, so a write is never sent twice #1891's gemini-code-assist follow-up (isReadRequest now reads an operation only from a POST body). No conflicts. Focused vitest run over src/integrations/api, src/lib/restart, src/react-query, src/hooks and src/features/instance/operations passed 50 files / 697 tests, and npx tsc -b exited 0.

Stack: based on #1897 (claude/1890-outcome-unknown-writes) → #1891 → #1882 → stage. Overlap with the other restart chain, #1861 → #1867 (claude/1853-restart-cluster-load-failure): it reworks the same two loops in useRestartClusterClick.tsx and useRollingConfigUpdate.tsx, so whichever lands second resolves those conflicts, keeping #1867's load-failure and Cancel handling alongside this PR's stop rule and outcome accounting. The new loop tests live in *.unconfirmed.test.tsx so they don't add/add-conflict with #1867's new useRestartClusterClick.test.tsx / useRollingConfigUpdate.test.tsx; #1867's "failed to restart" test throws a plain Error, which still reads as failed here.

Review coverage gaps: the Cursor legs failed on every round (git fetch refused by the 1Password SSH agent), and the Harper domain adjudication leg failed on auth (expired OAuth session), so no round was adjudicated; outside findings were triaged by hand on evidence. Two gemini findings were rejected as factually wrong: the button does dismiss its loading toast (pinned by the first test above), and useRollingConfigUpdate marks nothing.

Closes #1895

_{🤖 Generated with Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

Related PRs: #1897 overlaps (this PR's base), #1882 overlaps (base of the stack; shares the restart TTL), #1867 overlaps (reworks both restart loops), #1861 overlaps (reworks useRollingConfigUpdate)
Complexity: medium

_{Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt),domain(auth); declined=cursor-kimi,cursor-muse; rounds=3; full=1 @ 06dd402}

_{Review-Attention: study ~9m (raised: degraded review, open major) @ 06dd402}

[gemini-code-assist]

Code Review

This pull request introduces the RestartUnconfirmedError to handle scenarios where a restart request is successfully accepted by Harper, but Studio cannot confirm that the instance has recovered. This prevents these cases from being incorrectly flagged as failures, which could lead to redundant restarts. The changes update the restart logic, rolling configuration updates, and chat tools to handle this error gracefully with clearer user notifications, and include comprehensive unit and integration tests. There are no review comments to assess, and I have no additional feedback to provide.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	69.99%	11001 / 15717
🔵	Statements	70.11%	11721 / 16717
🔵	Functions	63.38%	2830 / 4465
🔵	Branches	64.7%	8369 / 12934

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/instance/applications/components/Chat/tools/describeToolError.ts	100%	50%	100%	100%
src/features/instance/applications/components/Chat/tools/createApp/execute.ts	75%	50%	50%	75%	21-23, 75-91
src/features/instance/applications/components/Chat/tools/restartHTTPService/execute.ts	77.77%	100%	100%	77.77%	42-52
src/features/instance/operations/index.tsx	98.71%	88.15%	100%	98.7%	96
src/hooks/useRestartClusterClick.tsx	92.06%	82.92%	60%	93.33%	41, 145, 183, 194, 217
src/hooks/useRestartInstanceClick.ts	89.65%	73.07%	50%	89.65%	43, 84, 98
src/hooks/useRollingConfigUpdate.tsx	89.09%	68.88%	66.66%	88.88%	158-164, 168-175, 198
src/integrations/api/instance/status/restartInstance.ts	100%	100%	100%	100%
src/integrations/api/instance/status/restartUnconfirmed.ts	100%	94.44%	100%	100%
src/react-query/queryClient.ts	98.11%	100%	85.71%	98.11%	120

Generated in workflow #2185 for commit 06dd402 by the Vitest Coverage Report Action