feat(delivery): openframe-machine-delivery — engine, dispatcher and the first two specs (PR 1 of the plan) - #2200
Draft
semen-flamingo wants to merge 7 commits into
Draft
semen-flamingo wants to merge 7 commits into
semen-flamingo wants to merge 7 commits into
Conversation
Shape only, nothing wired: DeliveryKind/DeliveryStatus/DeliveryFailure, MachineDelivery document + repository, DeliverySpec + registry (mirrors NotificationTypeRegistry), DeliveryProperties (defaults + per-kind YAML overrides, gated by openframe.rmm.delivery.enabled) and the first spec, ToolInstallationDeliverySpec, on a new publish(machineId, message) overload of ToolInstallationNatsPublisher. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
Adds the pieces that make the shape reviewable end to end, still unwired: DeliveryDispatch (row + publish, data-nats), DeliverySweepService with the three PENDING branches (offline wait/skip, republish, exhausted), DeliveryWatchdogService for silent ACKED rows, DeliveryTracker (acknowledge/complete), DeliveryFailureRecorder, DeliveryMetrics, DeliverySweepScheduler under ShedLock, row-level policy override (DeliveryProperties.resolve(MachineDelivery)) and a second spec, ClientUninstallDeliverySpec, whose onFailed restores a machine parked in PENDING_DELETION. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
Answers "how do we know a delivery succeeded and stop re-sending": - ScriptExecutionAcknowledgeMessage gains kind/targetId; the listener routes any kind to DeliveryTracker.acknowledge (PENDING -> ACKED) and keeps the script path for legacy/SCRIPT_SCHEDULE acks. - InstalledAgentService completes TOOL_INSTALLATION rows on installed-agent, AgentUninstallService completes CLIENT_UNINSTALL on /api/agents/uninstall. - ToolInstallationService and ForceClientUninstallService publish through DeliveryDispatch, so the row exists before the message leaves. DeliveryDispatch and DeliveryTracker are interfaces with a recording and a pass-through/no-op implementation selected by openframe.rmm.delivery.enabled, so with the flag off every call site behaves exactly as today. Sweep and watchdog get separate try/catch in the scheduler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
… request, kind -> type - New module openframe-machine-delivery (com.openframe.delivery): engine only, no NATS dependency. data-nats depends on it and hosts the NATS specs. - Specs build their own DeliveryRequest (type, targetId, payload, publisher); call sites shrink to spec.request(...) + dispatch.send(request), so the targetId a spec publishes with is the one its completion hook expects. - DeliveryKind -> DeliveryType everywhere (document field, ack message, YAML `types`, metric tag `type`), matching NotificationType. - MachineDelivery moves to document.delivery, repository to repository.delivery. - MachineOnlineStatus is the single place the engine reads the Machine document. - Flags: openframe.delivery.enabled for the engine, openframe.delivery.sweep.enabled for the scheduler, so publishers of deliveries never run the sweep. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
…strategy behind the flag Mirrors NotificationEmitter: callers hand a Seed to DeliveryDispatcher.dispatch, the registry resolves the spec by the seed's type, the spec builds the DeliveryRequest and publishes. DeliveryRecorder (Mongo when enabled, noop otherwise) is the only flag-dependent piece; publishing is identical either way. Call sites no longer inject concrete specs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
… a follow-up Reverts the call-site wiring (ToolInstallationService, ForceClientUninstallService), the ack routing (ScriptExecutionAcknowledgeMessage type/targetId, listener) and the complete() hooks (InstalledAgentService, AgentUninstallService) with their tests. They come back as the next PR on top of this one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
…DeliveryId MachineDelivery is a plain @DaTa document like its neighbours; the id layout is engine knowledge used only by the recorder and the tracker. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY
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.
Draft. This PR adds a module and two specs and wires nothing into existing flows: no service changes behaviour after merge, with or without the flag. The wiring (ack routing, success hooks, call sites) is the stacked follow-up PR on top of this branch.
Context
Four server→agent flows (tool install, tool update, client update, client uninstall) go through JetStream with a durable consumer per machine; the server keeps no state and has no ack, watchdog or metric for them. Scheduled scripts already use an application-level model (
ScriptDeliveryRetryService,ScriptExecutionAcknowledgeListener, watchdog,openframe-rmm.yamlalerts). This is the shared engine that the four flows (and later the scripts) move onto; each delivery type is a spec, the same pattern asNotificationTypeSpec+NotificationTypeRegistry+NotificationEmitter.Module layout
Publishers of deliveries live in five modules of three services (api, management, client); only client will run the sweep. Hence a module every publisher can include, with the engine's only contact with the machine document isolated in
MachineOnlineStatus. "Machine" in the name is deliberate: the engine is transport-agnostic (specs ownpublish) but assumes a recipient with online/offline presence — it is not a generic outbox for push or Slack.The pattern, mirrored from notifications
NotificationTypeDeliveryTypeNotificationSeed(per type, nestedSpec.Seed)DeliverySeed(per type, nestedSpec.Seed)NotificationTypeSpec<S>DeliverySpec<S, P>—getType,getSeedClass,getPayloadClass,request(seed),publish(machineId, payload),onFailed(row, failure)NotificationTypeRegistryDeliverySpecRegistryNotificationEmitter.notify(request)DeliveryDispatcher.dispatch(seed)A caller hands a seed to the dispatcher and knows nothing else:
The dispatcher resolves the spec by
seed.type(), the spec builds theDeliveryRequest(payload,targetId,machineId),DeliveryRecorderwrites thePENDINGrow, the spec publishes. ThetargetIda spec publishes with is the one its completion hook will receive from the agent (toolAgent.getKey()↔agentTypein installed-agent; machine id ↔X-Machine-Idon/api/agents/uninstall), so that pairing lives in one class per type.How SUCCESS is detected and when re-delivery stops (wired in PR 2)
A row in
machine_deliveryisPENDING → ACKED → DONE | FAILED. The sweep only ever selectsPENDING, so re-delivery stops the moment the row leavesPENDING. No new subject carries success — the signals the agent already sends will close the row:machine.{id}.execution.acknowledge+type,targetIdScriptExecutionAcknowledgeListener→DeliveryTracker.acknowledgemachine.{id}.installed-agent {agentType, version}afterinstall()InstalledAgentService→complete(TOOL_INSTALLATION, agentType, machineId)POST /api/agents/uninstallAgentUninstallService→complete(CLIENT_UNINSTALL, machineId, machineId)script-execution.resultRmmResultService→complete(SCRIPT_SCHEDULE, executionId, machineId)installed-agent, matched on versionWhat the agent does not send today is a failure: after ack, a failed install is visible only as
FAILED/TIMEOUTfrom the watchdog; an explicit failure result is a Rust-side follow-up (PR 3).Engine
Override chain
Numbers never live in a spec. Resolution order, all in
DeliveryProperties:openframe.delivery.defaults.*(required, validated at startup) →openframe.delivery.types.<TYPE>.*→ row-levelofflineBehavior/reconnectWindowSeconds(carried fromScheduleScript). A spec overrides behaviour only:request,publish,onFailed— compareToolInstallationDeliverySpec.onFailed(no-op) withClientUninstallDeliverySpec.onFailed(restores a machine parked inPENDING_DELETION).What is in this PR
DeliverySeed,DeliverySpec,DeliverySpecRegistry,DeliveryRequest,DeliveryDispatcher,DeliveryRecorder(+ mongo / noop),DeliveryTracker(+ mongo / noop),DeliveryProperties,DeliverySweepService,DeliveryWatchdogService,DeliveryFailureRecorder,DeliveryMetrics(openframe.delivery.retried{type},openframe.delivery.failed{type,reason}),DeliverySweepScheduler,MachineOnlineStatus. 27 unit tests.DeliveryType,DeliveryStatus,DeliveryFailure,MachineDeliveryindocument.delivery; data-mongo-sync:MachineDeliveryRepository.ToolInstallationDeliverySpec,ClientUninstallDeliverySpecwith nestedSeed;publish(machineId, message)overloads and publicbuildMessageon the two publishers (existing callers unchanged). 5 tests.shedlock-springpinned like client-core (not managed by the parent).Nothing outside the module and data-nats changes behaviour. Every flag-dependent bean has a no-op counterpart selected by
matchIfMissing, so services that include the module without the YAML boot as before.Plan by PRs
type/targetIdonScriptExecutionAcknowledgeMessage+ listener routing;complete()inInstalledAgentService/AgentUninstallService;ToolInstallationServiceandForceClientUninstallServicethroughDeliveryDispatcher. Flag still off everywhere.tool_installation_message_listener.rs,client_uninstall_message_listener.rs→ core subscribe + ack withtype/targetId(pattern:execution_listener.rs); delete deadtool_uninstall_message_listener.rs; explicit failure result. Owner needed.legacy-jetstream-publish; row only for machines whose agent ≥ the version from 3 (installed_agents,openframe-client), so old agents never produce false FAILED.openframe.deliveryYAML block,enabled=true(client, api, management),sweep.enabled=true(client only), pinoss.libs.version; dev → stage → prod.openframe_delivery_failed_total{type,reason}inopenframe-rmm.yaml.ScheduleFireDispatcher→ dispatcher,ScriptScheduleDeliverySpecin client-core because itsonFailedneedsScheduleJobExecutionWatchdogService, deleteScriptDeliveryRetry*andScheduleDeliveryRepublisher); ad-hoc scripts from api-lib gain the retry they lack today.machine.all.*→ per-machine fan-out with version on the row; deletePublishState+AgentVersionUpdatePublishFallbackScheduler; management dispatches through the same specs;CLIENT_UPDATElast (it is the agent rollout channel).legacy-jetstream-publish=false, drop$JS.API.*from the NATS configmap, delete the streams explicitly (their consumers go with them), Mongock dropscript_delivery_retry, removeopenframe.rmm.execution.retry.*.In parallel, outside the plan: NATS
service/adminpasswords into ExternalSecret per tenant.Open decisions
ScriptExecutionAcknowledgeMessagewithtype/targetId(PR 2) vs. a new subject.PENDING_DELETIONon exhausted uninstall: revert + alert (done here inClientUninstallDeliverySpec) vs. leave.Verification
mvn -pl openframe-machine-delivery,openframe-data-nats,openframe-client-core,openframe-api-service-core -am test -Dtest='Delivery*Test,*DeliverySpecTest,*DeliveryRecorderTest,MongoDeliveryTrackerTest,MachineOnlineStatusTest'— build green on JDK 21; existing client-core and api-service-core sources are byte-identical tomain.🤖 Generated with Claude Code
https://claude.ai/code/session_01BnYUpYgKvuMw6hB5VZSimY