Skip to content

feat: agent-friendly CLI surface (--json, --no-interactive)#61

Merged
kalil0321 merged 10 commits into
mainfrom
feat/agent-friendly-cli
May 5, 2026
Merged

feat: agent-friendly CLI surface (--json, --no-interactive)#61
kalil0321 merged 10 commits into
mainfrom
feat/agent-friendly-cli

Conversation

@kalil0321
Copy link
Copy Markdown
Owner

@kalil0321 kalil0321 commented May 5, 2026

Summary

  • Adds --json and --no-interactive to the agent command so other agents can drive reverse engineering without TTY prompts. --json redirects all Rich output to stderr and emits a single stable JSON object on stdout (schema_version, status, run_id, prompt, url, mode, har_path, script_path, usage, error).
  • Normalizes run_auto_capture return shape so the JSON payload is stable regardless of the underlying engineer SDK (Claude/OpenCode/Copilot).
  • list --json now emits [] on empty history; show <id> --json emits a structured {"error": ...} and exits 1 when the run is missing.
  • run gains --no-interactive (fail instead of opening the script-picker) and --auto-install (install missing deps without confirm).
  • Documents scripted usage and exit-code contract in the README; adds tests/test_cli_agent_json.py (10 new tests).

Branched from main so it stacks cleanly without depending on #60.

Example

```bash
reverse-api-engineer agent \
--prompt "capture the public jobs api" \
--url https://example.com/jobs \
--json | jq

{

"schema_version": 1,

"status": "ok",

"run_id": "deadbeef0001",

"har_path": "/.../recording.har",

"script_path": "/.../api_client.py",

...

}

```

Exit codes: `0` success, `1` runtime error, `2` misuse (missing required args).

Test plan

  • `uv run pytest tests/test_cli_agent_json.py` (10/10)
  • `uv run pytest` full suite (695 pass; 6 pre-existing failures reproduce on `main`)
  • `uv run ruff check src/reverse_api/cli.py tests/test_cli_agent_json.py` (only pre-existing complexity warnings remain)
  • Manual smoke: `agent --json` (no prompt) returns exit 2 + JSON error; `list --json` on empty history returns `[]` + exit 0
  • End-to-end smoke of an actual capture before merge

🤖 Generated with Claude Code


Summary by cubic

Makes the CLI fully scriptable and non-blocking for agents with --json/--no-interactive, adds agent --headless for CI/headless runs, and standardizes JSON payloads and exit codes across commands. Also prevents the REPL from hanging when stdin isn’t a TTY.

  • New Features

    • agent supports --json (single payload on stdout; logs to stderr), --no-interactive (fail fast on missing input), and --headless (runs MCP-controlled browser headless; disables --autoConnect for chrome-mcp).
    • engineer adds --json/--no-interactive with a stable JSON schema; fields: schema_version, status, run_id, prompt, fresh, script_path, usage, error.
    • Stable agent --json payload; fields: schema_version, status, run_id, prompt, url, mode, har_path, script_path, usage, error.
    • Non-interactive mode suppresses follow-up prompts in engineers (no stdin reads); preserves interactive UX otherwise.
    • list --json returns [] on no results; show <id> --json returns {"error": ...} and exits 1 when missing.
    • run keeps --no-interactive (no picker; requires --file when multiple scripts) and --auto-install (installs missing deps without confirm).
    • Root --help and command epilogs document examples, schemas, and exit codes; README adds a Scripted / Agent Usage section.
    • Exit codes: 0 success, 1 runtime error, 2 misuse.
  • Bug Fixes

    • Interrupted auto-captures now report status: "error" with error: "interrupted" (even when caught internally).
    • REPL refuses to start without a TTY (prints --help to stderr, exits 2) to avoid hangs in CI/agents.
    • har_path resolves against the provided --output-dir.
    • Misuse under --json emits the full documented schema (fields present with nulls).
    • Stdout purity: --json outputs exactly one JSON object; all Rich output is routed to stderr.

Written for commit 7a8d8b6. Summary will update on new commits.

Greptile Summary

This PR adds a machine-readable CLI surface (--json, --no-interactive, --auto-install) to the agent, list, show, and run commands, and normalises the return shape of run_auto_capture so the JSON payload is stable across SDK backends. The implementation is well-structured and the test coverage is solid; the findings below are all P2 quality-of-life items.

