From ea2045a923a00fd9adf73bb7cea7696a38873dcc Mon Sep 17 00:00:00 2001 From: kina0630 Date: Sat, 9 May 2026 22:22:23 +0800 Subject: [PATCH] feat(remote): add approval scope choices Remote attach and repl needed parity with the local approval UI so users can approve once, approve for the session, or reject with feedback from the CLI. --- .../adr-0019-pr8-remote-approval-scope.md | 37 +++++++ .../src/coding_agent/remote/client.py | 26 +++-- coding-agent/tests/cli/test_remote_client.py | 96 +++++++++++++++++-- 3 files changed, 144 insertions(+), 15 deletions(-) create mode 100644 coding-agent/.opencode/prompts/tasks/adr-0019-pr8-remote-approval-scope.md diff --git a/coding-agent/.opencode/prompts/tasks/adr-0019-pr8-remote-approval-scope.md b/coding-agent/.opencode/prompts/tasks/adr-0019-pr8-remote-approval-scope.md new file mode 100644 index 000000000..dc7a72b10 --- /dev/null +++ b/coding-agent/.opencode/prompts/tasks/adr-0019-pr8-remote-approval-scope.md @@ -0,0 +1,37 @@ +Goal: +Add the missing ADR-0019 remote approval prompt choices so remote attach/repl users can approve once, approve for the session, or reject with feedback from the CLI. + +Scope: +- Extend `src/coding_agent/remote/client.py` remote approval prompting to capture session-scope approval and rejection feedback. +- Add focused CLI regression tests for the remote approval request flow. +- Keep the change local to the remote client approval UX without changing server approval semantics. + +Out of scope: +- Do not change HTTP approval schema or session-manager approval coordination. +- Do not add retries, validation loops, or broader remote attach UX refactors. + +Context: +- ADRs: + - `docs/adr/0019-remote-client-cloud-workspace-deployment.md` +- Relevant files: + - `src/coding_agent/remote/client.py` + - `src/coding_agent/ui/approval_prompt.py` + - `tests/cli/test_remote_client.py` + - `tests/ui/test_http_server.py` + +Target tests: +- `uv run pytest tests/cli/test_remote_client.py -k "approval_request or formats_approval" -v` +- `uv run pytest tests/ui/test_http_server.py -k "approval and session" -v` +- `uv run basedpyright --level error src/coding_agent/remote/client.py tests/cli/test_remote_client.py` + +Loop policy: +- Engineer implements the smallest correct change and runs the target tests. +- Reviewer reviews only the resulting diff and affected tests. +- Reviewer reports only P1/P2 findings. +- Engineer fixes only accepted P1/P2 findings and reruns the same target tests. +- Verifier reruns the exact target tests and reports pass/fail only. + +Stop conditions: +- At most one review/fix/retest cycle. +- Escalate architectural redirection or scope expansion to the human. +- Ignore non-blocking optimization suggestions. diff --git a/coding-agent/src/coding_agent/remote/client.py b/coding-agent/src/coding_agent/remote/client.py index b08578694..ffee74c0e 100644 --- a/coding-agent/src/coding_agent/remote/client.py +++ b/coding-agent/src/coding_agent/remote/client.py @@ -318,13 +318,27 @@ def _prompt_for_approval(payload: dict[str, object]) -> _ApprovalDecision: sort_keys=True, ) ) - # TODO(adr-0019): Extend the remote approval prompt to capture session-scope - # approvals and user feedback instead of the current yes/no confirmation. - approved = click.confirm( - f"Approve remote tool request {tool_name}?", - default=False, + click.echo( + "[y]=approve [a]=approve all (session) [n]=reject [r]=reject with reason" ) - return _ApprovalDecision(approved=approved) + choice = cast( + str, click.prompt("→", default="n", show_default=False) + ).strip().lower() + if choice in ("y", "yes"): + return _ApprovalDecision(approved=True, scope="once") + if choice in ("a", "all"): + return _ApprovalDecision(approved=True, scope="session") + if choice in ("r", "reason"): + feedback = cast( + str, + click.prompt("Reason", default="Rejected by user", show_default=False), + ) + return _ApprovalDecision( + approved=False, + feedback=feedback.strip() or "Rejected by user", + scope="once", + ) + return _ApprovalDecision(approved=False, feedback="Rejected by user", scope="once") def _end_inline_stream_line(line_open: bool) -> bool: diff --git a/coding-agent/tests/cli/test_remote_client.py b/coding-agent/tests/cli/test_remote_client.py index 314b731a1..44e2b5106 100644 --- a/coding-agent/tests/cli/test_remote_client.py +++ b/coding-agent/tests/cli/test_remote_client.py @@ -821,7 +821,7 @@ def fake_stream_prompt( def test_remote_approval_request_prompts_before_submitting_decision(monkeypatch) -> None: approvals: list[dict[str, object] | None] = [] - prompts: list[tuple[str, bool]] = [] + prompts: list[str] = [] class FakeResponse: def raise_for_status(self) -> None: @@ -847,12 +847,13 @@ def post( approvals.append(json) return FakeResponse() - def fake_confirm(text: str, default: bool) -> bool: - prompts.append((text, default)) - return False + def fake_prompt(text: str, default: str | None = None, show_default: bool = True) -> str: + del default, show_default + prompts.append(text) + return "a" monkeypatch.setattr("coding_agent.remote.client.httpx.Client", FakeClient) - monkeypatch.setattr("coding_agent.remote.client.click.confirm", fake_confirm) + monkeypatch.setattr("coding_agent.remote.client.click.prompt", fake_prompt) from coding_agent.remote.client import handle_sse_event @@ -875,9 +876,79 @@ def fake_confirm(text: str, default: bool) -> bool: assert status is None assert line_open is False - assert prompts == [("Approve remote tool request bash_run?", False)] + assert prompts == ["→"] assert approvals == [ - {"request_id": "req-1", "approved": False, "scope": "once"} + {"request_id": "req-1", "approved": True, "scope": "session"} + ] + + +def test_remote_approval_request_can_reject_with_reason(monkeypatch) -> None: + approvals: list[dict[str, object] | None] = [] + prompts: list[str] = [] + + class FakeResponse: + def raise_for_status(self) -> None: + return None + + def json(self) -> dict[str, object]: + return {"status": "ok"} + + class FakeClient: + def __init__(self, **kwargs: object) -> None: + del kwargs + + def __enter__(self) -> FakeClient: + return self + + def __exit__(self, exc_type: object, exc: object, tb: object) -> None: + return None + + def post( + self, path: str, json: dict[str, object] | None = None + ) -> FakeResponse: + del path + approvals.append(json) + return FakeResponse() + + answers = iter(["r", "Need a safer command"]) + + def fake_prompt(text: str, default: str | None = None, show_default: bool = True) -> str: + del default, show_default + prompts.append(text) + return next(answers) + + monkeypatch.setattr("coding_agent.remote.client.httpx.Client", FakeClient) + monkeypatch.setattr("coding_agent.remote.client.click.prompt", fake_prompt) + + from coding_agent.remote.client import handle_sse_event + + status, line_open = handle_sse_event( + base_url="http://agent.example", + session_id="sess-approval", + headers={}, + event="ApprovalRequest", + data=json.dumps( + { + "request_id": "req-1", + "tool_call": { + "tool_name": "bash_run", + "arguments": {"command": "rm -rf scratch"}, + "call_id": "call-1", + }, + } + ), + ) + + assert status is None + assert line_open is False + assert prompts == ["→", "Reason"] + assert approvals == [ + { + "request_id": "req-1", + "approved": False, + "scope": "once", + "feedback": "Need a safer command", + } ] @@ -912,7 +983,8 @@ def post( monkeypatch.setattr("coding_agent.remote.client.httpx.Client", FakeClient) monkeypatch.setattr( - "coding_agent.remote.client.click.confirm", lambda text, default: False + "coding_agent.remote.client.click.prompt", + lambda text, default=None, show_default=True: "n", ) from coding_agent.remote.client import handle_sse_event @@ -949,12 +1021,18 @@ def post( assert status is None assert line_open is False assert approvals == [ - {"request_id": "req-1", "approved": False, "scope": "once"} + { + "request_id": "req-1", + "approved": False, + "scope": "once", + "feedback": "Rejected by user", + } ] assert capsys.readouterr().out == ( "partial\n" "[approval] Remote tool request bash_run\n" '{\n "command": "rm -rf scratch"\n}\n' + "[y]=approve [a]=approve all (session) [n]=reject [r]=reject with reason\n" )