Skip to content

feat: swamp open — local web UI for extensions, workflows, vaults#1173

Merged
stack72 merged 5 commits intomainfrom
feat/swamp-open
Apr 13, 2026
Merged

feat: swamp open — local web UI for extensions, workflows, vaults#1173
stack72 merged 5 commits intomainfrom
feat/swamp-open

Conversation

@johnrwatson
Copy link
Copy Markdown
Contributor

Summary

Adds swamp open — a top-level command (also registered as swamp serve open) that starts a localhost web UI on port 9191 for browsing and operating a swamp repository from a browser. It's a thin HTTP + SPA layer over libswamp: every operation goes through the same generators and deps the CLI already uses.

Everything renders in a themed dark UI inspired by the swamp-club /manual page — Orbitron + JetBrains Mono, neon green on black, cyber-border panels, scanline background.

Features

Repo picker — Boots even when run outside a swamp repo. Lists existing swamps found on disk, plus any paths in optional ~/.config/swamp/index.yaml. Per-repo metadata sidecar (.swamp/serve_open_meta.yaml) with name, description, tags for search.

Extensions tab — Sidebar of installed + registry extensions with client-side filtering. Uninstalled rows open a rich detail pane (models, methods, arguments, workflows, vaults, datastores, drivers, reports, skills, release notes) sourced from the swamp-club /latest endpoint when the user is logged in. Install via pullExtension with hot-reload of modelRegistry / vaultTypeRegistry / driverTypeRegistry / reportRegistry so new types appear without restarting. Multi-type extensions get a type picker. Model instances support create / edit (including rename that preserves the ID and history) / delete.

Workflows tab — Sidebar of all workflows, click one to see jobs + an inputs form. Run triggers an SSE stream rendered in a unified task follower: top meta strip, DAG job graph, per-job DAG of steps with topological column layering and rectilinear edges, expandable step rows with logs, errors, data artifacts, and reports. Click-to-center, scroll-preservation across live re-renders. Historical workflow runs and model-method runs both render through the same component via run-state mappers.

Reports tab — Lists registered reports (built-in + extension-provided) with scope and labels.

