Skip to content

feat(cli): TTY-detect at REPL + engineer --json (issue #62 items 1, 2, 6)#65

Merged
kalil0321 merged 5 commits into
feat/agent-friendly-clifrom
feat/agent-friendly-followups
May 5, 2026
Merged

feat(cli): TTY-detect at REPL + engineer --json (issue #62 items 1, 2, 6)#65
kalil0321 merged 5 commits into
feat/agent-friendly-clifrom
feat/agent-friendly-followups

Conversation

@kalil0321
Copy link
Copy Markdown
Owner

@kalil0321 kalil0321 commented May 5, 2026

Stacked on top of #61 (feat/agent-friendly-cli). Once #61 lands, the base will rebase to main.

Summary

Knocks out the first two blocker items from #62 (agent friendliness) plus a partial fix for item #6.

Item #1 — TTY-detect at REPL entry

Without a TTY and no subcommand, reverse-api-engineer (bare) would block on prompt_toolkit forever — a footgun for CI invocations or wrappers that forget the subcommand. Now exits 2 with --help printed to stderr.

```bash
$ reverse-api-engineer < /dev/null; echo $?
... (--help output) ...
error: no subcommand given and stdin is not a TTY; the interactive REPL requires a terminal.
2
```

Item #2engineer --json / --no-interactive

Mirrors the agent-mode JSON contract (#61). Stable schema:
```json
{
"schema_version": 1,
"status": "ok" | "error",
"run_id": "",
"prompt": "..." | null,
"fresh": false,
"script_path": "/abs/api_client.py" | null,
"usage": { "input_tokens": ..., "output_tokens": ..., "total_cost": ... },
"error": "" | null
}
```

Now an agent can chain the full pipeline end-to-end:
```
agent --json → run_id → engineer <run_id> --json → run <run_id> --no-interactive
```

Item #6 (partial) — root --help advertises scripted features

Added a one-liner in the root command docstring so an agent inspecting --help discovers that subcommands accept --json / --no-interactive.

Still open from #62

Test plan

  • `uv run pytest tests/test_cli_followups.py` (14/14 — TTY-detect via real subprocess, `_build_engineer_payload` edge cases, engineer click wiring with success/not-found/KeyboardInterrupt/Exception, --prompt vs --fresh threading)
  • `uv run pytest` full suite — 673 passing; 5 pre-existing failures all reproduce on the base branch
  • Manual smoke: `reverse-api-engineer < /dev/null` exits 2; `engineer doesnotexist --json` returns full schema with status=error
  • End-to-end smoke of an actual capture+engineer chain before merge

🤖 Generated with Claude Code


Summary by cubic

Adds TTY detection to prevent REPL hangs, adds engineer --json/--no-interactive with a stable JSON output, and adds --headless to agent for CI/VPS use; non-interactive runs now skip follow-up prompts. Addresses issue #62 items 1, 2, and partially 6; root --help now mentions scripted flags, and manual is explicitly human-only.

  • New Features

    • engineer --json emits one JSON payload (logs to stderr) with: schema_version, status, run_id, prompt, fresh, script_path, usage, error; exits 0 on ok, 1 on error. --no-interactive implies non-interactive.
    • agent --headless: For MCP, chrome-mcp headless drops --autoConnect and adds --headless; playwright adds --headless. manual stays human-only and does not accept --headless (help text points agents to agent --headless).
  • Bug Fixes

    • No subcommand + non-TTY prints --help to stderr and exits 2 instead of hanging.
    • In --json or --no-interactive modes, follow-up prompts are fully suppressed: interactive=False is threaded through engineer, agent, run_reverse_engineering, and into BaseEngineer (whose _prompt_follow_up() now returns immediately).

Written for commit 0e5ef06. Summary will update on new commits.

Greptile Summary

This PR adds two agent-friendliness improvements to the CLI: a TTY guard that prevents prompt_toolkit from blocking indefinitely when the tool is invoked without a subcommand in a non-TTY environment (exits 2 with help on stderr), and a --json / --no-interactive flag pair on the engineer subcommand that mirrors the existing agent --json contract.

  • TTY detection (main): checks sys.stdin.isatty() before entering repl_loop; prints help to stderr and calls sys.exit(2) on failure — tested end-to-end with a real subprocess.
  • engineer --json (engineer command + _build_engineer_payload): wraps run_engineer in _quiet_consoles_for_json, catches KeyboardInterrupt and generic exceptions, and emits a stable JSON schema on stdout with exit code 0/1; two minor style inconsistencies with the sibling _build_agent_payload function noted.
  • Root --help update: adds a one-liner advertising --json / --no-interactive support to the root command docstring.

Confidence Score: 4/5

Safe to merge; the new code paths are well-tested and follow an established pattern already used by the agent command.

The TTY guard and engineer JSON path are straightforward, well-covered by 14 new tests (subprocess + unit + Click wiring), and mirror a pattern already proven by the agent command. Two cosmetic inconsistencies between _build_engineer_payload and _build_agent_payload were flagged but neither affects correctness.

No files require special attention; the only items worth a second glance are the redundant isinstance guards in _build_engineer_payload.

Important Files Changed

Filename Overview
src/reverse_api/cli.py Adds _build_engineer_payload, TTY-guard in main, and --json/--no-interactive flags to the engineer command. Logic is sound and mirrors the existing agent JSON path; two minor style inconsistencies with _build_agent_payload noted.
tests/test_cli_followups.py New test file covering TTY detection (via real subprocess), payload-builder edge cases, Click command wiring, and --prompt/--fresh threading. Good coverage of success, not-found, KeyboardInterrupt, and exception paths.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/reverse_api/cli.py:138-150
After line 138 reassigns `result` to `{}` when it is `None`, `result` is always a `dict`. The `isinstance(result, dict)` guards on the next three lines are therefore always `True` and the branches they guard against can never fire. `_build_agent_payload` does the same normalization without any `isinstance` guards, and the inconsistency may confuse future readers. Simpler to mirror the agent function's style.

```suggestion
    result = result if result is not None else {}
    inner_error = result.get("error")
    final_error = error or inner_error or (None if result else "engineering produced no result")
    return {
        "schema_version": AGENT_JSON_SCHEMA_VERSION,
        "status": "error" if final_error else "ok",
        "run_id": run_id,
        "prompt": prompt,
        "fresh": fresh,
        "script_path": result.get("script_path"),
        "usage": result.get("usage") or {},
        "error": final_error,
    }
```

### Issue 2 of 2
src/reverse_api/cli.py:151-152
PEP 8 requires two blank lines between top-level definitions. There is only one blank line between the closing brace of `_build_engineer_payload` and the `# Mode definitions` comment, while everywhere else in this file top-level blocks are separated by two blank lines.

Reviews (1): Last reviewed commit: "feat(cli): TTY-detect at REPL entry + en..." | Re-trigger Greptile

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>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2c4e49b86

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/reverse_api/cli.py
Comment on lines +1496 to +1503
result = run_engineer(
run_id,
prompt=main_prompt,
additional_instructions=additional,
model=model,
output_dir=output_dir,
is_fresh=fresh,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Disable follow-up prompts in JSON engineer mode

When the default claude SDK is used and stdin is still a TTY, this --json path calls the same interactive run_engineer flow, and ClaudeEngineer.analyze_and_generate() then waits in BaseEngineer._prompt_follow_up() via input(" > ") after the first successful generation. That means reverse-api-engineer engineer <run_id> --json | jq can block before emitting the JSON payload, which defeats the scripted/non-interactive contract advertised by this change; the JSON/no-interactive path needs to suppress follow-up prompting before calling the engineer.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ack — fixed in ea9ea77. Confirmed by reading ClaudeEngineer.analyze_and_generate (engineer.py:172-176): the conversation loop calls _prompt_follow_up() after the first generation, which awaits input(" > ") via a thread executor. Same bug exists in auto_engineer.py:170 so agent --json had the same latent issue.

Fix:

  • Added interactive: bool = True to BaseEngineer; _prompt_follow_up short-circuits to None when interactive=False (still flushes sync so partial output reaches disk)
  • Threaded interactive through run_reverse_engineering, all three SDK constructors, run_engineer, run_auto_capture, run_agent_capture and the click commands
  • engineer and agent derive interactive = not (as_json or no_interactive) and pass it down
  • Added 6 regression tests in test_cli_followups.py, including one that patches builtins.input to raise so it'd fail loudly if the short-circuit ever regressed

@kind-agent
Copy link
Copy Markdown

kind-agent Bot commented May 5, 2026

Test Results

🛡️ 5.5/6

Results

# Test Status Details
1 Follow-up CLI regression suite ✅ passed Ran uv run pytest tests/test_cli_followups.py -vv; all 14 tests passed, covering bare-command no-TTY exit, engineer JSON payload normalization, error paths, prompt/fresh threading, and root help assertions.
2 Bare CLI with non-TTY stdin ✅ passed Invoking uv run reverse-api-engineer with stdin from /dev/null exited quickly with code 2, printed help to stderr, and did not hang. list --json still worked non-interactively with clean stdout JSON, showing the guard only applies to the no-subcommand REPL path.
3 engineer --json contract and edge cases ✅ passed Verified via the new tests and direct payload checks that success/error outputs keep the expected stable keys, handle result=None, missing script_path, empty usage, and explicit error override, and keep machine-readable JSON on stdout.
4 Prompt/fresh and exception wiring ✅ passed Confirmed coverage for --prompt additive behavior without --fresh, prompt replacement with --fresh, KeyboardInterrupt, generic exception, not-found behavior, and acceptance of --no-interactive; all passed.
5 Root and engineer help text ✅ passed Real CLI output shows root help now advertises --json and --no-interactive, while engineer --help documents prompt/fresh semantics, scripted flags, examples, JSON schema, and exit-code guidance.
6 Agent → engineer chaining smoke ⚠️ inconclusive I partially exercised the chain: agent --json --url https://example.com --no-interactive returned a machine-readable payload with a run_id, but the sandbox is not logged in, so engineer <run_id> --json produced structured error JSON and I could not meaningfully continue into run --no-interactive.
7 Broader pytest regression sweep ✅ passed uv run pytest still exits non-zero, but the visible failures are outside this PR’s scope (tests/test_sync.py and tests/test_tui.py). The changed CLI areas, including tests/test_cli_followups.py, passed within the full run.

Summary

The PR’s targeted CLI changes behaved correctly in testing: the new non-TTY REPL guard works, engineer --json follows the documented machine-readable contract, and the discoverability/help text updates are present in real command output. I did not find a regression attributable to this PR; the only unclosed gap was the full end-to-end chain, which was blocked by the sandbox’s logged-out state rather than by the CLI changes under review.


View full run details · Tested by Kind

I tested d2c4e49 (feat(cli): TTY-detect at REPL entry + engineer --json/--no-interactive)

📹 View browser recording

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.

1 issue found across 2 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:1496">
P1: The `--json` path calls `run_engineer()` without passing any flag to suppress follow-up prompting. If the engineer internally calls `input()` for follow-up questions (e.g., in `BaseEngineer._prompt_follow_up()`), `engineer <run_id> --json | jq` will block indefinitely before emitting JSON output. Either pass a `non_interactive=True` parameter through to `run_engineer`, or patch `stdin` to immediately signal EOF within the `--json` branch.</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
kalil0321 and others added 4 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>
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.

1 issue found across 3 files (changes from recent commits).

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/browser.py">

<violation number="1">
P1: Do not unconditionally strip `--no-sandbox`; this can make Chromium fail to launch in CI/container/root setups where Playwright’s default no-sandbox behavior is required.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

@kalil0321 kalil0321 merged commit 7a8d8b6 into feat/agent-friendly-cli May 5, 2026
2 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