Confidence Score: 4/5

Safe to merge; all findings are P2 style/robustness concerns with no correctness impact on the happy path.

No P0 or P1 defects were found. Three P2 issues: inconsistent use of Rich's private _file attribute for console restoration, a test that doesn't truly validate stderr routing, and a loss of filtering context in the empty-JSON-array response for list --json. None of these affect the primary feature behaviour.

The _quiet_consoles_for_json() context manager in src/reverse_api/cli.py and the corresponding test in tests/test_cli_agent_json.py are the two areas that could use a quick follow-up.

Important Files Changed

Filename Overview
src/reverse_api/cli.py Adds --json/--no-interactive to agent and run commands, normalises run_auto_capture return shape; uses private Rich Console._file attribute for restoration, which may be fragile across Rich versions.
tests/test_cli_agent_json.py 10 new unit tests covering agent --json, list --json, show --json; test_no_interactive_without_prompt_exits_2 checks stderr without configuring mix_stderr=False, making the assertion unreliable.
README.md Adds a Scripted / Agent Usage section documenting --json, --no-interactive, exit codes, and example jq usage; content is accurate and matches the implementation.

Comments Outside Diff (1)

  1. src/reverse_api/cli.py, line 2018-2025 (link)

    P2 JSON mode loses empty-vs-filtered distinction

    When as_json=True, both an empty history and a filter that excludes all runs produce the same [] response. An agent consumer that passes --mode or --search has no way to tell whether the empty array means "no runs at all" or "your filter matched nothing", which could silently mask mistakes in filter arguments.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/reverse_api/cli.py
    Line: 2018-2025
    
    Comment:
    **JSON mode loses empty-vs-filtered distinction**
    
    When `as_json=True`, both an empty history and a filter that excludes all runs produce the same `[]` response. An agent consumer that passes `--mode` or `--search` has no way to tell whether the empty array means "no runs at all" or "your filter matched nothing", which could silently mask mistakes in filter arguments.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/reverse_api/cli.py:89-92
The context manager sets `candidate.file` via the public property setter (which can trigger side-effects like `_detect_color_system()` in some Rich versions) but restores with `c._file = original_inner`, bypassing those same side-effects entirely. This inconsistency could silently leave consoles in a wrong color/capability state after the context exits, and may break silently when Rich updates its internals. Prefer the public `file` setter for both directions.

```suggestion
    finally:
        sys.stdout = real_stdout
        for c, original_inner in redirected_consoles:
            c.file = original_inner
```

### Issue 2 of 3
tests/test_cli_agent_json.py:80-85
`CliRunner` defaults to `mix_stderr=True`, which merges stderr into `result.output` and leaves `result.stderr` as an empty string. The guard `result.stderr or result.output` always evaluates to `result.output`, so this test never actually verifies that the error was written to stderr specifically. Use `mix_stderr=False` and assert on `result.stderr` to make the intent explicit and reliable.

```suggestion
    def test_no_interactive_without_prompt_exits_2(self):
        runner = CliRunner(mix_stderr=False)
        result = runner.invoke(agent, ["--no-interactive"])
        assert result.exit_code == 2
        # Plain text on stderr, not JSON, since --json wasn't requested
        assert "prompt" in result.stderr.lower()
```

### Issue 3 of 3
src/reverse_api/cli.py:2018-2025
**JSON mode loses empty-vs-filtered distinction**

When `as_json=True`, both an empty history and a filter that excludes all runs produce the same `[]` response. An agent consumer that passes `--mode` or `--search` has no way to tell whether the empty array means "no runs at all" or "your filter matched nothing", which could silently mask mistakes in filter arguments.

Reviews (1): Last reviewed commit: "feat: agent-friendly CLI surface" | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Add scriptable / non-interactive flags so other agents can drive the
CLI for reverse engineering without TTY prompts.

- `agent` gains `--no-interactive` and `--json`; --json suppresses
  Rich output to stderr and emits a single stable JSON object on
  stdout (schema_version, status, run_id, prompt, url, mode, har_path,
  script_path, usage, error). Missing --prompt under --json/--no-interactive
  errors out with exit code 2 instead of opening questionary
