Add backend WARP actions for nodes#184
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Production smoke test update (2026-06-03):
Note: direct HTTP calls to the backend container IP are intentionally rejected by the existing proxy-check middleware; the successful test used the production HTTPS domain. |
Greptile SummaryThis PR adds five WARP management endpoints to the nodes module (status, install, enable, disable, uninstall), proxying requests through the backend to each node's local WARP API. It also extends the contract schemas with
Confidence Score: 3/5Safe to merge for most scenarios; the main concern is that WARP action responses silently omit warp state data when the node stats cache has expired, which could leave the frontend with an incomplete view after the very operation the user just triggered. The five new warp endpoints are structurally sound and correctly guarded. The mergeWarpStatus silent early-return means that on a cold cache the response from any of the four mutation endpoints will not contain the warp status object, even though the action ran successfully on the node. This is a real, if intermittent, data-freshness gap on the critical happy-path response. The inconsistent import alias and error-re-wrapping pattern in getNodeWarpStatus are minor but add noise to an otherwise clean implementation. src/modules/nodes/nodes-system-cache.service.ts — the mergeWarpStatus early-exit and the inconsistent import path both live here and warrant a second look before merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Client
participant Ctrl as NodesController
participant Svc as NodesService
participant Repo as NodesRepository
participant Cache as NodesSystemCacheService
participant Axios as AxiosService
participant Node as Remote Node
C->>Ctrl: POST /nodes/:uuid/actions/warp/install
Ctrl->>Svc: installNodeWarp(uuid)
Svc->>Repo: findByUUID(uuid)
Repo-->>Svc: NodesEntity (or null)
alt node missing / disabled / disconnected
Svc-->>Ctrl: fail(error)
Ctrl-->>C: HTTP error
end
Svc->>Axios: installWarp(address, port) [timeout 180s]
Axios->>Node: POST /node/warp/install
Node-->>Axios: TWarpStatus response
Axios-->>Svc: ok(TNodeWarpResponse)
Svc->>Cache: mergeWarpStatus(uuid, warpStatus)
alt stats cache is present
Cache->>Cache: spread stats + warp → set with TTL
else cache is absent
Cache-->>Svc: (silent no-op — warp not cached)
end
Svc->>Repo: findByUUID(uuid)
Svc->>Cache: getOne(uuid)
Cache-->>Svc: INodeSystem (may lack warp field)
Svc-->>Ctrl: ok(NodeResponseModel)
Ctrl-->>C: HTTP 200 NodeResponseModel
|
| if (!nodeResult.isOk) { | ||
| return fail({ | ||
| code: nodeResult.code ?? ERRORS.NODE_NOT_FOUND.code, | ||
| message: nodeResult.message ?? ERRORS.NODE_NOT_FOUND.message, | ||
| }); |
There was a problem hiding this comment.
Inconsistent error propagation in
getNodeWarpStatus
All four other warp action methods (enable/install/disable/uninstall) forward the nodeResult failure directly (return nodeResult), but getNodeWarpStatus re-wraps it with fail({ code: nodeResult.code ?? ..., message: nodeResult.message ?? ... }). If getNodeForWarpAction returns NODE_IS_DISABLED or "Node is not connected", the code and message from those errors are still propagated correctly here (the ?? fallback is never reached in practice). However, the pattern diverges from the other four methods without any apparent reason, making the error-handling contract harder to reason about.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| import { Injectable } from '@nestjs/common'; | ||
|
|
||
| import { RawCacheService } from '@common/raw-cache'; | ||
| import { TWarpStatus } from '@libs/contracts/models'; |
There was a problem hiding this comment.
Inconsistent import path alias
TWarpStatus is imported via @libs/contracts/models here, but both nodes.service.ts and axios.service.ts use the @contract/models alias for the same type. The two aliases likely resolve to the same location at runtime, but the inconsistency will cause confusion and may trip up tools that track import sources.
| import { TWarpStatus } from '@libs/contracts/models'; | |
| import { TWarpStatus } from '@contract/models'; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Verification
nodestable shows all 4 nodes connected and enabled