Skip to content

feat(cli): auto-detect piped stdin for method run and workflow run#1402

Merged
stack72 merged 2 commits into
mainfrom
worktree-336
May 19, 2026
Merged

feat(cli): auto-detect piped stdin for method run and workflow run#1402
stack72 merged 2 commits into
mainfrom
worktree-336

Conversation

@stack72
Copy link
Copy Markdown
Contributor

@stack72 stack72 commented May 19, 2026

Summary

Closes swamp-club#336.

  • Auto-detect piped stdin in method run and workflow run — no flag needed, consistent with vault put, model edit, and workflow edit
  • parseStdinContent() detects JSON objects, JSON arrays, NDJSON (one JSON per line), and YAML
  • Multiple items (array or NDJSON) produce one discrete run per item with [i/N] progress logging
  • --input key=value overrides are deep-merged onto each stdin item (overrides win)
  • Piped stdin and --input-file are mutually exclusive (clear error)
  • Updated design/inputs.md, swamp-model skill, and swamp-workflow skill with stdin docs and pipe composition examples

Test Plan

  • 12 unit tests for parseStdinContent() covering all format detection paths (JSON object, JSON array, NDJSON, YAML, empty, malformed, non-object elements)
  • 15 end-to-end tests against compiled binary in scratch repo:
    1. Single JSON object piped to method run
    2. NDJSON iteration (3 items → 3 discrete runs)
    3. Full pipe: data query --json | jq | method run
    4. Normal --input without pipe (regression)
    5. Deep merge: stdin + --input override
    6. Piped stdin + --input-file mutual exclusivity error
    7. JSON array (2 items → 2 runs)
    8. Single JSON piped to workflow run
    9. NDJSON iteration on workflow run (2 items → 2 runs)
    10. Workflow piped stdin + --input-file error
    11. Normal workflow --input without pipe (regression)
    12. Malformed stdin (clear parse error)
    13. Empty stdin (clear error message)
    14. YAML from stdin
    15. NDJSON + --input overrides applied to every item
  • Full test suite: 5998 passed, 0 failed

🤖 Generated with Claude Code

…wamp-club#336)

Enable Unix pipe composition by auto-detecting piped stdin in method run
and workflow run. Supports JSON objects, JSON arrays, NDJSON (one JSON
per line), and YAML. Multiple items produce one discrete run per item
with progress logging. --input overrides are deep-merged onto each stdin
item.

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.

CLI UX Review

