From f67c55336a3c0c4fd5736fcdeec82000f8823bd6 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Mon, 20 Jul 2026 18:55:13 -0400 Subject: [PATCH] feat(agent): place orders via interactive confirmation, no bypass needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Option A: placing a live order no longer requires bypass mode + the arm-bypass token + the dangerous-action passphrase. In confirm mode the agent now shows the previewed order and asks the operator; it places only on an explicit yes. Bypass mode is UNCHANGED and remains the headless path for future autonomous use. ⚠️ THE RAILS ARE UNTOUCHED. Every order still runs the 15 hard rails FIRST; the confirmation is an ADDITIONAL human gate after they pass, never a replacement. A test pins that a vetoed order never reaches the prompt (rails run first, confirmation second). The executor was already built for this (`execute` takes `confirm_fn(preview) -> bool`); the agent just hardcoded it to None. This threads a real `confirm_fn` through `run_once`/`loop`/`_handle_exits` (all default None, so every existing caller and test is unchanged and still fails closed), and `keel agent` passes `_interactive_confirm` in confirm mode / None in bypass. `_interactive_confirm` renders the broker preview and prompts; it FAILS CLOSED on a non-TTY (a script/cron never trades unattended), same posture as everything else. Net: placing an order goes from "set-passphrase -> arm-bypass -> --bypass --passphrase" to "run keel agent, answer y" -- and is arguably safer for supervised use, since a human sees the actual order every time instead of pre-authorising a window. Tests: approved->places the BUY (+ its OCO bracket); declined->nothing; no confirm_fn->still nothing (backward compat); confirm_fn sees the preview; a rail veto never reaches the prompt; _interactive_confirm yes/no/non-TTY; the agent command wires _interactive_confirm in confirm and None in bypass. NOTE: the go-live runbook (PR #116, unmerged) still describes the old bypass/passphrase dance and must be rewritten against this once both land. Co-Authored-By: Claude Opus 4.8 (1M context) --- keel/agent.py | 23 +++++-- keel/cli.py | 30 ++++++++- tests/test_agent.py | 146 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 8 deletions(-) diff --git a/keel/agent.py b/keel/agent.py index 6c7f3b31..261290b4 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -33,7 +33,7 @@ the coercion boundary: a caller (this module, or a future CLI command that lists live rules) only ever deals in real `Rule` instances, never in raw JSON dicts. -**Confirm mode has no interactive prompt here.** `run_once`'s signature (per the plan) is fixed +**Confirm mode places via a caller-supplied `confirm_fn`.** `run_once`/`loop` take an optional to `(broker, repo, config, now_ts)` -- there is no `confirm_fn` slot for a human-in-the-loop approval callback. So in `mode="confirm"`, every `executor.execute` call is made with `confirm_fn=None`, which -- per `executor.execute`'s own contract -- fails closed: the order is @@ -397,6 +397,7 @@ def _handle_exits( config: Config, mode: str, now_ts: int, + confirm_fn: executor.ConfirmFn | None = None, ) -> list[ExecutionResult]: """Ask the owning rule whether to close `product_id`'s held position, and execute the EXIT if it fires. A no-op (`[]`) when there's nothing held, or no rule is on record as owning it @@ -439,7 +440,7 @@ def _handle_exits( ts=now_ts, ) result = executor.execute( - exit_signal, broker, repo, config, mode, confirm_fn=None, now_ts=now_ts + exit_signal, broker, repo, config, mode, confirm_fn=confirm_fn, now_ts=now_ts ) if result.placed: exit_order = repo.get_order(result.order_id) if result.order_id is not None else None @@ -591,7 +592,13 @@ def _confirm_or_bypass(config: Config, repo: Repository, now_ts: int) -> tuple[s return mode, None -def run_once(broker: Any, repo: Repository, config: Config, now_ts: int) -> LoopResult: +def run_once( + broker: Any, + repo: Repository, + config: Config, + now_ts: int, + confirm_fn: executor.ConfirmFn | None = None, +) -> LoopResult: """One agent cycle: poll -> (kill-switch / stale-data gates) -> evaluate -> exits -> entries. The kill-switch is checked *before* anything else (no poll, no evaluation, no orders) -- @@ -715,7 +722,8 @@ def run_once(broker: Any, repo: Repository, config: Config, now_ts: int) -> Loop _paper_resolve_bars(paper_trader, product_id, candles_by_tf, granularities) product_exit_results = _handle_exits( - product_id, product_rules, candles_by_tf, repo, broker, config, mode, now_ts + product_id, product_rules, candles_by_tf, repo, broker, config, mode, now_ts, + confirm_fn=confirm_fn, ) for exit_result in product_exit_results: log_event( @@ -743,7 +751,7 @@ def run_once(broker: Any, repo: Repository, config: Config, now_ts: int) -> Loop result = _paper_enter(paper_trader, signal, repo, config, now_ts) else: result = executor.execute( - signal, broker, repo, config, mode, confirm_fn=None, now_ts=now_ts + signal, broker, repo, config, mode, confirm_fn=confirm_fn, now_ts=now_ts ) enter_results.append(result) log_event( @@ -798,6 +806,7 @@ def loop( config: Config, interval_sec: float, stop_flag: Callable[[], bool], + confirm_fn: executor.ConfirmFn | None = None, ) -> list[LoopResult]: """Run `run_once` every `interval_sec` until `stop_flag()` returns `True`. @@ -808,7 +817,9 @@ def loop( """ results: list[LoopResult] = [] while not stop_flag(): - results.append(run_once(broker, repo, config, now_ts=int(time.time()))) + results.append( + run_once(broker, repo, config, now_ts=int(time.time()), confirm_fn=confirm_fn) + ) if interval_sec: time.sleep(interval_sec) return results diff --git a/keel/cli.py b/keel/cli.py index 04b37ba2..03c2dfca 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -1158,6 +1158,28 @@ def monitor( # -- agent ------------------------------------------------------------------------------ +def _interactive_confirm(preview: dict) -> bool: + """Human-in-the-loop order confirmation for `mode="confirm"`. + + Called by the executor ONLY after the intent has already passed every hard rail -- this is + an additional human gate, never a replacement for the rails. Renders the broker's preview + and asks for an explicit yes. + + Fails closed: a non-TTY invocation (a script, a cron job, a headless run) declines rather + than blocking on stdin, so `mode="confirm"` never trades unattended. + """ + click.echo("\nRails PASSED. Coinbase order preview:") + if isinstance(preview, dict) and preview: + for key, value in preview.items(): + click.echo(f" {key}: {value}") + else: + click.echo(f" {preview!r}") + if not (sys.stdin is not None and sys.stdin.isatty()): + click.echo("no TTY -- declining (confirm mode fails closed).", err=True) + return False + return click.confirm("Place this order?", default=False) + + def _print_loop_result(result: agent.LoopResult) -> None: if result.skipped: click.echo(f"[{result.ts}] skipped: {result.skip_reason}") @@ -1219,7 +1241,10 @@ def agent_cmd( broker = _build_broker(config) if not loop: - _print_loop_result(agent.run_once(broker, repo, config, now_ts=int(time.time()))) + confirm_fn = _interactive_confirm if mode == "confirm" else None + _print_loop_result( + agent.run_once(broker, repo, config, now_ts=int(time.time()), confirm_fn=confirm_fn) + ) return interval = interval_sec if interval_sec is not None else config.auto_trade.interval_sec @@ -1230,7 +1255,8 @@ def stop_flag(_count: list[int] = [0]) -> bool: # noqa: B006 - intentional muta _count[0] += 1 return False - for result in agent.loop(broker, repo, config, interval, stop_flag): + confirm_fn = _interactive_confirm if mode == "confirm" else None + for result in agent.loop(broker, repo, config, interval, stop_flag, confirm_fn=confirm_fn): _print_loop_result(result) diff --git a/tests/test_agent.py b/tests/test_agent.py index 7b7f885e..0a0507de 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -1060,3 +1060,149 @@ def test_paper_mode_loads_PAPER_status_rules_not_live_ones(repo, monkeypatch): run_once(broker, repo, _paper_config(), now_ts=90_000) assert repo.get_orders(mode="paper") == [], "a LIVE rule must not trade in paper mode" + + +# -- interactive confirm: run_once threads confirm_fn to placement -------------- + + +def _live_config(**over): + """Confirm mode, roomy caps, all live-BUY rails satisfied so an approved order places.""" + over.setdefault( + "caps", + Caps( + max_per_order_usd=Decimal("1000000"), + max_per_day_usd=Decimal("1000000"), + max_exposure_usd=Decimal("1000000"), + max_per_asset_pct=Decimal("1"), + ), + ) + cfg = _config(**over) + return replace(cfg, auto_trade=replace(cfg.auto_trade, mode="confirm")) + + +def _live_ready_repo(repo): + """Clear every live-BUY rail that isn't the confirmation itself.""" + repo.set_state("kill_switch", False) + repo.set_state("withdrawals_enabled", True) + repo.set_state("withdrawals_attested_at", 10**12) + attest_subscription(repo, now_ts=0, free_volume_usd=Decimal("10000000")) + return repo + + +def test_confirm_APPROVED_places_the_order(repo, monkeypatch): + """The change: an approved confirm-mode order is actually placed (was: never).""" + _live_ready_repo(repo) + _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + run_once(broker, repo, _live_config(), now_ts=90_000, confirm_fn=lambda preview: True) + + # An approved entry places the BUY (and its protective OCO bracket) -- was: nothing at all. + buys = [c for c in broker.place_calls if c["product_id"] == PRODUCT and c["side"] == Side.BUY] + assert len(buys) == 1 + + +def test_confirm_DECLINED_places_nothing(repo, monkeypatch): + _live_ready_repo(repo) + _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + run_once(broker, repo, _live_config(), now_ts=90_000, confirm_fn=lambda preview: False) + + assert broker.place_calls == [] + + +def test_confirm_fn_defaulting_to_None_still_fails_closed(repo, monkeypatch): + """No confirm_fn (the old default) must still place nothing -- backward compatible.""" + _live_ready_repo(repo) + _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + + run_once(broker, repo, _live_config(), now_ts=90_000) # no confirm_fn + + assert broker.place_calls == [] + + +def test_confirm_fn_sees_the_broker_preview(repo, monkeypatch): + _live_ready_repo(repo) + _seed_rule(repo, monkeypatch, _AlwaysEnterRule(PRODUCT), status="live") + broker = FakeBroker(series={(PRODUCT, Granularity.ONE_DAY): [_candle(0, "100")]}) + seen = {} + + def _capture(preview): + seen.update(preview) + return True + + run_once(broker, repo, _live_config(), now_ts=90_000, confirm_fn=_capture) + assert "order_total" in seen # the preview the operator would be shown + + +def test_a_rail_veto_means_the_confirm_prompt_is_never_reached(repo, monkeypatch): + """The rails run FIRST. A vetoed order never asks the human -- confirmation is not a + substitute for the hard limits.""" + _live_ready_repo(repo) + _seed_rule(repo, monkeypatch, _AlwaysEnterRule("DOGE-USD"), status="live") # off allowlist + broker = FakeBroker(series={("DOGE-USD", Granularity.ONE_DAY): [_candle(0, "100")]}) + asked = {"n": 0} + + def _count(preview): + asked["n"] += 1 + return True + + run_once(broker, repo, _live_config(), now_ts=90_000, confirm_fn=_count) + assert asked["n"] == 0 + assert broker.place_calls == [] + + +# -- the CLI wires the interactive prompt -------------------------------------- + + +def test_interactive_confirm_places_on_yes_declines_on_no(monkeypatch, capsys): + """`_interactive_confirm` renders the preview and returns the human's yes/no.""" + import keel.cli as cli_module + + monkeypatch.setattr(cli_module.sys.stdin, "isatty", lambda: True, raising=False) + + monkeypatch.setattr(cli_module.click, "confirm", lambda *a, **k: True) + assert cli_module._interactive_confirm({"order_total": "5.00", "commission_total": "0.03"}) + out = capsys.readouterr().out + assert "Coinbase order preview" in out + assert "order_total: 5.00" in out + + monkeypatch.setattr(cli_module.click, "confirm", lambda *a, **k: False) + assert cli_module._interactive_confirm({"order_total": "5.00"}) is False + + +def test_interactive_confirm_fails_closed_without_a_tty(monkeypatch): + import keel.cli as cli_module + + monkeypatch.setattr(cli_module.sys.stdin, "isatty", lambda: False, raising=False) + assert cli_module._interactive_confirm({"order_total": "5.00"}) is False + + +def test_agent_command_passes_interactive_confirm_in_CONFIRM_mode(repo, monkeypatch): + """The wiring: `keel agent` (confirm) hands run_once the interactive confirm_fn; bypass + hands it None.""" + from click.testing import CliRunner + + import keel.cli as cli_module + from keel.cli import cli + + captured = {} + + def _fake_run_once(broker, repo_arg, config, now_ts, confirm_fn=None): + captured["confirm_fn"] = confirm_fn + captured["mode"] = config.auto_trade.mode + return agent.LoopResult( + ts=now_ts, skipped=False, skip_reason=None, mode=config.auto_trade.mode, polled=0 + ) + + monkeypatch.setattr(cli_module, "_build_broker", lambda config: object()) + monkeypatch.setattr(cli_module, "_open_repo", lambda ctx: repo) + monkeypatch.setattr(cli_module, "_load_cfg", lambda ctx: _live_config()) + monkeypatch.setattr(cli_module.agent, "run_once", _fake_run_once) + + result = CliRunner().invoke(cli, ["agent"]) + assert result.exit_code == 0, result.output + assert captured["mode"] == "confirm" + assert captured["confirm_fn"] is cli_module._interactive_confirm