- `run_auto_capture` now returns a normalized dict so the JSON payload
  has a stable shape regardless of the underlying engineer SDK
- `list --json` emits `[]` on empty history (was a human-readable
  "No runs found." line); `show <id> --json` emits a structured error
  and exits 1 when the run can't be found
- `run` gains `--no-interactive` (fail instead of opening a script
  picker) and `--auto-install` (install missing deps without confirm)
- Documented scripted usage and exit-code contract in the README
- Added tests/test_cli_agent_json.py covering the JSON payload shape
  and CLI-level edge cases (10 new tests)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kind-agent
Copy link
Copy Markdown

kind-agent Bot commented May 5, 2026

⚠️ Error — Not enough testing credits. Upgrade your plan or buy credits to continue running tests.

Repository owner deleted a comment from kind-agent Bot May 5, 2026
Comment thread src/reverse_api/cli.py
Comment on lines +89 to +92
finally:
sys.stdout = real_stdout
for c, original_inner in redirected_consoles:
c._file = original_inner
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The context manager sets candidate.file via the public property setter (which can trigger side-effects like _detect_color_system() in some Rich versions) but restores with c._file = original_inner, bypassing those same side-effects entirely. This inconsistency could silently leave consoles in a wrong color/capability state after the context exits, and may break silently when Rich updates its internals. Prefer the public file setter for both directions.

Suggested change
finally:
sys.stdout = real_stdout
for c, original_inner in redirected_consoles:
c._file = original_inner
finally:
sys.stdout = real_stdout
for c, original_inner in redirected_consoles:
c.file = original_inner
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/reverse_api/cli.py
Line: 89-92

Comment:
The context manager sets `candidate.file` via the public property setter (which can trigger side-effects like `_detect_color_system()` in some Rich versions) but restores with `c._file = original_inner`, bypassing those same side-effects entirely. This inconsistency could silently leave consoles in a wrong color/capability state after the context exits, and may break silently when Rich updates its internals. Prefer the public `file` setter for both directions.

```suggestion
    finally:
        sys.stdout = real_stdout
        for c, original_inner in redirected_consoles:
            c.file = original_inner
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +80 to +85
def test_no_interactive_without_prompt_exits_2(self):
runner = CliRunner()
result = runner.invoke(agent, ["--no-interactive"])
assert result.exit_code == 2
# Plain text on stderr, not JSON, since --json wasn't requested
assert "prompt" in (result.stderr or result.output).lower()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 CliRunner defaults to mix_stderr=True, which merges stderr into result.output and leaves result.stderr as an empty string. The guard result.stderr or result.output always evaluates to result.output, so this test never actually verifies that the error was written to stderr specifically. Use mix_stderr=False and assert on result.stderr to make the intent explicit and reliable.

Suggested change
def test_no_interactive_without_prompt_exits_2(self):
runner = CliRunner()
result = runner.invoke(agent, ["--no-interactive"])
assert result.exit_code == 2
# Plain text on stderr, not JSON, since --json wasn't requested
assert "prompt" in (result.stderr or result.output).lower()
def test_no_interactive_without_prompt_exits_2(self):
runner = CliRunner(mix_stderr=False)
result = runner.invoke(agent, ["--no-interactive"])
assert result.exit_code == 2
# Plain text on stderr, not JSON, since --json wasn't requested
assert "prompt" in result.stderr.lower()
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_cli_agent_json.py
Line: 80-85

Comment:
`CliRunner` defaults to `mix_stderr=True`, which merges stderr into `result.output` and leaves `result.stderr` as an empty string. The guard `result.stderr or result.output` always evaluates to `result.output`, so this test never actually verifies that the error was written to stderr specifically. Use `mix_stderr=False` and assert on `result.stderr` to make the intent explicit and reliable.

```suggestion
    def test_no_interactive_without_prompt_exits_2(self):
        runner = CliRunner(mix_stderr=False)
        result = runner.invoke(agent, ["--no-interactive"])
        assert result.exit_code == 2
        # Plain text on stderr, not JSON, since --json wasn't requested
        assert "prompt" in result.stderr.lower()
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/reverse_api/cli.py">

<violation number="1" location="src/reverse_api/cli.py:109">
P2: `har_path` is resolved against the default output dir, so JSON output can be wrong when `--output-dir` (or non-default config) is used.</violation>

<violation number="2" location="src/reverse_api/cli.py:1343">
P2: `agent --json` emits an inconsistent object shape on missing `--prompt`, which breaks stable-schema parsing for automation.</violation>

<violation number="3" location="src/reverse_api/cli.py:1367">
P1: Interrupted auto-capture can be reported as successful because `KeyboardInterrupt` is converted to a result without an error flag.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/reverse_api/cli.py
Comment thread src/reverse_api/cli.py Outdated
Comment thread src/reverse_api/cli.py Outdated
@kind-agent
Copy link
Copy Markdown

kind-agent Bot commented May 5, 2026

Test Results

🛡️ 3.75/6

Results

# Test Status Details
1 Targeted CLI JSON tests ✅ passed uv run pytest tests/test_cli_agent_json.py passed (10/10), covering _build_agent_payload, agent --json, list --json, show --json, and documented non-interactive edge cases.
2 Full pytest regression suite ❌ failed uv run pytest did not pass on this branch. In addition to the new tests passing, there were still failures in unrelated areas (test_config, test_opencode_ui, and test_sync), so I could not verify a clean regression-free suite.
3 Ruff on changed files ❌ failed uv run ruff check src/reverse_api/cli.py tests/test_cli_agent_json.py failed on existing issues in src/reverse_api/cli.py (three C901 complexity findings and one B904 raise-from warning). The new test file did not add lint errors.
4 agent --json stdout/stderr contract ❌ failed Manual invocation confirmed that --json keeps stdout machine-readable and pushes Rich/log output to stderr, but the command ignored my patched capture return and emitted a different runtime-generated run_id with script_path: null and empty usage after a login-related failure. That makes the JSON success path less trustworthy than the new tests suggest.
5 Non-interactive misuse/runtime contracts ⚠️ inconclusive I verified agent --json missing required args exits 2 with a JSON error, list --json returns [] for empty history, and show missing --json exits 1 with {"error": ..., "run_id": ...}. I also hit a TypeError while manually stubbing run --no-interactive, but that reproduction used an invalid stub shape, so I’m not treating it as a confirmed product bug.
6 README scripted usage docs ✅ passed The README examples and exit-code notes match the visible CLI help: agent exposes --json/--no-interactive, run exposes --no-interactive/--auto-install, and the documented JSON-oriented list/show usage behaved as described in isolated checks.

Issues Found

agent --json success path not reliably honoring mocked capture result: In a manual CliRunner invocation, patching reverse_api.cli.run_agent_browser to return a fixed payload did not produce that payload on stdout. Instead, the command attempted a real login-dependent flow, logged a "Not logged in · Please run /login" error to stderr, and still emitted an ok JSON object with a different run_id, script_path: null, and empty usage. I expected the final JSON to reflect the patched result shape exactly. Impact: automation consumers may get a superficially successful JSON object whose contents do not match the underlying run outcome.

Branch not green outside the targeted tests: The requested focused tests pass, but the overall branch still fails uv run pytest and uv run ruff check .... That lowers confidence in merge readiness even if the new CLI surface is mostly behaving.

Summary

The new targeted tests for the agent-friendly CLI surface pass, and the documented flags/JSON/error contracts mostly line up with the implementation. However, I found a concerning mismatch in manual agent --json behavior, and the branch is not green under the full pytest and requested ruff commands, so I’m not comfortable calling this PR a clean pass.


View full run details · Tested by Kind

I tested 51680c9 (feat: agent-friendly CLI surface)

📹 View browser recording

- Add 7 more tests covering: run --no-interactive (multi-script error,
  --file passthrough, refusal to install missing deps), --auto-install
  (skips questionary.confirm), agent --json KeyboardInterrupt path, and
  stdout-purity (Rich noise lands on stderr, not stdout)
- Add --help epilogs to agent / list / show / run with examples, JSON
  schema, and exit-code documentation so an agent inspecting --help
  can self-discover the scripted contract
- Expand README "Scripted / Agent Usage" section with a per-field JSON
  schema table and an exit-code table

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kalil0321 kalil0321 mentioned this pull request May 5, 2026
Three issues flagged by cubic-dev-ai on PR #61:

P1 — Interrupted auto-capture silently reported as ok
  When KeyboardInterrupt was caught inside run_auto_capture, the
  function returned a dict with no error flag, so _build_agent_payload
  saw a run_id and emitted status=ok. Now the inner KeyboardInterrupt
  bubbles up an "error: interrupted" key in the result dict, which
  _build_agent_payload propagates to status=error.

P2 — har_path resolved against the wrong output_dir
  _build_agent_payload computed the HAR path with the default config
  root, ignoring the user's --output-dir. Threaded output_dir through
  the agent click handler to _build_agent_payload so HAR resolution
  matches the actual capture location.

P2 — misuse JSON emitted incomplete schema
  When --json was set and --prompt was missing, the early-exit path
  printed only {schema_version, status, error}. Agents parsing the
  output expected every documented field. Now uses _build_agent_payload
  so misuse responses contain the full schema (with nulls).

Adds 3 regression tests covering each fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kalil0321
Copy link
Copy Markdown
Owner Author

Thanks for the reviews. Status of the feedback:

@cubic-dev-ai — all three issues acknowledged and fixed in c91d1fa

  • P1 — KeyboardInterrupt silently producing ok: Ack. Confirmed: run_auto_capture swallowed KeyboardInterrupt into result = None and then returned a dict with no error flag, so _build_agent_payload saw a run_id and reported status=ok. Fixed by setting error: "interrupted" on the inner result; added regression test test_inner_keyboard_interrupt_does_not_silently_succeed.
  • P2 — har_path against wrong output_dir: Ack. _build_agent_payload was calling get_har_dir(run_id, None), ignoring --output-dir. Threaded output_dir through the click handler; added test_har_path_resolves_against_provided_output_dir.
  • P2 — misuse JSON had incomplete shape: Ack. Early-exit on missing --prompt was emitting only 3 keys. Now routed through _build_agent_payload so all 10 fields are always present; added test_json_without_prompt_emits_full_schema.

@kind-agent — partial Ack

  • Test 4 (manual agent --json mismatch): No ACK as a product bug. The reproduction patched reverse_api.cli.run_agent_browser, which on this branch (and after cleanup: simplify agent mode providers #60 lands) is not the function the auto path uses — agent with the default agent_provider="auto" delegates to run_auto_capture, not run_agent_browser. Patching reverse_api.cli.run_agent_capture (the layer the click handler actually calls) is the correct seam — that's how the new tests in test_cli_agent_json.py mock it, and they verify the patched payload propagates to stdout exactly. The "Not logged in / Please run /login" message you saw is the real engineer flow firing because no SDK auth is configured in your sandbox; it's unrelated to the --json contract.
  • Test 2 (full pytest) and Test 3 (ruff): No ACK as new regressions. The 4 pytest failures (test_config::test_has_required_keys, test_opencode_ui::test_sync_flash, test_sync::test_is_temporary_file*, test_tui::test_thinking_truncation) and the C901/B904 lint findings all reproduce on main — verified via git stash + run on main. They're tracked separately and weren't touched by this PR.
  • Test 5 (TypeError on run --no-interactive stub): No ACK — you flagged it as an invalid stub on your side, so nothing to act on.

Re-review welcome at c91d1fa.

@kind-agent
Copy link
Copy Markdown

kind-agent Bot commented May 5, 2026

Re-running tests because the PR changed after the previous run.


Triggered by Kind

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai Bot commented May 5, 2026

Thanks for the reviews. Status of the feedback:

@cubic-dev-ai — all three issues acknowledged and fixed in c91d1fa

  • P1 — KeyboardInterrupt silently producing ok: Ack. Confirmed: run_auto_capture swallowed KeyboardInterrupt into result = None and then returned a dict with no error flag, so _build_agent_payload saw a run_id and reported status=ok. Fixed by setting error: "interrupted" on the inner result; added regression test test_inner_keyboard_interrupt_does_not_silently_succeed.
    ...

@kalil0321 All three fixes look correct in the latest diff — thanks for the thorough ack.

A couple of optional nits from the Greptile review that are still open (both P2 and easy to knock out):

  1. _quiet_consoles_for_json restores via private _file — the setter uses candidate.file = sys.stderr (the public property, which triggers color-system detection) but restores with c._file = original_inner (bypassing the property). If Rich ever changes its internals, those consoles could end up with a stale color capability. Swapping to c.file = original_inner on both sides avoids the asymmetry.

  2. mix_stderr in test_no_interactive_without_prompt_exits_2 — Click’s CliRunner defaults to mix_stderr=True, so result.stderr is always empty and the assertion falls through to result.output. Passing mix_stderr=False and asserting on result.stderr directly would make the test actually verify the stderr contract.

  3. list --json empty-vs-filtered — already flagged above. Not a correctness issue, just an agent-consumer quality-of-life thing.

None of these block the merge. The core --json/--no-interactive contract is solid, the tests are well-structured, and the README + --help epilogs make the contract discoverable. Good to go.

@kind-agent
Copy link
Copy Markdown

kind-agent Bot commented May 5, 2026

Test Results

🛡️ 4/6

Results

# Test Status Details
1 Focused CLI JSON regression tests ✅ passed uv run pytest tests/test_cli_agent_json.py -q passed 20/20. This covers the three acknowledged fixes: inner KeyboardInterrupt now surfaces as an error payload, har_path resolves against the provided --output-dir, and agent --json misuse on missing --prompt emits the full 10-key schema.
2 Direct agent --json stdout/stderr contract smoke ❌ failed I invoked the Click command with a patched capture seam that emits console output during a successful --json run. In that case, the log line appeared on stdout before the JSON payload as well as on stderr, so stdout was not a single parseable JSON document. This breaks the advertised machine-readable contract for success cases that print logs/progress.
3 run --no-interactive / --auto-install behavior ✅ passed Focused run-flag coverage passed (uv run pytest tests/test_cli_agent_json.py -k 'no_interactive or auto_install or run_' -q, 8/8). The command fails fast instead of prompting and supports automatic dependency install retry as intended.
4 Broader pytest / ruff baseline ✅ passed Broader checks still show failures, but they appear unrelated to this PR as the author noted: full pytest still has existing failures elsewhere, and ruff still reports many unrelated issues. I did not find new broad regressions in the changed CLI surface beyond the stdout contamination issue above.
5 README/help contract vs implementation ❌ failed The updated help and README match the intended flags/schema, but the statement that --json leaves stdout containing exactly one JSON document is not currently true under the successful logging scenario above, so the docs over-promise current behavior.

Issues Found

Direct agent --json stdout/stderr contract smoke: In a successful agent --json invocation, if the patched capture layer emits Rich/console output, that output can still leak to stdout before the final JSON object. I expected stdout to contain exactly one JSON document and all logs/progress to go only to stderr. Impact: wrapper scripts piping stdout into jq or another parser can fail even though the command otherwise succeeds. Repro note: invoke the agent Click command with --json and patch reverse_api.cli.run_agent_capture to call reverse_api.cli.console.print(...) before returning a normal result.

Summary

The re-reviewed targeted fixes from the author are in place and the new focused regression tests pass cleanly. However, I found a remaining contract bug in the primary user-facing agent --json success path: stdout can still be contaminated by console output, which means the advertised single-JSON machine-readable output is not yet reliable in all successful runs.


View full run details · Tested by Kind

I tested c91d1fa (fix(cli): address review feedback on agent --json contract)

📹 View browser recording

kalil0321 and others added 2 commits May 5, 2026 20:16
# Conflicts:
#	src/reverse_api/cli.py
Implements the first two items of the agent-friendliness backlog (#62):

#1 TTY-detect at REPL entry
  Without a TTY and no subcommand, the prompt_toolkit REPL would block on
  stdin forever. Now detects `not sys.stdin.isatty()`, prints `--help` to
  stderr, and exits 2 (misuse). Subcommands are unaffected.

#2 engineer --json / --no-interactive
  Mirrors the agent-mode contract (PR #61): --json redirects Rich output
  to stderr and emits a stable JSON payload on stdout. New helper
  `_build_engineer_payload` produces the schema {schema_version, status,
  run_id, prompt, fresh, script_path, usage, error}. KeyboardInterrupt
  surfaces as `error: "interrupted"`. Exit codes: 0 ok, 1 runtime error.
  --no-interactive is reserved for symmetry (engineer mode has no
  questionary prompts internally).

Also surfaces `--json` / `--no-interactive` in the root --help (item #6
partial).

Adds tests/test_cli_followups.py (14 tests) covering TTY-detect end-to-
end via subprocess, _build_engineer_payload edge cases, and the click
wiring for engineer --json (success, not-found, KeyboardInterrupt,
exception, prompt-vs-fresh threading).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kalil0321 and others added 5 commits May 5, 2026 20:31
After the first successful generation, ClaudeEngineer (and ClaudeAuto-
Engineer) loops on `BaseEngineer._prompt_follow_up()` which awaits
`input("  > ")` via an executor. With `--json` or `--no-interactive`
that block on stdin defeats the scripted contract: pipelines like
`engineer <run_id> --json | jq` would hang before emitting the JSON.

- Add `interactive: bool = True` to BaseEngineer; `_prompt_follow_up`
  now returns None immediately when interactive is False (still flushes
  sync so partial output reaches disk).
- Thread `interactive` through `run_reverse_engineering` and all three
  SDK constructors (Claude, OpenCode, Copilot — the latter two don't
  use the follow-up loop today but are wired for symmetry / future).
- Thread `interactive` through `cli.run_engineer`, `cli.run_auto_capture`,
  `cli.run_agent_capture`, and the auto-engineer constructors.
- The `agent` and `engineer` click commands derive `interactive = not
  (as_json or no_interactive)` and pass it down.

Adds 6 regression tests in test_cli_followups.py covering: the base
short-circuit (input() raises if reached), default interactive=True
(REPL UX preserved), and that all three click-command flag combinations
(--json, --no-interactive, default) thread the right value through
run_engineer / run_agent_capture.

Reported by chatgpt-codex-connector on PR #65 (P2).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Caught by ruff F401 during end-to-end smoke testing of PR #65.
Required for scripted/CI/VPS use cases where no X server is available.
Threads through the full chain to both ManualBrowser (Playwright launch)
and the MCP-spawned browser (auto and chrome-mcp providers).

Behavior:
- `manual --headless`: Playwright launches Chromium with headless=True.
  Also drops `--no-sandbox` from `ignore_default_args` so Chromium can
  start on hosts without unprivileged user namespaces (Ubuntu 24.04+
  with AppArmor restrictions). Headed mode keeps the existing stealth
  default (sandbox stripped for fingerprint realism).
- `agent --headless` (auto provider): rae-playwright-mcp spawned with
  `--headless` so the MCP-controlled Chromium runs without UI.
- `agent --headless` (chrome-mcp provider): drops `--autoConnect` from
  the MCP args because auto-connect requires a real headed Chrome with
  remote-debugging enabled. Adds `--headless` so the MCP spawns its
  own headless Chromium instead. The "auto-connect setup" preamble is
  replaced by a one-line note explaining the headless path.

All three SDK paths (Claude/OpenCode/Copilot, both ClaudeAutoEngineer
and the OpenCode/Copilot variants) accept and propagate `headless`
via kwargs to their auto-engineer constructors.

8 new tests in test_cli_followups.py covering: click flag wiring on
manual + agent (default false, --headless threads true), and the MCP
arg construction for all four headed/headless × playwright/chrome-mcp
combinations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per design intent, `manual` is interactive-only: a human navigates a
real, visible browser, clicks through flows, submits forms, etc. A
headless variant defeats the purpose; agents and CI should use
`agent --json --headless` instead.

- Remove --headless from the manual click command (passing it now
  fails with `No such option`)
- Remove headless parameter from ManualBrowser; restore the original
  Playwright launch (always headed, always strips --no-sandbox for
  fingerprint realism)
- Restore run_manual_capture signature
- Update the manual command docstring to make it explicit that the
  mode requires a human and to redirect agents to `agent --headless`

Tests: replace the two `manual --headless` wiring tests with checks
that (a) `manual --headless` exits non-zero with `No such option`
and (b) `manual --help` mentions both "human" and "agent" so an
agent inspecting --help self-routes correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(cli): TTY-detect at REPL + engineer --json (issue #62 items 1, 2, 6)
@kalil0321 kalil0321 merged commit 7b3d2f5 into main May 5, 2026
4 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.

1 participant