Repository navigation
fix(gitops): capture Pilot recovery and stop refused rollbacks from parking the application - #2087
Merged
Merged
Conversation
The leaf's apply-local route honored allowGitManagedContent and captureRecovery only under the node_proxy machine scope, but a request forwarded through the Pilot tunnel arrives with the agent's pilot_tunnel loopback token. The auth middleware treats the two scopes as the same hub-to-leaf credential and every sibling route already accepts both, so the gate silently dropped the capture and a later rollout rollback on a Pilot target failed with no recovery point to restore. Accept either machine scope for both flags. Browser sessions and API tokens hold neither and are still refused.
A rollback a node refused before its restore touched anything was recorded application-wide as a failed recovery. The source facet then read recovery_failed, offered no action, withheld deploys, and stopped polling, while the only exit, a successful restore, was impossible on the refused target. Recovering it took manual row edits. Classify each failed restore by whether it can have mutated the target. The restoring node's own pre/post verdict, attached to the compensation error and carried over the rollback response, wins; otherwise only codes that prove the restore moved nothing are pre-mutation. A pre-mutation refusal is recorded on the target and settles the application restoring stamp in the same transaction, keeping a same-ref completion receipt when another target came back. A 2xx that does not echo the requested generation is its own unconfirmed code and still holds, because the node did restore something unproven. A later successful deploy acknowledgement now retires a stale recovery failure on the acknowledging target and, when no target still reports one, on the application.
A target refused before its restore changes anything keeps its visible failure, but it no longer holds the application. The source keeps converging and the next successful deploy to that target retires the stale failure; only a rollback that may have changed a target holds automated work.
…sals A later pre-mutation refusal could erase the application hold an earlier target still needed when that target may have been half-restored: the settle cleared the stamp without checking the other targets, and a retry refused during an unresolved failure overwrote the target's moved class. The settle now restores the hold from a surviving mutated failure, keeps the stamp while any restore is still moving, excludes the refusing targets themselves, and only a pre-mutation refusal defers to an earlier failure's class. A refusal whose own record write throws is settled by the route for every missed target, not just the first. Also carried through the same path: the node's own pre/post classification rides the compensation result and every uncoded refusal (a restore-not-sent outcome for a request that never left the hub, a coded stack-not-found, the self-stack refusal), stack-busy answers hold instead of settling because another operation is moving that stack, a failed mid-write restore whose own revert failed is classified post-mutation, and Enforce runtime repair follows the same pre-mutation release rule.
… claims The settle no longer keys on one operation's recovery ref: it derives the application phase from the live targets, so two overlapping rollbacks can no longer pin the application in restoring, and the other operation's refusal settles it. The open writes the target's active-operation marker, so a crashed open is selected by boot and reclassified instead of staying restoring, and a missed refusal record is retried for its target rather than only settling the application. The health executor settles a proven refusal whose record throws, the same as the manual route. A recovery hold is now claimed by one shared predicate, targetRecoveryFailureMoved: a failure stage that has moved on no longer claims a recovery, and a pre-mutation refusal never did. The settle, the acknowledgement clearing, and the repair binding use it, and the failure writers clear a superseded recovery phase, so a deploy failure on a refused target cannot re-park the application or re-hold repair.
… retry A deploy failure that replaced a moved recovery failure cleared the target's claim but left the application hold with no claimant, and the acknowledgement's clearing branch then never ran. Claims are now retired through one helper that re-derives the application hold in the same transaction; a successful bind, an acknowledgement and a revived placement retire the claim too, and every rollback terminal retires the recovery_started interruption a crashed open left. Repair checks a restore in flight before the failure claim, so a retry over a refused target holds repair again. A terminal rollback whose recovery ref no longer matches the target changes nothing, a newer rollback open takes over the active marker, and a failed record is retried with the class the attempt was proving for either class.
A recovery claim that may have moved a target was retired whenever a deploy or withdraw failure replaced it, releasing the application hold with nothing proving the target consistent. The claim now keeps its own class in a dedicated column, so a later failure records itself beside the claim instead of overwriting it. Only success evidence (an acknowledgement or a bind) retires a moved claim; a claim that moved nothing is still retired by a later failure. A restore in flight is never rewritten: retirement returns early on capturing, restoring and compensating, and a deploy, withdrawal or acknowledgement terminal that clears its own marker hands it back to the restore so a crash remains boot-recoverable. Superseded terminals are scoped by class: a refusal that moved nothing is a true no-op with no row write, history row or application stamp, while a failure that may have moved the target records its class without touching the newer attempt's phase, ref or marker. rollbackCompleted carries the same ref guard, and the settle re-reads each row after its retry and drops one a newer rollback re-opened. The application hold counts a restoring sibling as a claimant, and a tombstoned target stops answering for the hold it claimed.
A second rollback could open over a target whose restore was still running, take over its recovery slot and ref, and leave the application hold released while the first restore was still moving files. One restore per target is now the rule: rollbackInProgress and recoveryStarted refuse a target that is already restoring, the route answers with 409 ROLLBACK_IN_PROGRESS before any restore starts, and the health executor skips its rollback on a target that is already restoring. With overlap impossible, the machinery that arbitrated it is gone: the ref and marker takeover, the skipped-terminal plumbing and the ref guards. A refusal that defers to a moved claim leaves the shared failure slot alone, so a later deploy failure is not relabelled a recovery failure, and the claim keeps its own class and time in dedicated columns. rollbackCompleted, recoverySucceeded and a moved rollbackPartialFailed derive the application stamp from the live targets instead of writing it outright, so one target's outcome cannot release or overwrite a sibling's hold. A restore marker is handed back on every terminal that clears it, including through a tombstone, and both revival paths share one helper.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
The superseded-terminal tests removed with the rollback serialization also took the boot test that drove the real startup selection, leaving its import unused. Restore it against the current semantics: the open's marker makes the application selectable, boot reclassifies the restore as interrupted, and a later terminal rollback retires the interruption.
The health executor opened a rollback and then ran the hold and two store reads outside the block that records a terminal. A throw there left the target restoring, and with rollbacks now serialized per target that state refused every later rollback in-process. Everything after the open now lives inside the recording try, so every exit records a terminal; a regression test forces the hold to throw and checks that a later manual rollback is accepted. Boot also selects applications whose own or any target's recovery phase is in flight without an active-operation marker, and records the interruption as recovery_started, so a row an older build left restoring is reclassified instead of being refused for ever. The docs now say every Git-managed rollout deploy captures the node's previous project on every target type, and that a target can have one rollback at a time. Tests removed with the rollback serialization were restored where the behavior still exists.
Boot reclassification wrote recovery_started into the application's interruption_stage for a marker-less in-flight restore. No recovery terminal clears an application interruption stage, so after any crashed rollback the value stayed for ever, and the drift collectors read a set application stage as a standing suppression while a Direct source deferred its retry. The application row records the interrupted recovery in its recovery phase; the target keeps the operation stage. A regression test reclassifies a crashed rollback and asserts the application stays clearable through a later successful rollback. The route's 409 test now asserts the live rollout authorization and generation are untouched, so a regression that supersedes the rollout before refusing fails the test. The repair test for a refused recovery replaced by a deploy failure now drives the real writers, and the rollback docs no longer claim the source keeps converging.
…pture-and-rollback-hold
A held application settles every evaluation into recovery_required, which armed neither a retry cursor nor a fresh poll. The fetch that ran while the hold stood consumed the poll cursor, so once success evidence cleared the hold the source sat idle: a pushed commit was never fetched and next_poll_at stayed null until the poll interval was saved again. The controller now re-arms the poll for a recovery wait, an in-flight restore or a hold that success evidence retires. Automatic acceptance stays gated on success-shaped outcomes, so the hold still withholds progression. A regression test holds an application through the real rollback writers, consumes the cursor through the real fetch transitions, and asserts the cursor is re-armed. The rollback section of the Blueprint docs again states that every Git-managed rollout deploy captures the node's previous project on every target type.
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.
Summary
Two defects on the same path, found by QA on a three-node fleet (Local, proxy, Pilot).
1. Pilot deploys never captured a recovery generation. The leaf's
POST /api/blueprints/apply-localhonoredallowGitManagedContentandcaptureRecoveryonly for thenode_proxymachine scope, while a request forwarded through the Pilot tunnel authenticates aspilot_tunnel(the agent mints the loopback token; the auth middleware treats both scopes as the same hub-to-leaf credential). The flags were silently dropped, so a later rollout rollback on that target failed with "This stack has no recovery point to restore."2. A refused rollback parked the whole application. The failure was recorded application-wide as
recovery_phase='failed'plusfailure_stage='recovery'. The source facet then readrecovery_failed, offered no actions, withheld deploys, and stopped re-arming polls. The only exit, a successful restore, was impossible on the target with nothing to restore, so clearing it took manual database edits.What the fix does
apply-localaccepts either machine scope (node_proxyorpilot_tunnel) for both flags. Browser sessions and opaque API tokens hold neither and are still refused.pre_mutation/post_mutationverdict, carried on the compensation error, wins; otherwise only codes that prove the restore moved nothing are pre-mutation. A pre-mutation refusal is recorded on the target and does not hold the application. Anything that may have moved the target, including an unconfirmed response or a transport failure, holds the application, Enforce runtime repair, and the source pipeline.recovery_failure_classandrecovery_failure_at, so a later deploy or withdraw failure records itself beside the claim instead of erasing or relabelling it. Only success evidence (an acknowledgement or a bind) retires a claim, and a claim that moved nothing is retired by a later failure too.409 ROLLBACK_IN_PROGRESSbefore opening any target or withdrawing the live rollout;rollbackInProgressand the DirectrecoveryStartedrefuse a target that is already restoring; the health executor skips it with a log line. Every exit after a successful open records a terminal, including a thrown hold.Test plan
cd backend && npx tsc --noEmit: clean.npm run lint: 0 errors.node_proxyandpilot_tunnelhonor both flags; cookie refused; the binding reaches the service).rollbackCompleted,recoverySucceededand a movedrollbackPartialFailed, both revival paths, and the tombstone residue.Notes
gitops_target_current.recovery_failure_classandrecovery_failure_at, applied by the existing boot migration. There is no backfill: Sencho is greenfield, so no pre-column rows exist, and a row whose shared failure slot a later failure had already taken over carries no proof of what the claim was.stack:deployplus per-targetrequireDeployOnTarget, and the new refusal is a 409 with codeROLLBACK_IN_PROGRESS. The rollback response shape is unchanged, and the frontend surfaces the server's error text.