Three items deliberately deferred from the #3409 PR 0 stack (#3418 + #3438), recorded so they aren't lost. All three are pre-existing behaviors that the new services/scriptDispatch.ts seam now makes cheap to fix in one place.
1. Partial-batch semantics in executeScriptOnDevices
apps/api/src/services/scriptExecution.ts — the per-device loop does if (!dispatch.ok) throw new Error(dispatch.error). The comment claims the failure codes are unreachable because the eligibility filter (org access / OS / decommissioned) runs upfront, but that filter is one SELECT taken before a loop that then awaits a DB insert and a WS send per device. A device can transition (e.g. to decommissioned) inside that window.
When it fires: devices already processed have real script_executions / device_commands rows and may already be delivered, but the function throws, the route has no local catch, and the caller gets one generic 500 with no per-device breakdown. The batch row is also left pending because the status flip to queued only runs after the loop.
Fix direction: collect per-device dispatch failures and continue, returning them alongside executions, and only fail the whole call when nothing dispatched.
2. Commit-before-deliver on the manual/route path
dispatchScriptToDevice performs the device_commands INSERT and the sendCommandToAgent WS push inside the caller's ambient transaction for the manual and mobile paths. A fast agent can therefore report a result on another connection before the request commits, so handleScriptResult's UPDATE matches 0 rows and the output is dropped.
This is byte-for-byte the behavior the pre-#3418 code had, and it's the same class the AI path already fixes explicitly (see the two-phase runOutsideDbContext / withSystemDbAccessContext escape and its comment in services/aiToolsScripts.ts, plus the executeCommand precedent in services/commandQueue.ts). Now that all callers funnel through one seam, fixing it once covers everything.
Note for whoever picks this up: the escape must commit the insert under a real system-scoped context and then poll outside any held context — nesting the wait inside the same transaction reproduces the bug under a different transaction.
3. Unbounded audit fan-out in queueCommand
Every script command now emits an agent.command.script audit row via queueCommand, which fires a fire-and-forget runOutsideDbContext(() => withSystemDbAccessContext(...)) transaction per device (services/commandQueue.ts). Against a pool of ~30 (US prod lower), a large fleet run is the pool-starvation shape that has caused incidents before.
#3438 mitigated the entrypoint with .max(500) on deviceIds in executeScriptSchema and a comment recording why the cap exists; the automation path is separately bounded by runWithConcurrency(..., 5). The structural fix — batching the audit writes, or awaiting them under a concurrency limit — was out of scope for an extraction PR because queueCommand is a hot path shared by every command type.
Context
Found during the five-lens review of #3418 (general code review, test coverage, silent failures, type design, comment accuracy). Everything else that review found is fixed in #3438.
Three items deliberately deferred from the #3409 PR 0 stack (#3418 + #3438), recorded so they aren't lost. All three are pre-existing behaviors that the new
services/scriptDispatch.tsseam now makes cheap to fix in one place.1. Partial-batch semantics in
executeScriptOnDevicesapps/api/src/services/scriptExecution.ts— the per-device loop doesif (!dispatch.ok) throw new Error(dispatch.error). The comment claims the failure codes are unreachable because the eligibility filter (org access / OS / decommissioned) runs upfront, but that filter is oneSELECTtaken before a loop that then awaits a DB insert and a WS send per device. A device can transition (e.g. todecommissioned) inside that window.When it fires: devices already processed have real
script_executions/device_commandsrows and may already be delivered, but the function throws, the route has no local catch, and the caller gets one generic 500 with no per-device breakdown. The batch row is also leftpendingbecause the status flip toqueuedonly runs after the loop.Fix direction: collect per-device dispatch failures and continue, returning them alongside
executions, and only fail the whole call when nothing dispatched.2. Commit-before-deliver on the manual/route path
dispatchScriptToDeviceperforms thedevice_commandsINSERT and thesendCommandToAgentWS push inside the caller's ambient transaction for the manual and mobile paths. A fast agent can therefore report a result on another connection before the request commits, sohandleScriptResult's UPDATE matches 0 rows and the output is dropped.This is byte-for-byte the behavior the pre-#3418 code had, and it's the same class the AI path already fixes explicitly (see the two-phase
runOutsideDbContext/withSystemDbAccessContextescape and its comment inservices/aiToolsScripts.ts, plus theexecuteCommandprecedent inservices/commandQueue.ts). Now that all callers funnel through one seam, fixing it once covers everything.Note for whoever picks this up: the escape must commit the insert under a real system-scoped context and then poll outside any held context — nesting the wait inside the same transaction reproduces the bug under a different transaction.
3. Unbounded audit fan-out in
queueCommandEvery script command now emits an
agent.command.scriptaudit row viaqueueCommand, which fires a fire-and-forgetrunOutsideDbContext(() => withSystemDbAccessContext(...))transaction per device (services/commandQueue.ts). Against a pool of ~30 (US prod lower), a large fleet run is the pool-starvation shape that has caused incidents before.#3438 mitigated the entrypoint with
.max(500)ondeviceIdsinexecuteScriptSchemaand a comment recording why the cap exists; the automation path is separately bounded byrunWithConcurrency(..., 5). The structural fix — batching the audit writes, or awaiting them under a concurrency limit — was out of scope for an extraction PR becausequeueCommandis a hot path shared by every command type.Context
Found during the five-lens review of #3418 (general code review, test coverage, silent failures, type design, comment accuracy). Everything else that review found is fixed in #3438.