refactor(api): share the command-result handlers with the HTTP transport (#3097)

[bdunncompany]

Step 2 of 2 on #3097, as sequenced in that thread. #3172 (the reaper guard) landed in v0.104.0; this is the extraction.

The bug

The per-command-type result handlers lived inside agentWs.ts and were only ever reachable over the websocket, so a result submitted to POST /agents/:id/commands/:commandId/result reached none of them.

For script that means script_executions is never updated on the HTTP path. Measured on a live instance: 394 of 1070 executions (37%) carried a timeout status that did not match what happened, 89 of them having completed successfully with the output sitting in device_commands.result the whole time. #3172 stopped the reaper mislabelling those; this stops them being stranded.

Handlers byte-identical

The handler block and its commandResultHandlers registry move verbatim from routes/agentWs.ts to services/commandResultHandlers.ts. Everything they touch is an importable service or job, so there is no edge back into agentWs.

The moved block is 385 lines before and after, and the diff between the two is eleven lines:

• four handler signatures take commandId as a parameter
• the four result.commandId reads that fed them become that parameter
• two dynamic ./agents/helpers imports become ../routes/agents/helpers
• the registry gains export

Nothing else in the block changed. The two dynamic imports stay dynamic. Reproduce the proof with:

git show origin/main:apps/api/src/routes/agentWs.ts   # slice handleDiscoveryResult..registry
diff -u <that slice> <the same slice of services/commandResultHandlers.ts>

Rebased onto a3dc568 so this carries your #3223 SNMP backoff change rather than reverting it — that landed inside the moved block, and the diff above is exactly what caught it.

commandId as a parameter

Per your ruling on the issue: option 1. The two transports disagree structurally, not just cosmetically — the websocket envelope carries commandId inline while REST takes it from the path and authorizes against that path value. A body-supplied id read inside a handler would be the confused-deputy shape you described, so the id is supplied by whichever transport authorized it and is never read off the payload.

Your grep warning was right: 4 of ~45 result.commandId reads were inside the moved block and became the parameter; the other 41 stay in agentWs reading the parsed envelope.

Schema unification (the one intentional behaviour change)

The two commandResultSchema definitions diverged on the 1 MB cap: routes/agents/schemas.ts measured Buffer.byteLength, agentWs.ts used .length (UTF-16 code units), so the websocket accepted roughly 3x the intended budget for CJK-heavy output while REST rejected at 1 MB. The byte-accurate one is now canonical; agentWs extends it with the two envelope fields it adds (type, commandId).

As you noted, the cap is a .refine() on the result field, not the object — so the shared base stays a plain ZodObject that REST consumes directly and the websocket can still .extend().

Which types the HTTP path now dispatches

Five: network_discovery, hyperv_backup, mssql_backup, snmp_poll, script — the ones this route never post-processed at all.

Deliberately not the whole registry. Thirteen of its eighteen keys already have an equivalent inline block in the route (backup verification, restore, vault sync, sensitive-data, CIS); dispatching those as well would run each of them twice.

Converging the overlapping thirteen is left as follow-up rather than folded in here, because they are nearly but not exactly equivalent: the CIS and sensitive-data handlers forward the derived stdout while the route's inline blocks forward normalizedData verbatim. Replacing them would change what those two post-processors receive for a result sent as result: {...} with no stdout — a real behaviour change, and beyond the one this PR claims. Happy to do that convergence as its own PR if you want it.

One related asymmetry I did not touch: on a validation failure REST returns early, while the websocket still dispatches the handler for TERMINAL_TRANSITION_FAMILIES_ON_VALIDATION_FAILURE so backup/restore records transition to failed. Pre-existing, separate question.

No DB-context wrap on the HTTP side

agentAuthMiddleware already holds an org-scoped withDbAccessContext open across the request, with the same shape runWithAgentOrgDbAccess builds (scope organization, the device's orgId, accessibleOrgIds: [orgId], no partner access). The websocket needs its own wrap only because that path deliberately runs contextless (#3021).

This route is not one of the SELF_MANAGED_DB_CONTEXT_ACTIONS — those match on the final path segment, which here is result, not commands — so the request-long wrap is active. A nested wrap would be a no-op regardless (withDbAccessContext returns fn() unchanged when a context is already on the ALS), and opening a second real transaction is the #1105 double-hold this route was cleaned up to avoid.

The registry is imported dynamically here, like the DR handler below it: it pulls in the discovery and SNMP workers and through them the Drizzle schema module, which is more than this hot route should carry statically — and enough to break suites that partially mock db/schema.

Tests

• a script result over HTTP reaches the shared handler with the path-derived commandId, the resolved device and the derived stdout
• a type the route handles inline is not re-dispatched through the registry

Both were run against the un-fixed code first: emptying the dispatch set fails the first and only the first; adding cis_benchmark to it fails the second.

The #1105 enqueue contract scan now covers services/commandResultHandlers.ts as well as agentWs.ts. The pipeline's seven enqueue sites are now five plus two, and the contract is about the pipeline rather than the file — leaving the scan pinned to agentWs.ts would have silently dropped the two that moved. Confirmed by unwrapping one of the moved enqueues and watching the scan name it by file and line.

Local gate (on a3dc568)

• tsc --noEmit across apps/api — exit 0, no output
• vitest run across apps/api — 1284 files, 20500 tests, 0 failures, 5 files / 48 tests skipped
• eslint on all five changed files — exit 0, clean

Closes #3097.