sidecar: defer single-step deployment restore to wake (CL-6648) - #327
Merged
Conversation
Boot-time restore currently replays a deployment's frozen `sources`
snapshot verbatim, including a dead credential frozen before a
provider was reconfigured. These tests pin the fix: a single-step
("warm-keep") deployment's restore must defer to its existing wake
path (which re-resolves inference sources fresh against the live
tenant catalog) rather than ever spawn from the frozen sources, while
a true multi-step deployment keeps restoring eagerly, unchanged.
Boot-time restore reused a deployment's persisted `sources` verbatim
for every deployment, including a single-step ("warm-keep") one whose
chain died after a provider was reconfigured -- the deployment reads
as "already live" to every later check, so nothing ever refreshed it.
A single-step deployment already has a working lazy-wake port
(ensureAwake -> wakeFoldedRun -> deployAtHead) that re-resolves
inference sources fresh against the tenant's current catalog on every
wake. `restoreDeploymentFromRecord` now detects that shape from the
record alone (its `sources` map always carries exactly one entry per
step) and defers to that path instead of restoring eagerly, so a room
from any earlier build wakes and works against the current catalog on
its next message -- without a new sidecar-to-hub source-resolution
channel, which the sidecar has no pre-connect path to build. A true
multi-step deployment has no such wake port yet, so it keeps
restoring eagerly from its frozen sources, unchanged.
Updated the existing quarantine tests' fixture to a two-step
definition, since they exercise the (unchanged) eager-restore
classification path this change now reserves for multi-step
deployments.
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
sources[]verbatim, including a chain whose credential died after a provider was reconfigured -- the deployment came back "live" but permanently broken, with nothing to ever refresh it.ensureAwake->wakeFoldedRun->deployAtHead) that re-resolves inference sources fresh against the tenant's live catalog on every wake.restoreDeploymentFromRecordnow detects that shape from the record alone (itssourcesmap always has exactly one entry per step -- no schema change needed) and defers restore to that path instead of replaying the frozen snapshot.Why this shape
The sidecar's boot-time restore runs entirely before the hub WebSocket connection opens, has no HTTP/RPC path to the hub, and the on-disk
WorkflowDeploymentRecordcarries no tenant id -- there is no way for restore itself to callresolveDefinitionSources(hub-side, DB-backed). Rather than build a new sidecar-to-hub source-resolution channel, this defers to the existing, already-correct wake path: the address stays unroutable until its next message or routine fire, at which point the ordinary wake redeploys it against a current catalog resolution. This is the forward-compat contract for every future update -- a room from any earlier build wakes and works, and a record whose closure or address really is corrupt still surfaces (just at its next real deploy attempt, via the wake path, instead of preemptively at boot).Test plan
WORKBENCH_CHECK_SINCE=origin/main bun run typecheck-- cleanWORKBENCH_CHECK_SINCE=origin/main bun run lint-- 0 errors (pre-existing warnings unrelated)WORKBENCH_CHECK_SINCE=origin/main bun run test-- 212/212 sidecar tests passbun run check:structural-- all checks passapps/sidecar/test/workflow-restore-defer-to-wake.test.ts): a single-step record with a dead source is never spawned or gated on buildability at restore and stays untouched on disk; a multi-step record still restores eagerly from frozen sources, unchanged.:3000directly: after this lands, restart the sidecar, confirm the boot log's "Deferred N single-step workflow deployment(s) to their lazy-wake path" line for dana's tenant, then message one of her previously-broken rooms and confirm a real Ollama reply.Refs CL-6648.