Blocking

  1. No --help examples for stdin in either commandmodel_method_run.ts and workflow_run.ts received no new .example() calls showing stdin usage. Running swamp model method run --help or swamp workflow run --help gives zero indication that piping JSON/NDJSON/YAML is supported. The PR explicitly cites vault put as the design model, but vault put documents stdin prominently: once in the .description() body ("Piped via stdin: echo "val" | swamp vault put ") and again as a dedicated .example(\"Pipe from stdin (recommended for scripts)\", ...). Without matching treatment here, the feature is invisible to users who discover the CLI through its own help text.

    Suggested fix for model_method_run.ts (mirror pattern in workflow_run.ts):

    .example(
      "Pipe inputs from stdin",
      'echo \'{"env":"prod"}\' | swamp model method run my-server deploy',
    )
    .example(
      "Batch run via NDJSON from stdin",
      'printf \'{"env":"dev"}\\n{"env":"prod"}\' | swamp model method run my-server deploy',
    )

Suggestions

  1. --input-file option description not updated in command definitions — The SKILL.md was updated to say "Input values from YAML file (cannot combine with piped stdin)", but both command files still register the option as "Input values from YAML file". The conflict error message is clear when it fires, but --help won't surface the constraint. Low priority since the error message is actionable.

  2. JSON mode with multiple items emits multiple JSON objects — When piping NDJSON or a JSON array in --json mode, each run emits its own JSON object to stdout (one per iteration). This is technically valid NDJSON output, but a user expecting a single JSON array of all results (common expectation for --json flags) may be surprised. Worth documenting or considering a JSON array wrapper for multi-item runs.

Verdict

NEEDS CHANGES — the stdin feature is well-designed and the error messages are clear, but --help gives users no hint it exists. Adding examples to both command definitions would make the feature discoverable and complete the consistency with vault put.

github-actions[bot]
github-actions Bot previously approved these changes May 19, 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.

Code Review

Blocking Issues

None.

Suggestions

  1. Lock release inconsistency before Deno.exit(1) — In workflow_run.ts:410-411, locks are explicitly flushed before Deno.exit(1) inside the iteration loop (if (flushModelLocks) await flushModelLocks()), but model_method_run.ts:379 calls Deno.exit(1) without flushing. Deno.exit() bypasses finally blocks, so the lock cleanup at line 389 won't run. Consider adding the same pre-exit flush for consistency:

    if (renderer.runFailed()) {
      if (flushModelLocks) await flushModelLocks();
      Deno.exit(1);
    }
  2. Multi-line docstring on parseStdinContent (input_parser.ts:327-336) — CLAUDE.md says "one short line max" for comments. The format detection order is non-obvious behavior worth documenting, but the 10-line block could be condensed to a single line like // Detects JSON object/array, NDJSON, or YAML; returns one record per item. with the detailed order discoverable from the code itself.

What looks good

  • Clean separation: stdin parsing stays in the CLI layer, not domain — correct DDD placement
  • All libswamp imports go through mod.ts — import boundary respected
  • UserError used consistently for all user-facing error paths
  • Thorough unit test coverage (12 tests) for parseStdinContent covering all format paths and edge cases
  • NDJSON blank-line filtering, empty-array rejection, and non-object-element validation are solid
  • deepMerge reuse (already tested) for --input overrides onto stdin items is clean
  • LogTape tagged template usage follows bare-interpolation convention
  • Design doc, skill docs, and scenarios all updated consistently

Add --help examples for stdin piping to both method run and workflow
run. Flush model locks before Deno.exit(1) in method run iteration
loop (Deno.exit bypasses finally blocks). Update --input-file
description to note stdin mutual exclusivity. Condense parseStdinContent
docstring to one line per CLAUDE.md conventions.

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.

CLI UX Review

Blocking

None.

Suggestions

  1. YAML stdin not shown in help examples (model_method_run.ts, workflow_run.ts .example() blocks)
    The feature supports JSON, JSON array, NDJSON, and YAML — but only JSON and NDJSON appear in --help examples. A short third example would surface YAML discoverability:

    printf 'environment: prod\nreplicas: 3' | swamp model method run my-server deploy
    
  2. --input override semantics not hinted in flag description (model_method_run.ts:113, workflow_run.ts:111)
    The description "Input values (key=value or JSON)" doesn't mention that when combined with piped stdin, --input values win on conflict. Users who pipe JSON and also pass --input have no help-text signal about precedence. A short addendum — e.g., "…; overrides piped stdin values" — would make the behavior self-documenting without reading examples.

  3. Batch --json mode produces NDJSON, not a single JSON document (undocumented)
    When multiple items are piped and --json is set, each run emits its own JSON object — jq . would fail, jq -s . or NDJSON-aware consumers are needed. This is the right design for pipe composition, but it's surprising if discovered accidentally. A single note in design/inputs.md (e.g., "In --json mode, each item produces one JSON object — output is NDJSON-shaped") would prevent confusion.

Verdict

PASS — feature is well-executed. Help text discoverability is good (examples cover the two most common cases), error messages are clear and actionable ("No input received from stdin. Pipe JSON, NDJSON, or YAML into the command." is excellent), progress logging [i/N] only appears in batch mode, exit-code and lock-flush handling is correct in both commands. Consistent with the existing vault put / model edit / workflow edit stdin pattern.

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

Clean, well-structured feature that adds piped stdin support to method run and workflow run. The implementation follows established patterns (readStdin from vault_put, model_edit, workflow_edit) and maintains proper layered architecture — stdin I/O in infrastructure, parsing in CLI layer, domain unchanged.

Blocking Issues

None.

Suggestions

  1. Duplicate wiring logic — The stdin-detect → parse → merge → iterate pattern in model_method_run.ts and workflow_run.ts is nearly identical (~25 lines each). Extracting a shared helper (e.g., buildInputSets(stdinContent, cliInputs, inputFile)) would reduce duplication, but the current approach is readable and the blast radius of a refactor may not be worth it right now.

  2. Single-line invalid JSON falls through to YAML — If a user pipes {broken json (single line, fails JSON.parse), the NDJSON path is skipped (lines.length is 1, not > 1), and YAML parsing is attempted. The final error message is still correct ("Failed to parse stdin content…"), but a user intending to send JSON might find the "Expected JSON object, JSON array, NDJSON, or YAML" message slightly confusing. Low priority — the error is still actionable.

  3. No committed integration tests for the stdin piping flow — The PR body describes 15 e2e scenarios which is great, but they don't appear in the diff. The unit tests for parseStdinContent (12 tests) cover the parsing logic thoroughly, so this isn't blocking. Adding a few integration tests in integration/ for the pipe composition flow would increase confidence for future regressions.

DDD compliance: Good. Stdin handling stays in the CLI/infrastructure layers. Domain services (modelMethodRun, workflowRun) continue to accept plain inputs: Record<string, unknown> — no domain boundary changes.

libswamp import boundary: Compliant. Both commands import parseTags from ../../libswamp/mod.ts, no internal path imports.

CLAUDE.md compliance: License headers present, named exports used, no any types, parseStdinContent tests live next to source (input_parser_test.ts), LogTape tagged templates use bare interpolation.

@stack72 stack72 merged commit 383721f into main May 19, 2026
11 checks passed
@stack72 stack72 deleted the worktree-336 branch May 19, 2026 19:55
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