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