Repository navigation
fix(errors): say a write's outcome is unknown when it got no verdict, not that it failed - #1897
Merged
dawsontoth merged 16 commits intoOct 13, 2026
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a robust error-handling mechanism for 'unanswered writes'—write operations that fail with gateway errors (502, 503, 504) or network timeouts, leaving their execution status unknown. It adds a new utility module unansweredWrite.ts and updates various UI components, hooks, and mutations to prevent misleading 'failed' messages, instead prompting users to verify the outcome before retrying. Additionally, the PR updates DESIGN.md to document this behavior, adjusts gateway retry configurations for costly read operations like package_component, and includes comprehensive unit and integration tests. I have no feedback to provide as there are no review comments.
This was referenced Oct 11, 2026
fix(deploy): resend a live deploy only when Harper refused it, never after it may have started
#1898
Merged
dawsontoth
added this pull request to stack #1900
October 11, 2026 21:24
dawsontoth
added a commit
that referenced
this pull request
Oct 12, 2026
…es' into claude/1895-restart-accepted-not-back Brings in #1897's adjudicated-review fixes: a restart with no verdict keeps its mark until the target answers again, the license warning counts refused licenses, and an unanswered redeploy or import refreshes. Conflicts were import lists and adjacent inserts, kept from both sides: - useRestartInstanceClick.ts: the restartInstance import names both RESTART_MARK_TTL_MS and waitUntilAnsweringAfterRestart; the finally block holds the mark on an unanswered restart and keeps this branch's comment. - useRestartClusterClick.tsx: the catch counts instancesFailed and then waits on an unanswered member, after this branch's RestartUnconfirmedError break. - useRestartInstanceClick.test.tsx: both sides' imports. Also adds RESTART_MARK_TTL_MS to the new useRestartClusterClick.test.tsx mock, which this branch's hook now reads on every run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
… not that it failed A write answered 502/503/504, or not at all (a timeout, ERR_NETWORK), may already have been applied, but Studio showed it as an ordinary failure: the gateway's body or "Request failed with status code 504", and callers' own "Failed to restart …" / "Failed to import …" on top. Retrying by hand repeated the side effect. isUnansweredWrite (new) recognizes it from the request (isReadRequest) and the status, never the body. describeError checks it first and returns "Outcome unknown" copy, so the global mutation toast and every describeError caller get it. Callers that add their own failure toast to a mutation ask shouldToastOwnFailure first; rename and the chat tools, which have no mutation behind them, show the same copy; the cluster restart, rolling config and license loops count these apart from failures. secrets.ts no longer re-wraps such an error, so the predicate can still see it. package_component is classified as the read it is, and skips the gateway replay because each attempt rebuilds the archive. Closes #1890 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e TXT record Validating pending domains posts one request per domain and, on any failure, said "Validation failed! Please make sure the TXT record has been put in place." A validation central manager answered 502/503/504, or never answered, may have succeeded, so it is now counted apart: the toast says the outcome is unknown and the list is refetched to show what changed. Also from review: isUnansweredWrite checks the status before it parses a serialized body, so only an ambiguous failure pays for the parse; trims comments that only restated their identifiers; guards a test teardown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…answers again A restart answered 502/503/504, or not at all, may have run: central manager's proxy answers 504 after forwarding it, and Harper can go down mid-reply. The Restart button still released its restart mark at once on that failure, so the tracker's refetches hit an instance going down and their connectivity errors toasted beside "Outcome unknown". The cluster loop also released the member and restarted the next one while it might still be down, which defeats the one-at-a-time restart. Both now hold the mark until waitUntilAnsweringAfterRestart sees the target again, as the operations console already does: the button in the background, after showing the warning; the loop before it moves on. From the adjudicated cross-model review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sh after an unanswered deploy The license loop's "Outcome unknown" toast dropped the licenses that were refused outright, so a run with one of each left the refused one unaccounted for. It now adds "N other licenses failed to apply", as the domain validation toast does. A redeploy or import that got no answer is told to check whether it took effect, but nothing refreshed what the user would check. The redeploy modal now refetches the components list on that failure, live progress or not, and the import refetches the instance and reloads the file tree. Also trims the isUnansweredWrite JSDoc to the part the code doesn't say. From the adjudicated cross-model review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h gives up The note read as though the mark held indefinitely and the cluster loop never moved past a member that never answers. Both release when waitUntilAnsweringAfterRestart gives up, as they do after an accepted restart. From the adjudicated cross-model review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
force-pushed
the
claude/1890-outcome-unknown-writes
branch
from
October 13, 2026 00:52
844c8bd to
2aa5d26
Compare
…ailed The rebase onto stage brought in a write surface this PR's caller audit predates: activating a staged release (#1876's chain). Its mutation opts out of the global toast, like import and redeploy since #1871, so an activation that timed out or got a 504 showed "Failed to activate <app>" with axios's own text and no warning that it may have gone through. deployFailureTitle (new, beside the deploy reporters) is the one place a deploy's failure title is decided: "Couldn't confirm <app> was activated" for a write with no verdict, "Failed to activate <app>" otherwise. The import form and the redeploy modal, which the rebase had given their own copies of that rule, now use it too. reportDeployFailure shows the unknown-outcome copy, and the activation's progress panel says so as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eat their neighbours Review round after the rebase (codex nit, kept by the Harper domain adjudicator): DeployVerb had no consumer outside reportDeployHealth.ts, since every caller passes a literal. The redeploy modal's second refresh comment repeated the first, and the skipGatewayRetry type doc listed package_component, whose own call site already says why it opts out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at it failed restartInstance sent the restart and then watched for the target with user_info, and both steps rejected the same promise, so a target slow to answer again read as a failed restart everywhere: "Failed to restart instance.", "Only N of M instances restarted", and success: false to the chat agent. Each of those invites a second restart of a target that already went down once. Harper's answer to the restart request is its only verdict: `restart` answers before it tears down (bin/restart.ts schedules the teardown 50ms out), and `restart_service` once its job has started (executeJob). So a probe failure after a 2xx now rejects with RestartUnconfirmedError (cause: the last probe error), shown as "Not back yet ... check its status before you restart it again". - The watch keeps going past the usual recovery time (a second probe schedule, no second settle) and calls onSlowToReturn once in between, so the Restart button and both loops say "still waiting" instead of giving up. Every restart mark's TTL comes from the same constants, and the probes skip the gateway replay, so no mark lapses mid-watch. - The cluster restart and rolling config update stop at a member that never comes back instead of restarting the next one, which could leave no member serving, and report the members they left alone. - createApp keeps success: true (the app exists) and restartHTTPService returns copy without the "Error:" prefix telling the agent not to restart again. Both now replace their progress toast on any failure, which used to keep spinning. Closes #1895 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…members too Review of the previous commit: - A Cancel clicked while the loop waited on a member that never came back replaced the "Not back yet, check before you restart it again" toast with "Cancelled", hiding the one warning that matters. The uncertain outcome now wins; Cancelled still shows when nothing is uncertain. - The uncertain-outcome toast counted the members that refused only implicitly. It now says "N instances failed to restart", from one helper both loops share (describeUncertainRollingOutcome). - onSlowToReturn is queued rather than called inline, so a callback that throws cannot cut the watch short or become its cause. Not adopted: counting a 401/403 probe as the target being back. Through central manager's proxy a 403 is CM refusing the caller before it contacts the instance (notAllowed() in HDBInstance.js), so it says nothing about the target; recorded in DESIGN.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hat it never answered Review round 2: - A probe can fail with an answer (a 401/403, a Harper 500), so "hasn't answered since" was not always true. Every surface now says Studio couldn't confirm the target came back, which is what a failed watch proves. The watch itself still counts only a successful user_info as back: through central manager's proxy a 403 is CM refusing the caller (notAllowed() in HDBInstance.js) before it contacts the instance. - A throwing onSlowToReturn is caught and logged instead of escaping as an uncaught microtask exception; the watch carries on either way. - RollingOutcome is no longer exported; it has no consumer outside its module. Not adopted: dismissing the button's loading toast for an unconfirmed restart. It already is: the catch dismisses it before deciding whether to add its own toast. A test now pins that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ever answered Review round 3: DESIGN.md and restartInstance's doc comment still said the target "never answers", which contradicts the rule that only a successful user_info counts as back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efused config-update members Mutation checks after the rebase onto the new restart loops found three guards no test held: the restart request's explicit 60s timeout (the client default is also 60s, so it only shows with that default cleared), RESTART_MARK_TTL_MS on the Restart button and both cluster-loop marks (restoring the old five minutes stayed green, since sleep is mocked), and the rolling config update's failed count. - restartInstance: the request's bound, every probe's timeout and every wait add up to less than RESTART_MARK_TTL_MS. - The Restart button and the cluster loop still hold their marks six minutes in, past the old TTL, while the watch runs. - A config update names a member that refused alongside the one not back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fig fan-out After the rebase onto #1861's rolling config update, the stop rule also decides the fan-out: a canary that is not back never proved it can run the new configuration, so it is never fanned out from, and a stop after the fan-out keeps the note that the members left alone may already have the configuration saved. Both are pinned in useRollingConfigUpdate.test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ay what Cancel left Review round 1 after the port (codex major, kept by the Harper domain adjudicator): the cluster loop waits on a member whose restart got no verdict, but when that watch also ran out it discarded the rejection and restarted the next member. On a two-member cluster, a 504 after the restart ran plus a member that stays down left no member serving, which is exactly what the stop rule exists to prevent. The loop now stops there too, and the report says Studio couldn't confirm that member came back either, instead of claiming it accepted the restart. The adjudicator's nit: a Cancel after a no-verdict member now yields the "Outcome unknown" report, which never said Cancel skipped the rest. Both loops now name the members Cancel left alone in that report. Also from the round: drop the test comments that only narrated their setup, and trim restartInstance's JSDoc to its contract (DESIGN.md carries the reasoning). Not taken: Cancel during set_configuration still restarts that member; that check is #1867's, unchanged here. The watch outliving the progress toasts' durations is not a problem: sonner never starts an auto-close timer for a loading toast (sonner 2.0.8, dist/index.mjs:644). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…onor Cancel mid-write Review round 2 after the port (codex major twice; the Harper domain adjudicator kept both as minor and said the base stack could take them): - An unanswered restart_service moved on to the next member after 3s. If the restart ran onto a configuration that broke the member, the next member got the same configuration. The config update now waits on that member, as the cluster restart does, and stops there if the watch gives up too. An unanswered set_configuration does not wait: nothing restarted that member. - A Cancel clicked while a member's set_configuration was in flight still restarted it. The loop now stops before that restart, and the Cancelled report says the configuration may be pending there, as after a fan-out. - After a Cancel, the "Waiting On Instance" toast that onSlowToReturn brings back offered a second Cancel, as if the first had not landed. Both loops now show it without the action and say no other instance will be restarted or updated. Also drops the test helper comments the round flagged as narration. Rejected on evidence: gemini's "axiosRetry resends the restart POST" (the POST is sent once; axiosRetry wraps only the probes, and #1891's replay never repeats a write), and "a Cancel aborts the watch into RestartUnconfirmedError" (Cancel only sets a flag; no signal reaches the watch). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l was clicked Review round 3 (codex minor, kept by the Harper domain adjudicator): round 2 took the Cancel action off the slow-return toast, but the no-verdict follow-up toast round 2 added to the config update, the cluster loop's one, and both loops' "carry on momentarily" toasts still came back with a Cancel button after Cancel had closed the toast, and promised to carry on for a watch of up to about seven minutes. One helper per loop now builds every progress toast after the first: the usual Cancel action and copy, or, once Cancel was clicked, neither, and a note that no other instance will be restarted or updated. Also drops a comment that only restated its flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
marked this pull request as ready for review
October 13, 2026 02:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⊙ Problem
A write that gets a 502, 503 or 504, times out, or never gets a response may already have been applied. Studio still showed it as an ordinary failure. The global mutation toast showed the gateway's body (an nginx HTML page, or central manager's
Proxied operation timed out after 30s on instance …) or axios'sRequest failed with status code 504. Callers then added their own "Failed to restart instance.", "Failed to import …" or "Validation failed! Please make sure the TXT record has been put in place." Someone who believed that and tried again could apply the write a second time: insert the same records, restart again, deploy again. #1891 stopped Studio from replaying writes automatically. The manual retry this copy invited was the remaining way to send one twice.No status in that set shows the write was not applied. Central manager's operations proxy answers 504 after it has already forwarded the operation (central-manager
src/resources/hdbInstance/HDBInstance.js:91-95). Harper answers 503 when the worker it handed an operation to exits or runs out of time (harperserver/serverHelpers/registeredOperations.ts:129,:234). Axios reports a connection that dropped after the request went out asERR_NETWORK, which is the same code it uses for a CORS failure.💡 Solution
isUnansweredWriterecognizes these failures from the request and the status, never the body. It takes a realAxiosErrorthat carries itsconfigand has no verdict: a 502, 503 or 504, or no response withERR_NETWORK,ETIMEDOUTorECONNABORTED. Then it checks that the request was not a read, using #1891'sisReadRequest, so a write is classified by the same rule the gateway retry uses. That check comes last because it may parse a serialized body, and only these failures need it.describeErrorchecks it first and returns our own copy: the title "Outcome unknown", then the message "The server couldn't confirm whether that went through. Check whether it took effect before you try again, or it may happen twice." Every mutation that keeps the global toast now shows that copy, and so does every otherdescribeErrorcaller. The predicate only affects what is displayed. The error itself is not rewritten, becauseshouldKeepEventmatches timeouts andNetwork Erroron the message text, and callers readerror.response.status.The surfaces that word their own failure had to change too, or they would contradict that toast:
shouldToastOwnFailurefirst. That covers restart, secrets, orphan-blob cleanup, import, role add/edit/delete, cluster terminate, OAuth settings, and removing a user from the organization.describeError's text itself. Those are rename and the chat tools. The agent now reads the same warning throughdescribeToolErrorand does not retry an insert as though it had failed.describeUnansweredWritesinstead of "Only N of M restarted".request-timeout.⚖️ Alternatives
The planning review (codex) returned
Framing-Verdict: better-alternative-exists. I adopted all three corrections. The plan had left the per-target loops, the chat tools and the central-manager caller toasts for a follow-up, and they are all covered here. It had used one shared toast id for every unknown write, which would have let two concurrent warnings overwrite each other, so these toasts now have no id. It had also madepackage_componenta read, which would have let the gateway retry rebuild a large archive three more times, so that request now setsskipGatewayRetry.error.messageor wraps the error would also reach everyerror.messageandString(error)caller. Rejected:shouldKeepEvent(src/integrations/datadog/shouldKeepEvent.ts:224-275) recognizes timeouts, 401s, 5xx andNetwork Errorby message, so the [Monitoring] AxiosError: API request timeouts (15–60s) affecting multiple views (regression since v2.111.3) #1371 flood would return to Error Tracking. Callers that branch onresponse.statuswould also break. And it would cover instance clients only, not central-manager writes.errorHandler. Rejected: rename and the per-target loops never reach the global toast, the oldsecrets.tswrapping hid the status even from it, and callers' own "Failed to …" toasts would still contradict it.isIndeterminateplus a bounded refetch). This gives a real verdict, but many operations have no state that can be queried afterwards, so it can't replace the general warning. The domain validation loop does refetch now.🔧 Changes
src/integrations/api/unansweredWrite.ts(new): the predicate, the copy, the aggregate sentence, andNO_RESPONSE_CODES, now shared with the auth forms.src/react-query/queryClient.ts:describeError's unknown-outcome branch,shouldToastOwnFailure, and toasts that are not collapsed.src/features/auth/describeAuthFailure.ts: uses the shared code set instead of its own copy. Its behavior is unchanged.src/integrations/api/isReadOperation.ts:package_componentis a read (harpercomponents/operations.js:393-448only packs files). Before this change, a download that timed out would have been reported as possibly applied. The restart gate now holds it instead of refusing it, and the operations console no longer asks for confirmation before sending it.packageComponent.tssetsskipGatewayRetry, whichretryGatewayErrors.tsnow documents.src/integrations/api/instance/secrets/secrets.ts: an unansweredAxiosErroris rethrown unchanged, checked before fix(datadog): withhold failed secrets-operation text, which quotes secret names #1806'sSecretsOperationErrorwrapping: central manager's proxy answers a 504 with a JSONerrorbody of the same shape as a Harper refusal, so without the check first it would be re-wrapped and lose its status (pinned). Datadog reads onlyname,message,stack,causeanddd_contextfrom an error, notconfig(@datadog/browser-core7.15.0src/domain/console/consoleObservable.ts:72-119,src/domain/error/error.ts:139-142), and anAxiosError's message is axios's own, so fix(datadog): withhold failed secrets-operation text, which quotes secret names #1806's secret-name withholding is unaffected.shouldToastOwnFailure: restart, secrets add/edit/delete and grants, orphan-blob cleanup, role add, role edit/delete, cluster terminate, OAuth settings, and organization removal, which now shows no success toast when every failure was unanswered.useRenameFilesandRenameFileModal. Since Show a failed app import or redeploy once, and keep a failed import beside its form #1871 the deploy mutation opts out of the global toast, so every deploy surface shows the copy itself under one title,deployFailureTitle("Couldn't confirm my-app was imported / staged / redeployed / activated", never "Failed to …"): the import form, the redeploy modal's progress panel and late toast and buffered toast, and the staged-release activation, whose progress panel says so too.getClusterRestartTargets, the 5.2+ fan-out), with this PR's counting applied to them. The license warning also counts licenses refused outright beside the unanswered ones, as domain validation does. The cluster-restart and rolling-config warnings get that count from fix(restart): say a restart Harper accepted is not back yet, never that it failed #1899'sdescribeUncertainRollingOutcome, which rewrites both toasts.waitUntilAnsweringAfterRestartsees the target again or gives up, as the operations console already did. The Restart button shows the warning and then holds the mark while it watches. The cluster loop waits on that member before it restarts the next one. Releasing at once let the tracker refetch an instance that was going down, so connectivity errors toasted beside the warning, and let the loop take down the next member while this one might still be down.DESIGN.mdrecords it.describeToolError:createApp,deleteTableRecords,dropComponentFile,insertTableRecords,restartHTTPService,setComponentFile,updateTableRecords.DESIGN.md: a new note, "A write with no verdict never reads as failed".🔀 Rebase onto stage
Resolved by hand, against
git merge-tree --merge-base <old #1891 head> <new #1891 head> <old #1897 head>. Every auto-merged file matches that reference; the conflicted ones differ where noted:shouldToastOwnFailurechecks this PR had there would have left an unanswered import or redeploy with no warning at all. Instead each showsdescribeError's copy in its own display (the import form's inline failure, the redeploy panel, its late and buffered toasts) under a title that never says "Failed", and still refreshes what the warning says to check. Show a failed app import or redeploy once, and keep a failed import beside its form #1871's own tests moved into this PR's cases: import, redeploy; the.tsxtest files this PR used to add are gone.deployFailureTitlethe one place the deploy title is decided (pinned, panel).SecretsOperationErrorwrapping, as above.useApplyLicensesClick.test.tsxanduseRestartClusterClick.test.tsxnow exist on stage; this PR's cases were folded into them on stage's harness (licenses).Filed while reviewing: #1895. A restart Harper accepted, on a target that is slow to answer again, still shows "Failed to restart". That request was answered with success, so it is not an unanswered write, and this PR does not change it.
✅ Verification
Through the real client chain:
unansweredWrite.integration.test.tsxbuilds a realgetInstanceClientclient whose adapter fails with anAxiosError. It runs the request as a mutation through aQueryClientbuilt on the app'smutationErrorHandler. A 504 oninsert, a 502 ondeploy_component, a 503 with Harper's worker-exit text onrestart, and a timeout ondrop_tableeach raise exactly one "Outcome unknown" toast, never the gateway body. A refused write keeps the server's words, a read keeps its own text after the gateway retry gives up, and apackage_componentdownload is sent once on a 504 and is never called possibly applied.Each surface: restart shows one warning and no "Failed to restart". rename shows the shared copy. licenses counts an unanswered license separately from a refused one. domain validation mixes a timeout with a 400 and shows both counts. role add skips its own toast only when the request went unanswered. secrets set, delete and grant reject with the
AxiosErroritself. From the adjudicated round: restart keeps the mark until the target answers again and releases it at once on a refusal. The cluster loop waits on an unanswered member before it restarts the next one, and does not wait on a refused one. Licenses still counts a refused license beside an unanswered one. Import and redeploy, with and without live progress, refresh after an unanswered request and not after a refused one.Unit:
unansweredWrite.test.tscovers all three statuses, all three no-response codes, object and serialized bodies, central-manager bodies, 4xx/500, local failures (ERR_CANCELED, config errors,ERR_TARGET_RESTARTING), reads, and non-Axios lookalikes.queryClient.test.tschecks the exact copy and confirms that toasts are not collapsed.isReadOperation.test.tschecks thatpackage_componentis a read andpackage_component_projectis not.Mutation checks, each reverted afterwards. Removing
describeError's branch turns 8 tests red, and it puts back exactly the behavior this PR fixes, so it doubles as the fails-on-base check. Each of these turns at least one test red: removing the restart check, restoring the secrets wrapping, removing the rename branch, droppingpackage_component$, droppingskipGatewayRetry, collapsing the toast id, dropping the read filter (7 red), removing the license counter, removing the role-add check, and removing the domain-validation branch. Reverting each of the adjudicated round's fixes also turns its test red: the restart hold, the cluster wait, the license refusal count and the import refresh 1 each, and the redeploy refresh 2.Gates on the head:
npx vitest runexit 0 (520 files, 4983 passed, 1 expected fail, 11 skipped; the pre-commit hook also ran the full suite on every commit),npx tsc -bexit 0,npx oxlint --format stylish .exit 0,npx dprint checkexit 0, all on Node 24.21.0.Mutation checks on the rebase's own guards, each reverted locally and each turned a test red: the secrets unanswered-first check (the 504-with-JSON test), the import title and its refresh, the redeploy title in both paths and both refreshes, the cluster and rolling-config unanswered counting, the cluster loop's wait, the license refused count, the shared
deployFailureTitlebranch, and the activation's toast and panel copy.Not browser-verified. A gateway status can't be produced on demand against a live instance, so the adapter-level test above is how this was verified end to end.
Cross-model review: four rounds. Rounds 1 and 2 ran codex (graded) and gemini only, because the Harper domain leg failed
auth(an expired OAuth session) both times, so nothing was adjudicated then. Round 1 found the domain-validation loop, which is fixed, and the slow-recovery restart, which is A restart Harper accepted reads as failed when the target is slow to answer again #1895. I rejected these findings on evidence:useRestartInstanceClickhas no mutation behind it. It does:useRestartInstanceis auseMutation, and the restart test runs through the global handler and sees the warning.EditUserModalcan leave the modal stuck.setIsRemoving(false)andonUserUpdated()run after every branch.RenameFileModal. That catch does not exist. The modal's only catch is the one this PR changed.Rounds 3 (full, at
a35e2bed) and 4 (delta, ata72b4b9a) ran codex, gemini and the Harper domain adjudicator. Both wereindependent=true, round 4 reportedconvergence=converged, and the adjudicated severity is minor. Accepted and fixed: an unanswered restart released its mark at once, the license warning dropped refused licenses, an unanswered redeploy or import refreshed nothing, the new DESIGN.md note overstated how long the mark holds, and one JSDoc only restated its code. Kept open, not fixed here:secrets.ts:60-61). Pre-existing, and codex's and gemini's "major" was downgraded to a nit.shouldToastOwnFailure's JSDoc opens by restating its use. fix(restart): say a restart Harper accepted is not back yet, never that it failed #1899 rewrites that comment.Rejected on evidence: Gemini said a user removal whose every role removal goes unanswered shows nothing. Each removal is a
useMutationwithoutskipGlobalErrorToast, so the global handler toasts it. The Cursor legs were not selected in rounds 3 and 4. In rounds 1 and 2 they failedno-receipt(the 1Password SSH agent refusedgit fetch).Rebase review round (full, at
03165c9c, after the rebase and the activation fix): codex (graded), gemini and the Harper domain adjudicator ran,independent=true, adjudicated severity minor. Both Cursor legs failedno-receipt(the 1Password SSH agent refused theirgit fetch).DeployVerbno longer exported (no consumer outside its module), and two comments that repeated their neighbours dropped.restart_service. Harper replaces HTTP workers an eighth at a time with the replacements started first (harperserver/threads/manageThreads.js:1403-1419), so the member keeps answering, and the diff does not change that control flow. fix(restart): say a restart Harper accepted is not back yet, never that it failed #1899 now waits and stops there too, for a restart that may have run onto a configuration that broke the member.useRemoveUserFromOrganizationRoleis auseMutationwithoutskipGlobalErrorToast, so each failure reaches the global handler's "Outcome unknown" toast.Delta round (at the head, after the nit commit): codex, gemini and the adjudicator re-ran; the two nits are resolved, nothing new in the delta. Left as filed: the relayed-500 ❓ below, the two loop cases fix(restart): say a restart Harper accepted is not back yet, never that it failed #1899 handles, two why-comments the adjudicator would still trim (
useImportApplication.ts's refresh note,queryClient.ts's never-collapsed note), and a pre-existing nit: a directory rename whose finaldrop_componentgets no verdict does not reload the tree the warning tells the user to check (RenameFileModal.tsx; this PR changed only its toast). Gemini's "EditUserModalstays open" was dropped again:setIsRemoving(false)andonUserUpdated()run on every path.Closes #1890
🤖 Generated with Claude Code (Claude Opus 5.5); posted via @dawsontoth.
Related PRs: #1891 overlaps (stack base, this PR uses its isReadRequest), #1882 overlaps (stack base, below #1891), #1899 overlaps (stacked on this PR: restarts Harper accepted but never confirmed back, the loop counts, and the stop at a no-verdict member that never came back), #1871 overlaps (merged; opted the deploy mutation out of the global toast, so import and redeploy now show the warning themselves), #1806 overlaps (merged; secrets error wrapping), #1861 overlaps (merged; rolling config loop), #1867 overlaps (merged; restart and license loops), #1876 overlaps (merged; the staged-release activation this PR now covers), #1898 overlaps (merged; when the deploy fallback may resend)
Complexity: medium
Review-Coverage: authored=claude; ran=codex,gemini; adjudicated=domain; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt); declined=cursor-kimi,cursor-muse; rounds=2; full=1 @ 7b6f747
Review-Attention: study ~15m (sensitive: describeAuthFailure.ts, SecretGrantsEditor.tsx +2; decisions: status-only-verdict, one-predicate-both-clients, package-component-as-read, advance-after-watch-gives-up, per-caller-toast-guard, do-less-alternative) @ 7b6f747