π fix(agent): cross-host update notifications and toasts now fire for agent-side containers (#289)#392
π fix(agent): cross-host update notifications and toasts now fire for agent-side containers (#289)#392s-b-e-n-s-o-n wants to merge 2 commits into
Conversation
β¦perations Cross-host updates (#289) silently dropped notification triggers (Pushover) and produced incomplete SSE toasts for containers running on a connected agent. The controller built the agent-scoped operation row from buildAgentOperationBase without the container snapshot, so when markOperationTerminal fired emitTerminalLifecycleEvent, the resulting emitContainerUpdateApplied / emitContainerUpdateFailed carried only {operationId, containerName, containerId}. The trigger handler then fell back to findContainerByBusinessId, which compares the agent's bare containerName (`tautulli`) against the controller's fullName (`mediavault_docker_tautulli`) and silently dropped. Threads container through buildAgentOperationBase / ensureAgentOperationForTerminal / markAgentOperationTerminal / maybeMarkAgentOperationSucceededFromAppliedPayload / maybeMarkAgentOperationFailedFromFailedPayload, stamping agent: this.name. Existing-row race (dd:update-operation-changed before dd:update-applied) patches container onto the active row via updateOperation before the terminal emit. Adds container to MutableUpdateOperationFields so patch types accept it. Fixes: #289
β¦er Unreleased Adds an Unreleased entry for the agent-scoped update-operation container threading fix shipped in a7f633b so the rc.27 release notes pick it up alongside the rc.26 batch.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
biggest-littlest
left a comment
There was a problem hiding this comment.
Approve: Diagnosis matches the symptom in the reporter's recording β only one toast appears for two updated containers because the agent-scoped operation row carries no container snapshot into the terminal lifecycle emit, dropping the trigger handler's lookup. Threading container through buildAgentOperationBase / ensureAgentOperationForTerminal is the right shape; the existing-row updateOperation patch closes the dd:update-operation-changed-first race without overwriting. Tests cover both branches and the race.
ALARGECOMPANY
left a comment
There was a problem hiding this comment.
Approve: container addition to MutableUpdateOperationFields and Pick-extended patch types are safe β no existing call site is constrained on the exact field set, and the persistOperationPatch spread accepts the new field at runtime. agent: this.name stamp prevents an inbound spoofed agent name from confusing controller-side container resolution. CHANGELOG entry is under Unreleased so rc.27 picks it up.
Codecov Reportβ All modified and coverable lines are covered by tests. π’ Thoughts on this report? Let us know! |
|
Superseded by #393. The two commits on this branch ( |
Fixes #289 β a follow-on cross-host case the rc.20 / rc.25 fixes did not cover.
Reporter tested rc.25 by running "Update All" on a
tautullistack with one instance on his controller host (datavault) and one on a connected agent host (mediavault). Both updates succeeded, but only the controller-side container produced a UI success toast and a Pushover notification; the agent-side container produced neither.Cause
When the agent finishes an update it emits
dd:update-appliedover SSE to the controller carrying a fullcontainersnapshot. The controller'sAgentClient.handleEventroutes the payload throughmaybeMarkAgentOperationSucceededFromAppliedPayloadβmarkAgentOperationTerminalβensureAgentOperationForTerminalβupdateOperationStore.insertOperation+markOperationTerminal. The helperbuildAgentOperationBaseconstructed the inserted row from{id, kind, containerName, containerId, newContainerId}only β the container snapshot was dropped on the floor.When
markOperationTerminalthen firedemitTerminalLifecycleEvent,buildTerminalLifecycleEventBaseemittedemitContainerUpdateApplied({operationId, containerName, containerId})with no container. The notification handlerhandleContainerUpdateAppliedEvent(app/triggers/providers/Trigger.ts) then fell back tofindContainerByBusinessId(containerName), which compares the agent's barecontainerName(tautulli) against the controller-sidefullName(mediavault_docker_tautulli) β and silently dropped. Same shape on thedd:update-failedbranch viamaybeMarkAgentOperationFailedFromFailedPayload.This is the same class of
findContainerByBusinessIdmiss as #385 but on the agent-scoped operation path that #385 did not cover.Fix
Thread the agent's container snapshot through every level of the agent-scoped operation pipeline so the store's terminal-lifecycle emit naturally carries it:
buildAgentOperationBaseaccepts an optionalcontainerand stampsagent: this.nameon it (caller wins β the controller's view always overrides any inboundagentfield).ensureAgentOperationForTerminalpatches the container onto an existing active row (thedd:update-operation-changed-before-dd:update-appliedrace) viaupdateOperation, but only when the existing row lacks a snapshot β never overwrites.markAgentOperationTerminal/maybeMarkAgentOperationSucceededFromAppliedPayload/maybeMarkAgentOperationFailedFromFailedPayloadthread the container through.containeris added toMutableUpdateOperationFieldsinapp/store/update-operation.tsso terminal and active patches accept it.No changes to the agent SSE serializer or to the controller-side dispatch path β the fix is upstream of the emit so the existing
payloadContainershortcut inhandleContainerUpdateAppliedEventnow succeeds for agent-originated updates. The earlyreturninAgentClient.handleEventis preserved, so there is no double-emit risk.Tests
app/agent/AgentClient.test.ts:should terminalize agent update-applied payloads with operation ids through the storetest to assertinsertOperationandmarkOperationTerminalare now called withcontainer.agent === 'test-agent'.container.agent.dd:update-operation-changedalready inserted an active row without container, thendd:update-appliedarrives; the active row is patched viaupdateOperationwith the container snapshot before the terminal emit runs.dd:update-failedwith container threads container intomarkOperationTerminal.Verification
[Unreleased]so the rc.27 release notes pick this up alongside the rc.26 batch.Closes #289