Skip to content

ci: add JSR publish workflow for @systeminit/swamp-lib#893

Closed
johnrwatson wants to merge 9 commits intomainfrom
feat/ci-checks-in-swamp
Closed

ci: add JSR publish workflow for @systeminit/swamp-lib#893
johnrwatson wants to merge 9 commits intomainfrom
feat/ci-checks-in-swamp

Conversation

@johnrwatson
Copy link
Copy Markdown
Contributor

Summary

Adds a GitHub Actions workflow that publishes the @systeminit/swamp-lib client package to JSR when files in packages/client/ change on merge to main.

  • Uses Deno's OIDC token auth (no secrets needed — JSR trusts GitHub Actions)
  • Auto-generates semver from date + commit count: 0.YYYYMMDD.N
  • Runs type check + tests before publishing
  • Also supports workflow_dispatch for manual publishing

Test plan

  • Workflow triggers on merge with packages/client/ changes
  • Version stamp produces valid semver
  • deno publish succeeds with OIDC auth

🤖 Generated with Claude Code

johnrwatson and others added 9 commits March 26, 2026 23:56
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>
…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>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…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>
Publishes packages/client to JSR on merge to main when files in
packages/client/ change. Uses OIDC token auth (no secrets needed).
Version is auto-generated from date + commit count.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@johnrwatson johnrwatson marked this pull request as draft March 27, 2026 18:50
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI Security Review

Critical / High

None.

Medium

  1. .github/workflows/publish-client.yml:32,36actions/checkout@v4 and denoland/setup-deno@v2 are pinned by tag, not full commit SHA. A compromised tag could inject malicious code into the publish pipeline. Both are from trusted publishers (GitHub and Deno), which lowers the risk. Suggested fix: Pin to full SHAs (e.g., actions/checkout@<sha>) for defense-in-depth, or accept the risk given the trusted publishers.

Low

  1. .github/workflows/publish-client.yml:18-19 — Permissions are declared at workflow level rather than job level. Since there is only one job, this has no practical security impact, but job-level scoping is the recommended pattern for maintainability as jobs are added.

Verdict

PASS — No critical or high severity findings. The workflow uses safe triggers (pull_request on closed, not pull_request_target), checks out main (not PR HEAD), has no LLM prompt injection surface, uses OIDC for secretless JSR publishing, and interpolates only self-generated data. The medium finding on SHA pinning is a hardening recommendation, not a blocking issue.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI UX Review

Blocking

None.

Suggestions

  1. serve.ts:113logger.info("Shutting down...") runs in both log and JSON modes (no else). In JSON mode this likely goes to stderr via LogTape so it's harmless, but adding else would make the intent explicit and match the pattern used in the onListen block above.

  2. serve.ts:53-56 — The non-loopback security warning uses logger.warn which in JSON mode goes to stderr rather than stdout. Consider also emitting a JSON warning object to stdout (e.g. {"warning": "binding to non-loopback address, no auth enforced", "host": ...}) so scripted consumers can detect it.

Verdict

PASS — serve command follows existing flag conventions (--repo-dir, consistent with other commands), both log and JSON output modes are properly handled, defaults are sensible (port 9090, host 127.0.0.1), and help text is clear.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Blocking Issues

None.

Suggestions

  1. Layering: src/serve/connection.ts:44 imports from ../cli/repo_context.ts — The src/serve/ module imports acquireModelLocks from the CLI layer. From a DDD perspective, src/serve/ is an application-level boundary (like src/cli/) and should not depend on sibling application layers. Consider extracting acquireModelLocks into src/domain/ or src/infrastructure/ so both src/cli/ and src/serve/ can depend on it without circular layering. Not blocking since it's functional and the function has no CLI-specific dependencies, but it creates an unfortunate dependency direction.

  2. no-explicit-any count in packages/client/client.ts — There are 4 deno-lint-ignore no-explicit-any directives in client.ts. These are understandable given the generic PendingRequest pattern, but the event.error as any cast at line 358 could be tightened to as { code?: string; message?: string; details?: unknown } to avoid the suppression.

  3. packages/client/deno.json missing imports for @std/assert — The test files import @std/assert which resolves through the workspace root. This works for development, but when the package is consumed standalone (or tests are run in the packages/client/ directory outside the workspace), the dependency won't resolve. Consider adding a devImports or imports entry if JSR supports it, or document that tests must be run from the workspace root.

  4. Protocol type duplicationpackages/client/protocol.ts and src/serve/protocol.ts define the same wire types independently. This is documented as intentional (zero CLI dependency for the published client), which is a reasonable tradeoff. Consider adding a comment in src/serve/protocol.ts noting the client-side mirror exists, so future changes to the server protocol prompt an update to the client types as well.

  5. src/serve/connection.ts:44acquireModelLocks import from ../cli/ — Already covered in suggestion 1, but worth noting this also means src/serve/ tests may transitively pull in CLI initialization code, which could make tests slower or more fragile.

Overall: Well-structured PR with good test coverage across all new modules. License headers present on all files. libswamp imports are correct (via mod.ts). The AnyOptions pattern follows existing codebase conventions. Security considerations are handled (loopback default, non-loopback warning). The WebSocket protocol design with multiplexed request IDs, cancellation support, and Zod validation is solid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant