feat: swamp serve WebSocket API + @swamp/client package#892
Conversation
Introduces a WebSocket API server for remote workflow and model method execution, plus a standalone TypeScript client published to JSR as @systeminit/swamp-lib. - `swamp serve --port --host --repo-dir` starts an HTTP+WS server - Multiplexed request/response protocol with cancellation support - Per-model locking for concurrent workflow isolation - Client supports callback-based and AsyncIterable streaming APIs - Health check endpoint at GET /health Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ethod_output events Wraps method.execute() with console interception so that extension models using console.log have their output streamed through the event system, matching the behavior of out-of-process drivers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The withConsoleCapture wrapper on the raw driver path caused concurrent forEach iterations to stomp on each other's console references. Removing it — in-process console.log output flows to the host process stdout which is sufficient for swamp serve. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
-
serve.ts:69-71—console.logbypasses LogTape and breaks--jsonmode
TheonListencallback has both alogger.infocall (line 65) and a bareconsole.log(line 69) that emit the same "server is ready" information. Theconsole.logoutputs raw text unconditionally, so in--jsonmode it produces a plain-text line intermixed with LogTape's JSON objects, making the output stream unparseable by tools likejq. Since thelogger.infoalready covers this (and LogTape handles JSON serialization correctly), theconsole.logshould be removed. If thews://URL format is preferred, fold it into thelogger.infomessage.Affected scenario:
swamp serve --json | jq .would fail to parse. -
serve.ts:65-68— startup message missing WebSocket URL
Thelogger.inforeports{host}:{port}but not the fullws://URL. Users have to mentally construct the URL. Small wording improvement: log"WebSocket API server listening on ws://{host}:{port}"so the connect address is directly copy-pasteable. -
No structured JSON startup event
When run with--json, orchestration scripts have no machine-readable signal for "server ready" beyond what LogTape happens to emit. A minimal JSON event like{"status":"listening","host":"...","port":9090,"url":"ws://..."}emitted on startup would let scripts reliably detect readiness without scraping log output.
Verdict
PASS — the new swamp serve command is well-structured, flags are consistent with existing commands, and the help text is clear. The console.log / JSON-mode interleaving issue is worth fixing but the severity is low in practice (serve is typically used in log mode, not piped to jq).
There was a problem hiding this comment.
Code Review
Blocking Issues
-
Libswamp import boundary violation:
src/serve/connection.ts,src/serve/deps.ts, andsrc/serve/serializer.tsimport from internal libswamp module paths (../libswamp/context.ts,../libswamp/workflows/run.ts,../libswamp/models/run.ts,../libswamp/errors.ts) instead of fromsrc/libswamp/mod.ts. Per CLAUDE.md: "CLI commands and presentation renderers must import libswamp types and functions fromsrc/libswamp/mod.ts— never from internal module paths." All required symbols (createLibSwampContext,workflowRun,modelMethodRun,SwampError,WorkflowRunDeps,ModelMethodRunDeps) are already exported frommod.ts. -
Missing AGPLv3 license headers:
packages/client/client.ts,packages/client/mod.ts,packages/client/protocol.ts, andpackages/client/stream.tsare missing the required copyright header. Per CLAUDE.md: "All.tsand.tsxfiles must include the AGPLv3 copyright header." -
No test coverage: 1,389 lines of new code across 11 new files with zero test files. Per CLAUDE.md: "Comprehensive unit test coverage" is required and "Unit tests live next to source files." At minimum, the serializer, connection handler, protocol parsing, client, and stream helpers all need tests. The
serializer.ts(pure functions) andstream.ts(pure helpers) are easy targets for unit tests. -
No authentication on WebSocket API: The server accepts connections from any client that can reach the port. With
--host 0.0.0.0, this exposes workflow and model method execution to the network with zero auth. This is a security concern — at minimum, add a--tokenflag for bearer token auth, or emit a prominent security warning when binding to non-loopback addresses.
Suggestions
-
JSON output mode: CLAUDE.md states "Every command must support both 'log' and 'json' output modes." The
servecommand creates a context withoutputModebut doesn't use it for its own output (e.g., the startup message goes toconsole.loginstead of respecting the output mode). -
Protocol type duplication:
packages/client/protocol.tsduplicatessrc/serve/protocol.tswith 200+ lines of mirrored types. Consider whether the client package could import from a shared protocol definition (e.g., via the workspace), or at minimum add a comment/test that validates the two stay in sync. -
deno-lint-ignore no-explicit-anyusage: There are 6no-explicit-anyignores inpackages/client/client.ts. CLAUDE.md requires "noanytypes." Some of these could be replaced withunknownor properly typed generics (e.g.,PendingRequest<T>handlers could useEventHandlers<StreamEvent>instead ofEventHandlers<any>). -
DDD:
src/serve/deps.tsmixes domain service construction with infrastructure concerns (log file path assembly,runFileSinkregistration). Consider extracting the log setup into a dedicated infrastructure factory so the dependency wiring stays focused on domain orchestration.
…ty warning - Fix libswamp import boundary: serve modules now import from src/libswamp/mod.ts instead of internal paths - Add AGPLv3 license headers to all packages/client/*.ts files - Add security warning when binding to non-loopback addresses - Remove bare console.log from onListen (logger.info covers it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- serializer_test.ts: serializeEvent, serializeSwampError, jsonSafeClone - protocol_test.ts: type validation, discriminated unions, JSON round-trips - stream_test.ts: SwampClientError, withDefaults, consumeStream, result - client_test.ts: connect/close lifecycle, workflowRun, error handling, AsyncIterable streaming, socket close rejection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
-
Mixed logger instances in serve.ts: The startup message uses
ctx.logger(line 43) while the listen and shutdown messages use the module-levellogger(lines 71, 101). Both resolve to the same LogTape category["serve"]so output is consistent, but the inconsistency could confuse future maintainers who assumectx.loggeris the canonical way to emit output in a command. -
Security warning silenced by
--quiet: The non-loopback warning (logger.warn) is suppressed when--quietis active (quiet sets log level toerror). A user who runsswamp serve --host 0.0.0.0 --quietwill get no indication that unauthenticated connections are accepted. Consider usingctx.logger.warnand checking whether it should survive quiet mode, or at minimum note it in the flag description for--host. -
servenot inSKIP_EXTENSION_COMMANDS: The server loads all user extensions on startup (models, vaults, drivers, datastores, reports) before binding the port. This is likely intentional since the server runs models/workflows in-process, but it means a cold start could be noticeably slow. A comment or the--helptext could set expectations (e.g. "Extensions are loaded from the repository on startup"). -
JSON mode is implicit:
swamp serve --jsonworks — LogTape formats all server lifecycle messages as structured JSON. But there is no explicit structured event like{"event":"server_started","host":"...","port":9090}. For a long-running server this is arguably fine (log-as-JSON is the normal pattern), but if scripted consumers want to reliably detect the ready signal they have to parse free-form log messages.
Verdict
PASS — no blocking UX issues. The command is well-described, flags have clear defaults, the security warning is present (for non-quiet runs), and JSON mode is handled via LogTape consistently with how other long-running-style commands work.
There was a problem hiding this comment.
Code Review
Blocking Issues
-
No unit tests for any new code. CLAUDE.md requires comprehensive unit test coverage, with unit tests living next to source files (
foo.ts→foo_test.ts). This PR adds 13 new files with ~1,469 lines of production code and zero test files. At minimum, the following need tests:src/serve/serializer.ts→src/serve/serializer_test.ts(pure function, trivially testable)src/serve/connection.ts→src/serve/connection_test.ts(message parsing, dispatch logic, error handling)packages/client/client.ts→packages/client/client_test.ts(message handling, AsyncIterableQueue)packages/client/stream.ts→packages/client/stream_test.ts(consumeStream, result, withDefaults)
-
No input validation on WebSocket messages (security).
src/serve/connection.ts:62-63parses incoming JSON withJSON.parseand casts directly toServerRequestwithout any schema validation. Since WebSocket messages are untrusted user input, this is a system boundary that requires validation per CLAUDE.md's security guidelines. Fields likepayload.workflowIdOrName,payload.modelIdOrName, andpayload.methodNameare passed directly into domain operations. At minimum, validate that required string fields exist and are strings, and thattypeis one of the known request types. Consider using Zod schemas (already used elsewhere in the project) for theServerRequesttype. -
src/serve/connection.ts:121-124— potential information disclosure in error for unknown request types. The lineUnknown request type: ${(request as { type: string }).type}reflects user-supplied input back in the error message. While lower risk over WebSocket than HTTP, this is still a pattern to avoid. Consider using a fixed message like"Unknown request type"without echoing the value.
Suggestions
-
src/serve/connection.tsimportsacquireModelLocksfrom../cli/repo_context.ts. While not violating the libswamp boundary rule, having theservemodule depend oncliinternals creates an awkward dependency direction (serve → cli). Consider extractingacquireModelLocksto a shared infrastructure or domain service so bothcliandservecan use it independently. -
src/cli/commands/serve.ts—--jsonoutput mode support. CLAUDE.md states "Every command must support bothlogandjsonoutput modes." The serve command creates a context withctx.outputModebut the long-running server doesn't appear to use it for structured JSON output of server lifecycle events (startup, shutdown, connections). Consider how--jsonwould work for this command. -
Protocol type duplication between
packages/client/protocol.tsandsrc/serve/protocol.ts. The comment explains the rationale (zero CLI dependencies for the client package), which is reasonable. However, these will drift over time. Consider adding a comment or test that verifies the two protocol definitions stay in sync. -
DDD observation:
src/serve/deps.tsis essentially an Application Service factory. ThecreateWorkflowRunDepsandcreateModelMethodRunDepsfunctions assemble domain services into dependency bags. This is appropriate for the application layer and correctly keeps domain logic out of the serve/connection handler. The overall architecture follows DDD principles well — domain objects are used through proper service boundaries. -
packages/client/client.ts:46-49—deno-lint-ignore no-explicit-anysuppression onPendingRequest. The genericanyonhandlersandqueuecould potentially be tightened with a union type matching the two concrete event types (WorkflowRunEvent | ModelMethodRunEvent).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CLI UX Review
Blocking
-
Missing JSON-mode output —
src/cli/commands/serve.ts,onListencallback and shutdown messageThe
swamp servecommand requires bothlogandjsonoutput modes (CLAUDE.md: "Every command must support both 'log' and 'json' output modes"). All output — the startup "listening on" message and the "Shutting down..." message — goes through the raw LogTapelogger.infocalls, completely bypassingctx.outputMode. Runningswamp serve --jsongives zero machine-readable output: no signal the server started, no address/port, no shutdown notification.Suggested fix: In the
onListencallback, branch onctx.outputMode:logmode: keep the existinglogger.infocalljsonmode: writeJSON.stringify({ status: "listening", host: hostname, port: listenPort })to stdout
Similarly emit
{ status: "stopped" }on shutdown in JSON mode. This makes it scriptable (e.g. a CI job waiting for the server to be ready can parse the JSON line).
Suggestions
-
Security warning goes to log only —
src/cli/commands/serve.tsline 55–58The non-loopback binding warning (
logger.warn) is not surfaced in--jsonmode. A script that startsswamp serve --host 0.0.0.0 --jsonand parses the output stream won't see this advisory. Consider including awarningfield in the JSON startup object (e.g.{ status: "listening", ..., warning: "no auth enforced on non-loopback address" }) when applicable. -
No
--verboseflag —src/cli/commands/serve.tsOther execution commands (
workflow run,model method run) expose--verboseto surface per-step detail. The serve command silently acceptsverbose: truein the WebSocket payload but offers no way to enable verbose logging for all connections from the CLI. Minor, since server verbosity is usually controlled differently, but worth noting for consistency.
Verdict
NEEDS CHANGES — The swamp serve command has no --json output mode support, violating a hard CLAUDE.md requirement.
There was a problem hiding this comment.
Code Review
Blocking Issues
-
Missing unit tests for
src/serve/connection.ts— This file contains significant logic (request dispatch, cancellation, lock acquisition, error handling) but has noconnection_test.ts. CLAUDE.md requires "Comprehensive unit test coverage" with "unit tests live next to source files." ThehandleMessagedispatch, duplicate ID rejection, cancel flow, and socket close cleanup are all unit-testable with a mock WebSocket. -
Missing unit tests for
src/serve/deps.ts— The factory functionscreateWorkflowRunDepsandcreateModelMethodRunDepsconstruct dependency bags and should have tests verifying the wiring is correct. CLAUDE.md requires comprehensive test coverage. -
No runtime validation of WebSocket message payloads —
connection.ts:63doesJSON.parse(event.data) as ServerRequestwith only a check forrequest.typeandrequest.id(line 69). If a client sends{ type: "workflow.run", id: "x", payload: {} }(missingworkflowIdOrName), the request passes validation and fails deep in libswamp with a cryptic error. The protocol types define required fields (workflowIdOrName,modelIdOrName,methodName) that should be validated at the boundary before dispatching. -
Missing unit test for
src/cli/commands/serve.ts— Other CLI commands in this codebase have companion test files (e.g.,data_get_test.ts). The serve command should have at least basic tests for option parsing. See the pattern insrc/cli/commands/data_get_test.tsreferenced in CLAUDE.md.
Suggestions
-
DDD layering:
src/serve/as a new top-level module — The existing architecture hassrc/cli/,src/domain/,src/infrastructure/,src/libswamp/, andsrc/presentation/. The HTTP/WebSocket server insrc/serve/is infrastructure-level code. Consider placing it undersrc/infrastructure/serve/to be consistent with the DDD layering (or document the rationale for keeping it top-level). -
src/serve/deps.ts:97-98— ThelogCategoryis always an empty array, which means all concurrent model method runs share the same log sink registration key. This could cause log interleaving or premature unregistration if multiple requests run in parallel on the same connection. -
No rate limiting or max connection limits — The WebSocket server accepts unbounded concurrent connections and requests. For production use, consider documenting this limitation or adding basic bounds.
-
packages/client/client.ts:267—const swampError = event.error as anyloses type safety. Since the error event type is defined in the protocol, this could use theSerializedErrortype instead.
…ests
- Add Zod validation for incoming WebSocket messages (rejects malformed
payloads at the boundary instead of crashing deep in libswamp)
- Fix information disclosure: unknown request types no longer echo
user-supplied input in error messages
- Add JSON output mode for swamp serve (emits structured
{"status":"listening",...} and {"status":"stopped"} events)
- Add connection_test.ts (13 tests: validation, dispatch, cancel, dupes)
- Add serve_test.ts (5 tests: command name, options, description)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
-
Log-mode startup message omits the WebSocket URL. JSON mode emits
url: "ws://host:port"which is exactly what a user needs to paste into client config. Log mode only printshost:port—"WebSocket API server listening on {host}:{port}". Consider printingws://{host}:{port}in the log-mode message too, so users don't have to construct it themselves. -
shutdown()callslogger.info("Shutting down...")unconditionally (both modes). TheonListencallback uses a cleanif (isJson) { ... } else { logger.info(...) }pattern. The shutdown path breaks this by callinglogger.infoafter the JSON branch, so in JSON mode the logger still fires. Minor inconsistency, but easy to align with theonListenpattern. -
Health check endpoint not surfaced in startup output. The server exposes
GET /healthbut neither log mode nor JSON mode mentions it on startup. A single extra field in the JSON ("healthUrl": "http://...") or a second log line would help operators script liveness checks without reading the docs.
Verdict
PASS — flags are consistent with the rest of the CLI (--repo-dir, --port, --host all match existing conventions), JSON output shape is well-formed and machine-friendly, and the security warning for non-loopback binding is a nice touch. No blocking issues.
There was a problem hiding this comment.
Code Review
Well-structured PR adding a WebSocket API server and standalone client package. The architecture follows the existing patterns cleanly.
Blocking Issues
None.
Suggestions
-
Stack trace exposure in
serializeEvent(src/serve/serializer.ts:63-64): ThejsonSafeClonefunction serializesErrorinstances including their.stackproperty, which could leak internal file paths to remote clients. Consider omittingstackfrom the serialized output (just keepmessage), or only including it when averbose/debugflag is set on the connection. -
Protocol type duplication: The client package (
packages/client/protocol.ts) duplicates the server protocol types fromsrc/serve/protocol.ts. This is documented as intentional for independent publishing, which is a reasonable trade-off. Consider adding a CI check or test that validates the two stay in sync over time — protocol drift between server and client could cause subtle runtime issues. -
AsyncIterableQueueerror path: Inpackages/client/client.ts, whenerror()is called on the queue after items are already buffered, the buffered items will still be yielded before the error is thrown on the nextnext()call. This is likely fine for the current use case but worth noting — if the server sends events before an error, the consumer will see them all before the rejection.
Overall this is clean, well-tested code that follows the project's conventions (license headers, named exports, AnyOptions pattern, libswamp boundary, test co-location, both log/json output modes). Good security posture with the default loopback bind and non-loopback warning.
Summary
swamp servecommand — a WebSocket API server for remote workflow and model method execution@swamp/clientTypeScript package (published to JSR as@systeminit/swamp-lib) for consuming the APIconsole.log/console.errorfrom in-process extension models asmethod_outputevents in the event streamDetails
swamp serveStarts an HTTP+WebSocket server. Health check at
GET /health, WebSocket upgrade at/. Supports multiplexed concurrent requests with per-model locking and cancellation viacancelmessages.@swamp/clientStandalone TypeScript client with zero CLI dependencies:
Console capture
Extension models using
console.lognow have their output captured asmethod_outputevents, matching the behavior of out-of-process drivers. Previously, in-processconsole.logwent to the host process stdout and was not visible in the event stream.Test plan
swamp servestarts and responds to health checks@swamp/clientcallback and AsyncIterable APIs work end-to-endmethod_outputevents for in-process modelscancelmessage aborts running workflowsdeno task checkpasses🤖 Generated with Claude Code