Vaults — Modal with a custom themed combobox (native <select> can't be reliably themed on macOS). Dynamic config form rendered per-provider from its Zod schema via zodToJsonSchema so AWS SM, 1Password, Azure KV etc. can be configured in-UI. Install additional vault providers from the registry inline. Every method/workflow input field has a 🔒 picker that fills the value with ${{ vault.get(...) }} so swamp's runtime expression evaluator resolves the secret server-side.

Auth — Header shows the logged-in swamp-club username via libswamp/auth.whoami.

Architecture notes

  • All imports in src/serve/open/http.ts go through src/libswamp/mod.ts per CLAUDE.md.
  • configureExtensionLoaders and configureExtensionAutoResolver extracted from src/cli/mod.ts so serve-open can reconfigure the global registries when the user picks a repo at runtime (the CLI bootstrap binds them to the launch cwd, which may not be the selected repo).
  • Favicon SVG is embedded as a string constant so the compiled binary can serve it without filesystem access.
  • UI has zero npm dependencies — single HTML file with inline CSS + vanilla JS, served as a string literal from src/serve/open/ui.ts. Fonts loaded from Google Fonts CDN.
  • All browser alert() dialogs replaced with a themed toast system.

New commands / endpoints

  • swamp open (alias: swamp serve open) — starts the server
  • HTTP endpoints (all GET unless noted): /api/repo/{status,discover,use,init,meta}, /api/fs/list, /api/extensions/{installed,search,:name,install}, /api/types[?prefix], /api/types/:name/describe, /api/definitions[?type] (+ POST, GET/:id, PUT/:id, DELETE/:id), /api/models/:name/{methods,methods/:m/{describe,run},history,outputs/:id}, /api/workflows[/:name][/describe][/run][/history][/runs/:id], /api/reports, /api/vaults (+ POST, POST /:name/keys), /api/vault-types[/registry][/:type/schema], /api/whoami, /favicon.svg.

Test plan

  • deno fmt --check passes
  • deno lint passes
  • deno run test — all 4305 tests pass
  • deno check passes
  • deno run compile produces a working binary
  • Manual smoke test: ./swamp open from a non-repo directory shows the picker, and from inside a repo loads the main UI
  • Manual smoke test: run a model method and see the task follower render live events
  • Manual smoke test: run a workflow (dag-showcase style) and see the DAG animate through state changes
  • Manual smoke test: create a local_encryption vault, add a key, use it in a method run

🤖 Generated with Claude Code

johnrwatson and others added 2 commits April 13, 2026 22:51
Adds a new `swamp open` top-level command (also registered as
`swamp serve open`) that starts a localhost web UI on port 9191 for
browsing and operating a swamp repository from a browser.

The command is a thin HTTP + SPA layer over libswamp — every operation
goes through the same generators and deps the CLI uses. Everything
renders in a themed dark UI inspired by the swamp-club /manual page
(Orbitron + JetBrains Mono, neon green on black, cyber-border panels,
scanline background).

Extensions tab
- Browse installed extensions and the swamp-club registry, with
  client-side filtering. Descriptions collapsed to latest version +
  relative publish date.
- Clicking an uninstalled extension renders a rich detail panel
  (version, author, labels, platforms, models, methods, arguments,
  workflows, vaults, datastores, drivers, reports, skills, release
  notes) pulled from the swamp-club `/latest` endpoint when auth is
  available.
- Install button calls `pullExtension` with force:true and hot-reloads
  `modelRegistry` / `vaultTypeRegistry` / `driverTypeRegistry` /
  `reportRegistry` so new types appear without restarting the server.
- After install, a type picker handles extensions that register multiple
  model types (e.g. `@swamp/gcp/storage/*`).
- Model instance management: create, edit (including rename, preserving
  ID + history), delete with inline error surface instead of alerts.

Run method / run workflow
- Unified SSE-based task follower ported from the swamp-club
  AdminMonitor: top meta strip, DAG job graph, per-job DAG of steps,
  expandable step rows with logs, errors, data artifacts, and reports.
- Topological column-layering layout with rectilinear edges, cyan
  number + Orbitron label section headers, corner-bracket selection.
- Historical runs (both method outputs and workflow runs) render in the
  exact same follower component via a run-state mapper.
- Click-to-center and scroll-position preservation across live
  re-renders so SSE updates don't jerk the viewport.

Vaults
- Modal with custom combobox type selector (native <select> can't be
  themed on macOS). Lists built-in + installed + registry-available
  vault providers with client-side search.
- Dynamic config form rendered from each provider's Zod schema via
  zodToJsonSchema so AWS SM, 1Password, etc. can be configured in-UI.
- Install additional vault providers from the extension registry
  inline.
- Vault value picker next to every method/workflow input field — fills
  the field with `${{ vault.get("vault", "key") }}` so swamp's runtime
  expression evaluator resolves secrets server-side.

Repo picker
- Browse the filesystem at startup with swamp marker detection.
- Existing swamps section with search and per-repo metadata sidecar
  (`.swamp/serve_open_meta.yaml` with name/description/tags).
- Create new swamp section with directory browser, dim-and-disable for
  directories that are already swamps.
- Optional user config at `~/.config/swamp/index.yaml` for adding
  explicitly-known repo paths outside the discover walk root.

Infrastructure
- `configureExtensionLoaders` and `configureExtensionAutoResolver`
  extracted from `src/cli/mod.ts` so serve-open can reconfigure the
  global registries when the user picks a repo at runtime.
- whoami badge in the header calls libswamp's auth.whoami — shows the
  swamp-club username when logged in.
- Favicon embedded as a string constant (compiled binary can't read
  sibling files from disk).
- Toast system replaces all browser alert() dialogs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`swamp open` is now a standalone top-level command, so the file and
the exported Command should reflect that rather than keeping the old
"serve open" subcommand name.

- rename src/cli/commands/serve_open.ts → src/cli/commands/open.ts
- export openCommand instead of serveOpenCommand
- drop `.command("open", serveOpenCommand)` from serveCommand — it's
  registered at the top level in cli/mod.ts and doesn't also need to
  live under `swamp serve`
- update log category from ["serve","open"] to ["open"]
- update example/description strings from "swamp serve open" to
  "swamp open"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions[bot]
github-actions bot previously approved these changes Apr 13, 2026
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. Command description undersells scope (src/cli/commands/open.ts, .description(...))
    Current: "Start a local web UI for browsing and running extensions"
    The UI also serves workflows, vaults, and reports. A user scanning swamp --help will miss these. Suggestion: "Start a local web UI for browsing extensions, workflows, vaults, and reports".

  2. Missing JSON shutdown events (src/cli/commands/open.ts, shutdown())
    The serve command emits { status: "stopping" } and { status: "stopped" } in JSON mode on SIGINT/SIGTERM. The open command's shutdown handler omits these. For scripts that wrap swamp open --json and watch stdout for lifecycle events, the shutdown is invisible. Low-impact since open is primarily interactive, but worth aligning with serve for consistency.

  3. Picker-mode behavior not discoverable from help text (src/cli/commands/open.ts)
    When run outside a swamp repo directory, the command silently starts in picker mode rather than failing — which is great UX. But there's no example or note in the help text that signals this capability. Adding .example("Browse without a repo", "swamp open") with a note, or a second example like "swamp open --repo-dir /path/to/repo", would help users discover the picker.

Verdict

PASS — no blocking issues. Flag names and defaults are consistent with serve. JSON startup output is well-structured. Error messages follow the existing { error: { message } } shape throughout the HTTP layer.

Three suggestions from the CLI UX review on #1173:

1. Broaden the command description from "browsing and running extensions"
   to cover workflows, vaults, and reports so `swamp --help` reflects
   the real scope.
2. Emit `{status: "stopping"}` / `{status: "stopped"}` on shutdown in
   JSON mode, matching `swamp serve` for lifecycle-watching scripts.
3. Add examples showing picker mode (run outside a repo) and the
   --repo-dir override so the picker is discoverable from --help.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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

  1. Import boundary violationsrc/serve/open/http.ts:68 imports zodToJsonSchema from ../../libswamp/types/schema_helpers.ts, an internal libswamp path. CLAUDE.md explicitly prohibits this: "CLI commands and presentation renderers must import libswamp types and functions from src/libswamp/mod.ts — never from internal module paths." The function is already exported from mod.ts (line 364), so moving the import to the existing ../../libswamp/mod.ts import block is a one-line fix.

  2. No unit tests for any new file — The PR adds 4 new source files (open.ts, http.ts, favicon.ts, ui.ts) with zero corresponding test files. Virtually every other CLI command (data_get_test.ts, serve_test.ts, model_create_test.ts, etc.) and serve module (connection_test.ts, protocol_test.ts, etc.) has tests. CLAUDE.md requires "Comprehensive unit test coverage" and "Unit tests live next to source files: foo.tsfoo_test.ts". At minimum, http.ts (the request handler with ~30 routes, input validation, and error handling) should have a test file covering the major endpoints.

  3. Missing AbortSignal with timeout on outbound fetchsrc/serve/open/http.ts:989 makes a fetch() call to an external server (/api/v1/extensions/.../latest) with no AbortSignal or timeout. CLAUDE.md: "For outbound network calls, pass an AbortSignal with a timeout so the caller controls cancellation." A hung upstream server would block the request handler indefinitely.

  4. Excessive any types — CLAUDE.md requires "TypeScript strict mode, no any types." The PR introduces 10 deno-lint-ignore no-explicit-any suppressions plus a type AnyOptions = any alias:

    • src/cli/commands/open.ts:58-59: type AnyOptions = any — other commands in this codebase type their options properly
    • src/serve/open/http.ts:742-744: Vault type registry lazy access via as any
    • src/serve/open/http.ts:1012: ...(info as any) spread on extension detail
    • src/serve/open/http.ts:1424-1497: Multiple as any casts in workflowRunSummary and handleWorkflowRunGet — these should use proper type interfaces for workflow run data
  5. Private field access via unchecked castsrc/cli/commands/open.ts:92-104 uses as unknown as Array<{ extensionsLoaded: boolean; extensionLoadPromise: Promise<void> | null }> to reset private registry state. This silently breaks if those fields are ever renamed or refactored. Consider adding a public reset() or reload() method to the registry base class instead.

Suggestions

  1. The PR description says the command is "also registered as swamp serve open" but the code only registers it as a top-level swamp openopenCommand is not added as a subcommand of serveCommand.

  2. DDD layer violation — Several handlers bypass the application layer (libswamp generators) and directly access domain repositories, e.g. handleWorkflowGet (line 1389, with a comment explaining why), handleHistory (line 1298), handleTypeList (line 926). For a POC this is understandable, but the missing data should ideally be exposed through the libswamp application layer so the presentation layer doesn't couple to domain internals.

  3. Filesystem exposurehandleFsList and handleRepoDiscover expose the entire filesystem (directories, swamp detection) to any process on localhost. The server binds to 127.0.0.1 which limits the attack surface, but consider documenting this or adding a configurable root boundary.

  4. The router in handleOpenRequest (~300 lines of chained if/else with regex matches) is hard to extend. A table-driven or pattern-matching approach would improve readability — but this is a style preference, not a blocker.

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. Missing --host option (src/cli/commands/open.ts): swamp serve exposes --host to bind to a custom interface; swamp open hardcodes 127.0.0.1. This is a reasonable security-first default (the web UI has full repo access), but it would be worth documenting explicitly in the command description or a note that it is intentionally localhost-only, so users don't go hunting for the flag.

  2. Listening log phrasing: swamp serve prints WebSocket API server listening on {host}:{port}; swamp open prints swamp open listening on {url}. The URL form is actually better for a browser-targeted command (clickable in most terminals), so no change needed — just noting the minor divergence.

  3. swamp serve open alias: The PR description claims the command is also reachable as swamp serve open, but the diff only registers it as a top-level open command under the main CLI (src/cli/mod.ts). The serve command itself was not changed. Either the alias was accidentally omitted, or the description is aspirational. Either way, swamp open works and the help text is accurate to what's actually registered.

Verdict

PASS — help text, flag names, JSON output shape, and error handling are all consistent with the existing swamp serve command and the wider CLI conventions.

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

  1. libswamp import boundary violationsrc/serve/open/http.ts:68 imports zodToJsonSchema from the internal path ../../libswamp/types/schema_helpers.ts. Per CLAUDE.md, non-libswamp code must import from src/libswamp/mod.ts only. The function is already re-exported from the barrel (src/libswamp/mod.ts:364). Fix: move the import to the existing ../../libswamp/mod.ts import block.

  2. Unrelated regression: Promise detection removed — This PR silently removes the Issue #88 Promise-detection safety guard from cel_evaluator.ts (the result instanceof Promise check + InvalidExpressionError) and the corresponding UserError wrapper in execution_service.ts (expandForEachSteps), plus deletes ~140 lines of test coverage across cel_evaluator_test.ts and execution_service_test.ts. These changes are unrelated to the swamp open feature, introduce a behavioral regression (async CEL functions in forEach.in will silently produce {} instead of a clear error), and are not mentioned in the PR description. They should be reverted from this PR and, if intentional, submitted as a separate PR with justification.

  3. Missing AbortSignal on outbound fetchsrc/serve/open/http.ts:989 calls fetch() to the swamp-club /latest endpoint without an AbortSignal timeout. CLAUDE.md requires: "For outbound network calls, pass an AbortSignal with a timeout so the caller controls cancellation." Add signal: AbortSignal.timeout(10_000) (or similar) to the fetch options.

  4. No tests for ~5,500 lines of new server code — The PR adds src/serve/open/http.ts (1820 lines), src/cli/commands/open.ts (308 lines), and src/serve/open/favicon.ts (90 lines) with zero test files. CLAUDE.md requires "comprehensive unit test coverage" with "unit tests live next to source files." The existing src/serve/ directory has test files (connection_test.ts, webhook_test.ts, etc.) and src/cli/commands/ has many *_test.ts files. At minimum, src/serve/open/http_test.ts should cover the route dispatch, error responses, and key handler behaviors (e.g., handleFsList path validation, requireRepo guard, definition CRUD).

  5. Unsafe private-field cast in reloadExtensionRegistriessrc/cli/commands/open.ts:92-104 casts four registries through as unknown as Array<{ extensionsLoaded: boolean; extensionLoadPromise: Promise<void> | null }> to reset private fields. This bypasses type safety entirely and will silently break if the internal field names are ever renamed. Consider adding a public reset() or reload() method to the registry base class instead.

Suggestions

  1. as any casts in workflow/vault handlershttp.ts has 8+ deno-lint-ignore no-explicit-any / as any casts for workflow runs and vault types (lines 742-745, 1424-1497). While the AnyOptions pattern is established in CLI commands, these handler-level casts indicate missing type exports from the domain layer. Consider defining proper types for WorkflowRun and its nested jobs/steps structures to eliminate the casts, or at minimum adding // TODO markers for follow-up.

  2. No CORS protection — The HTTP server has no Access-Control-Allow-Origin or CSRF protection. While binding to 127.0.0.1 limits network access, any website the user visits can make cross-origin requests to http://127.0.0.1:9191/api/* (browsers allow cross-origin fetch with no CORS preflight for simple requests). This could allow a malicious page to trigger model runs, install extensions, or browse the filesystem. Consider adding an Origin check or requiring a secret token.

  3. Filesystem browser scopehandleFsList accepts any absolute path via the path query parameter. While the server is localhost-only, a defense-in-depth approach could restrict browsing to $HOME and below, or require explicit opt-in for paths outside it.

  4. handleRepoMetaPut writes to arbitrary repo paths — The PUT /api/repo/meta handler writes a YAML sidecar file to any path that passes the markerRepo.exists check. Input validation confirms it's a swamp repo, which is good, but the body fields (name, description, tags) are written without length limits.

  5. Consider extracting route dispatch — The handleOpenRequest function is a single 400+ line if/else chain of regex matches. A table-driven router (even a simple Map<string, handler> + regex fallback) would improve readability and make it easier to add endpoints.

johnrwatson and others added 2 commits April 13, 2026 23:09
Five blocking items from the automated code review:

1. Import boundary violation — move `zodToJsonSchema` from the internal
   `libswamp/types/schema_helpers.ts` path to the public `libswamp/mod.ts`
   export, as required by CLAUDE.md.

2. Missing AbortSignal on outbound fetch — the swamp-club
   `/extensions/:name/latest` call in `handleExtensionDetail` now passes
   `AbortSignal.timeout(10_000)` so a hung upstream can't block the
   request handler.

3. Excessive `any` types — removed the `as any` casts around:
   - `vaultTypeRegistry.getAllLazy()` (it's already a public method)
   - `workflowRunSummary` / `handleWorkflowRunGet` (now typed with the
     domain `WorkflowRun`, `JobRun`, `StepRun` interfaces)
   - extension detail response spread (now uses `as unknown as
     Record<string, unknown>` with a comment explaining why the
     client's declared type is narrower than the actual payload)

4. Private-field access via unchecked cast — add public
   `resetLoadedFlag()` methods to `ModelRegistry`, `VaultTypeRegistry`,
   `DriverTypeRegistry`, and `ReportRegistry` so `swamp open` can
   re-run the extension loaders without casting through `unknown`.
   Update `open.ts` to use the new method.

5. No unit tests for new source files — add `src/serve/open/http_test.ts`
   covering the HTTP router's core behaviour: HTML/favicon serving,
   repo-status reporting, 412 guard on repo-gated endpoints, 404 for
   unknown routes, filesystem listing, and meta PUT validation. Plus
   a `ModelRegistry.resetLoadedFlag` test in model_test.ts.

4313 tests pass (8 new).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Follow-up from review feedback on #1173:

- Add an `originAllowed()` check in `handleOpenRequest` that rejects any
  request whose `Origin` header doesn't match the server's own host with
  a 403. Requests with no Origin (direct navigations, favicon fetches)
  still pass. Defense-in-depth against DNS rebinding and cross-origin
  `fetch` from a malicious page in the user's browser — the server
  already binds to 127.0.0.1, this adds another layer specifically for
  mutating and data-leaking routes.

- Bound `handleRepoMetaPut` input lengths: name 200 chars, description
  2000 chars, tags capped at 64 entries × 64 chars each, so a runaway
  client can't write a megabyte-scale YAML sidecar.

- Two new tests covering the cross-origin rejection and the
  same-origin-with-Origin-header pass-through.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@johnrwatson
Copy link
Copy Markdown
Contributor Author

Thanks for the review — quick response to each remaining item on commit 3a74e6c:

Blocking

  1. Import boundary — fixed in 01764cc (zodToJsonSchema now imported from libswamp/mod.ts).
  2. "Unrelated regression: Promise detection removed" — this is a false positive. This branch has zero changes to src/domain/expressions/cel_evaluator.ts, src/domain/workflows/execution_service.ts, cel_evaluator_test.ts, or execution_service_test.ts. Verified with:
    $ git diff main...HEAD -- src/domain/expressions/cel_evaluator.ts src/domain/workflows/execution_service.ts
    (empty)
    $ git log main..HEAD --oneline -- src/domain/expressions src/domain/workflows/execution_service.ts
    (empty)
    
    No fix needed — please re-run on the latest HEAD.
  3. Outbound fetch timeout — fixed in 01764cc with signal: AbortSignal.timeout(10_000).
  4. Tests for new server code — fixed in 01764cc (src/serve/open/http_test.ts covering HTML serving, favicon, repo status, 412/404/403 guards, filesystem listing, meta PUT validation) + a new ModelRegistry.resetLoadedFlag test in model_test.ts. This push (4bd9059) adds two more CORS guard tests.
  5. Private-field cast on registries — fixed in 01764cc with public resetLoadedFlag() methods on ModelRegistry, VaultTypeRegistry, DriverTypeRegistry, and ReportRegistry.

Suggestions

  1. as any casts in workflow/vault handlers — fixed in 01764cc. Workflow handlers now typed with WorkflowRun/JobRun/StepRun from the domain layer; the vault lazy-types cast dropped (method is already public). The remaining AnyOptions alias is the established Cliffy workaround pattern used by every other command in the repo.
  2. CORS / Origin protection — addressed in this push (4bd9059). handleOpenRequest now rejects any request whose Origin header doesn't match the server host with a 403.
  3. Filesystem browser scope — left as-is for the POC. The server is localhost-only, the Origin guard blocks cross-origin exploitation, and restricting to $HOME would break the existing picker-mode flow where users point swamp open at repos on external volumes.
  4. handleRepoMetaPut length limits — addressed in this push (4bd9059). Name capped at 200 chars, description at 2000, tags at 64 × 64.
  5. Route dispatch extraction — left as a follow-up. Style preference, not a behavioral change.

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. Picker-mode log message exposes raw error: open.ts:243 — the info message
    No initialized repository found — starting in picker mode (${e.message}) embeds
    the raw error string from requireInitializedRepoUnlocked. If that error is
    a technical message (e.g. a path or lock error), it adds noise. Consider omitting
    the parenthetical or replacing with a friendlier hint: (run \swamp init` to
    create a repo)`.

  2. "Loaded repository" log fires even in silent flows: open.ts:240
    ctx.logger.info\Loaded repository at ${state.repoDir}`` uses the tagged-template
    form while the listening message (line 264) uses the string-interpolation form.
    Minor style inconsistency with the same file; not user-visible.

  3. Logger namespace vs. command path mismatch: open.ts:61 registers the logger
    under ["serve", "open"], but the command is swamp open (not swamp serve open).
    Users filtering logs by namespace would see serve.open prefixed entries for a
    top-level command. Consider ["open"] to match the command tree.

  4. open is not in NON_REPO_COMMANDS: When the user runs swamp open from
    outside a repo, the CLI still runs extension-loader setup against the cwd before
    the command action starts. If the cwd has any swamp-adjacent state it could emit
    stale deferred warnings before the picker appears. Low probability but worth
    adding "open" to NON_REPO_COMMANDS (the command reconfigures loaders itself
    once a repo is picked).

Verdict

PASS — help text is clear, examples cover the main cases, JSON output includes
the URL on startup, --no-open follows the existing --no-X convention, and
log/json parity is correct. No blocking issues.

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. Picker-mode log message exposes raw error: open.ts:243 — the info message No initialized repository found — starting in picker mode (${e.message}) embeds the raw error string from requireInitializedRepoUnlocked. If that error is a technical message (e.g. a path or lock error), it adds noise. Consider omitting the parenthetical or replacing with a friendlier hint like (run swamp init to create a repo).

  2. Logger namespace vs. command path: open.ts:61 registers the logger under ["serve", "open"], but the command is swamp open (not swamp serve open). Users filtering logs by namespace would see serve.open prefixed entries for a top-level command. Consider ["open"] to match the command tree.

  3. open not in NON_REPO_COMMANDS: When run outside a repo, the CLI still runs extension-loader setup against cwd before the action starts. The command reconfigures loaders itself after the user picks a repo, so adding "open" to NON_REPO_COMMANDS would avoid any spurious deferred warnings during the initial setup.

  4. stopped emitted before server actually finishes: open.ts:288{ status: "stopped" } is emitted immediately after ac.abort() but before await server.finished resolves, so a script watching JSON output may assume the port is free before it is. This mirrors the same pattern in serve.ts, so it is consistent — just worth noting if either is fixed in the future.

Verdict

PASS — help text is clear, examples cover the main cases, JSON output includes the URL on startup, --no-open follows the existing --no-X convention, and log/JSON parity is correct. No blocking issues.

@stack72 stack72 merged commit 0900d93 into main Apr 13, 2026
9 checks passed
@stack72 stack72 deleted the feat/swamp-open branch April 13, 2026 22:16
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. Picker-mode example uses a shell side-effect (open.ts:129): The example cd /tmp && swamp open is the only one in the file that embeds a shell navigation step. All other examples are pure invocations. Consider replacing with a comment-style note or dropping the cd — the --repo-dir example already shows how to target a non-cwd path, and the description covers picker mode via context.

  2. Log-mode "listening" message omits port (open.ts:264): swamp serve logs listening on {host}:{port}, while swamp open logs the full URL swamp open listening on {url}. For a browser-based UI the full URL is actually more useful (clickable in most terminals), so this is intentional and fine — just noting the deliberate asymmetry.

  3. stopped JSON emitted synchronously before server drains (open.ts:288): The { status: "stopped" } event fires immediately after ac.abort(), before server.finished resolves. swamp serve has the same pattern, so it's consistent — but both could mislead a script that treats stopped as "server has fully drained". Not a regression; just calling it out.

Verdict

PASS — JSON and log modes are both properly supported, flag names are consistent with swamp serve conventions, help text is comprehensive, and picker-mode fallback is clearly communicated.

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

Well-structured PR that adds a local web UI for swamp. The architecture is sound: HTTP routing delegates to libswamp application services for mutations, reads directly from domain repositories, and the UI is a self-contained HTML/JS file with no external dependencies (beyond CDN fonts). Good security posture with origin checking, 127.0.0.1 binding, escapeHtml for XSS prevention, input size bounds, and AbortSignal on the outbound fetch.

Blocking Issues

None.

Suggestions

  1. as unknown as cast in handleWorkflowList (http.ts ~line 1425): The workflowRepo.findAll() as unknown as Promise<...> double-cast papers over a type mismatch between the repo return type and WorkflowSearchDeps. Consider creating a thin mapping adapter instead, which would catch type drift at compile time.

  2. Missing resetLoadedFlag tests for 3 of 4 registries: The test in model_test.ts covers ModelRegistry.resetLoadedFlag, but VaultTypeRegistry, DriverTypeRegistry, and ReportRegistry all gain the same method with no corresponding tests. The implementations are identical boilerplate, so this is low-risk, but adding matching tests would be consistent with the project's "comprehensive unit test coverage" requirement in CLAUDE.md.

  3. serve open alias not registered: The PR description mentions swamp serve open as an alias, but the diff only registers openCommand at the top level (.command("open", openCommand)). If the alias is intended, it needs to also be registered as a subcommand of serve. If not, the PR description should be updated.

  4. Route ordering clarity (http.ts ~line 257): The literal route /api/vault-types/registry appears after the regex /api/vault-types/(.+)/schema. While this works correctly (the regex requires /schema suffix), placing literal routes before regex routes makes precedence immediately obvious and guards against future regex changes that could accidentally swallow the literal.

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.

Adversarial Review

Critical / High

None found.

Medium

  1. handleDefinitionUpdate accepts arrays for globalArguments (http.ts:1126)
    The validation if (!body.globalArguments || typeof body.globalArguments !== "object") passes arrays, because typeof [] === "object" and ![] === false. If a client sends {"globalArguments": [1, 2, 3]}, then Object.entries([1,2,3]) produces [["0",1],["1",2],["2",3]] and the definition ends up with numeric string keys "0", "1", "2" as global arguments. The definition is saved with nonsensical arguments.
    Breaking input: PUT /api/definitions/myModel with body {"globalArguments": [1,2,3]}
    Suggested fix: Add || Array.isArray(body.globalArguments) to the guard, e.g.:

    if (!body.globalArguments || typeof body.globalArguments !== "object" || Array.isArray(body.globalArguments)) {
  2. Race condition: concurrent loadRepoIntoState can leave global registries misconfigured (open.ts:97-119)
    loadRepoIntoState has multiple await points: requireInitializedRepoUnlocked, configureExtensionLoaders, reloadExtensionRegistries. Between these awaits, other HTTP requests continue to be served. If two /api/repo/use calls arrive in quick succession (e.g., user double-clicks), call A might set state.repoDir to repo-A then await the loader, during which call B sets state.repoDir to repo-B and overwrites the loaders. Call A's reloadExtensionRegistries then loads extensions for repo-B's loaders, or vice versa — the final state is a mix of both repos. In practice this is a single-user localhost tool so the window is narrow, but it could produce confusing "wrong extensions" if triggered.
    Suggested fix: Add a mutex or sequencing guard around loadRepoIntoState so concurrent calls are serialized (e.g., store a Promise and chain subsequent calls onto it).

Low

  1. Malformed JSON bodies return 500 instead of 400 (http.ts, multiple handlers)
    Handlers like handleRepoInit (line 733), handleVaultCreate (line 902), handleRun (line 1735), handleWorkflowRun (line 1571), etc. call await req.json() without a local try/catch. If the client sends non-JSON (or an empty body), the thrown SyntaxError is caught by the top-level catch in handleOpenRequest (line 467) which returns HTTP 500. A 400 would be semantically correct. Low impact since the error IS handled — just with the wrong status code.

  2. handleRepoDiscover can follow symlink cycles (http.ts:684-718)
    The walk function tracks depth (maxDepth=4) and skips dotfiles, but uses Deno.readDir which follows symlinks by default. A symlink cycle (e.g., a/b -> a) combined with different resolved paths could escape the depth guard and cause the walk to loop until the depth check stops it. The seenPaths set deduplicates swamp markers but not the walk itself. Theoretical concern — maxDepth=4 makes this self-limiting, and it's a localhost tool browsing the user's own filesystem.

  3. handleWorkflowRun does not acquire model locks (http.ts:1566-1631)
    Unlike handleRun (line 1748) which calls acquireModelLocks, the workflow run handler does not acquire any locks before executing. If the WorkflowExecutionService handles locking internally, this is fine. If not, concurrent workflow+method runs could conflict on shared model state. I didn't trace far enough into the execution service to confirm.

Verdict

PASS — The code is well-structured with consistent XSS escaping in the UI, proper Origin checking, bounded filesystem operations, and correct error handling patterns. The medium issues are real but non-blocking: the array validation gap is a minor input validation hole, and the race condition requires deliberate double-clicking with effectively no data corruption risk.

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.

2 participants