Skip to content

feat: add -d @- and -d @file for Unix pipeline composability#3

Merged
turlockmike merged 3 commits intomasterfrom
feat/stdin-input
Apr 13, 2026
Merged

feat: add -d @- and -d @file for Unix pipeline composability#3
turlockmike merged 3 commits intomasterfrom
feat/stdin-input

Conversation

@bowlofarugula
Copy link
Copy Markdown

@bowlofarugula bowlofarugula commented Apr 11, 2026

Summary

  • Adds @- (stdin) and @path (file) support to the -d/--data flag, following the curl convention
  • Enables composing murl calls in Unix pipelines: murl $S/tools/search -d q=foo | jq '{id:.[0].text}' | murl $S/tools/get -d @-
  • Input must be a JSON object; arrays/scalars raise clear errors
  • Later -d flags override earlier ones, so @- can be combined with explicit key=value overrides

Test plan

  • @- reads JSON object from stdin
  • @path reads JSON object from file
  • @- merges correctly with explicit -d key=value flags (later wins)
  • Non-object JSON from stdin raises ValueError
  • Empty stdin raises ValueError
  • Missing file raises ValueError with clear message
  • All existing parse_data_flags tests still pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added ability to load JSON data from external files or standard input as part of the -d/--data flag
    • Data from files or stdin merges with explicitly provided flag values, with later values overriding earlier ones
  • Documentation

    • Enhanced help text and examples to document the new file and stdin input capabilities

Enables Unix pipeline composability — pipe JSON args from stdin or read
from a file, following the curl @ convention:

  echo '{"q":"foo"}' | murl $S/tools/search -d @-
  murl $S/tools/get -d @params.json

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 11, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d19a568e-19f0-46d5-a311-ae8da691565b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The changes add functionality to the CLI to load JSON data from external sources via the -d/--data flag. A new helper function _read_json_source is introduced to read and parse JSON from stdin (@-) or files (@path), with validation for non-empty content and dictionary-type values. The parse_data_flags function is updated to recognize -d entries starting with @, load the JSON, and merge it into the result with override behavior for duplicate keys. Error handling covers missing files, invalid JSON types, and empty input. CLI documentation is expanded with examples, and comprehensive tests verify the new functionality and error cases.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
tests/test_cli.py (1)

185-210: Add a negative test for non-object JSON from @path.

Great coverage overall. One gap remains: reject array/scalar JSON when source is a file (-d @file``), not just stdin.

Suggested test addition
+def test_parse_data_flags_file_non_object(tmp_path):
+    """@path with non-object JSON raises ValueError."""
+    f = tmp_path / "args.json"
+    f.write_text('[1, 2, 3]')
+    with pytest.raises(ValueError, match="must be an object, not list"):
+        parse_data_flags((f'@{f}',))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_cli.py` around lines 185 - 210, Add a negative test that mirrors
the stdin non-object case but reads from a file: create a temp file containing
non-object JSON (e.g., "[1,2,3]"), call parse_data_flags with the file path
prefixed by '@' (e.g., f'@{f}'), and assert it raises ValueError with the same
message used for non-object input (e.g., matching "must be an object, not
list"). Name the test similar to test_parse_data_flags_file_non_object so it
pairs with the existing test_parse_data_flags_file and ensures parse_data_flags
rejects arrays/scalars from files as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@murl/cli.py`:
- Line 395: The help text for the CLI option "-d, --data" contains a typo: the
stdin source token is written as "@->" but should be "@-". Update the help
string associated with the "-d, --data" option in murl/cli.py (the
usage/description for the Request data flag) to replace "@->" with "@-" so the
documentation and parser expectations match.
- Around line 122-123: The except blocks that currently do "except OSError as e:
raise ValueError(f'Cannot read {source}: {e}')" (and the analogous JSON parsing
handler) must preserve the original exception context by re-raising with "from
e" so the traceback retains the root cause; update both raises to "raise
ValueError(f'Cannot read {source}: {e}') from e" (and for the JSON handler,
"raise ValueError(f'Invalid JSON in {source}: {e}') from e") so the original
OSError/JSONDecodeError is chained into the new ValueError.

---

Nitpick comments:
In `@tests/test_cli.py`:
- Around line 185-210: Add a negative test that mirrors the stdin non-object
case but reads from a file: create a temp file containing non-object JSON (e.g.,
"[1,2,3]"), call parse_data_flags with the file path prefixed by '@' (e.g.,
f'@{f}'), and assert it raises ValueError with the same message used for
non-object input (e.g., matching "must be an object, not list"). Name the test
similar to test_parse_data_flags_file_non_object so it pairs with the existing
test_parse_data_flags_file and ensures parse_data_flags rejects arrays/scalars
from files as well.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9b70da9c-dbea-487b-b6de-b72aacaff5ca

📥 Commits

Reviewing files that changed from the base of the PR and between 42f9f80 and 1ae6973.

📒 Files selected for processing (2)
  • murl/cli.py
  • tests/test_cli.py

Comment thread murl/cli.py Outdated
Comment thread murl/cli.py Outdated
- Chain original exceptions with `raise ... from e` in _read_json_source
- Clarify -d help text to avoid @-> ambiguity

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bowlofarugula
Copy link
Copy Markdown
Author

Addressed both CodeRabbit findings in 4b5c454:

  1. raise ... from e — Both exception handlers in _read_json_source() now chain the original exception for proper tracebacks.
  2. Help text ambiguity — Changed -d, --data <key=value|JSON|@file|@-> to -d, --data <val> with formats listed in description text, avoiding the @-> visual ambiguity.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 11, 2026
Copy link
Copy Markdown

@turlockmike turlockmike left a comment

Choose a reason for hiding this comment

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

Two-lens review: Unix design, security.

Overall: Clean feature that follows the curl convention correctly. The _read_json_source extraction, JSON-object-only enforcement, and flag ordering semantics (later overrides earlier) are well done. A few items to address.

Key findings:

  1. @ detection precedence-d email=@user would be parsed as a @-source, not a key=value pair. Check for = first — if the token contains =, it's a key=value pair regardless of what comes after. See inline.

  2. Double stdin consumption — Passing -d @- -d @- silently fails on the second sys.stdin.read() (reads empty string). Detect multiple @- uses before the loop and error early. That's what curl does. See inline.

  3. Unbounded file readopen(path).read() with no size limit. If murl is composed into pipelines or CI that accepts user-supplied paths, -d @/dev/zero exhausts memory. A simple 10 MB ceiling closes this. See inline.

Additional observations:

  • No end-to-end test through CliRunner.invoke against the mcp_server fixture — all stdin tests exercise parse_data_flags directly. A test like runner.invoke(main, [url, "-d", "@-", "--no-auth"], input='{"message":"hi"}') would prove the feature works through the full CLI path.
  • The @path file-read behavior is worth documenting in the README so integrators who pipe user-controlled values into -d @<path> understand they're responsible for path sanitization.

What's good: The _read_json_source helper is the right extraction. Error messages are specific and actionable ("JSON from @- must be an object, not list"). The flag ordering semantics (later overrides earlier, so @- can combine with explicit overrides) mirrors curl correctly.

— Mike's AI Clone 🧬

Comment thread murl/cli.py
Comment thread murl/cli.py Outdated
Comment thread murl/cli.py
…d read

- Check for key=value before @-prefix so `-d email=@user` is treated as
  a literal value, not a file/stdin source
- Detect multiple @- flags upfront and raise a clear error instead of
  silently returning empty on second stdin read
- Cap file/stdin reads at 10 MB to prevent memory exhaustion from
  `-d @/dev/zero` or very large files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bowlofarugula
Copy link
Copy Markdown
Author

Addressed all three @turlockmike findings in 7d69ee6:

  1. @ detection orderparse_data_flags() now checks for = (key=value) before @ prefix, so -d email=@user correctly parses as {"email": "@user"} instead of trying to open a file.

  2. Double stdin consumption — An upfront scan detects multiple @- references and raises a clear error ("stdin (@-) can only be used once (it is consumed on first read)") before any reading happens.

  3. Unbounded file read — Added a 10 MB limit (_MAX_FILE_READ_BYTES). Both sys.stdin.read() and f.read() are capped, with a clear error if the input exceeds the limit.

Copy link
Copy Markdown

@turlockmike turlockmike left a comment

Choose a reason for hiding this comment

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

Follow-up review after fix commits 4b5c454 and 7d69ee6.

All three inline findings from the prior review are properly resolved:

  1. @ detection precedence ✅ — = check now runs before @ check, with explicit comment explaining the ordering.
  2. Double stdin consumption ✅ — Early guard counts @- occurrences and errors before any reads.
  3. Unbounded file read ✅ — 10 MB ceiling via _MAX_FILE_READ_BYTES with the classic read-one-over pattern.

Test coverage is solid: stdin reading, file reading, double-stdin error, size limit, non-object JSON, and empty input all covered.

Clean work addressing the feedback. Ship it.

— Mike's AI Clone 🧬

@turlockmike turlockmike merged commit bd264ca into master Apr 13, 2026
5 checks passed
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