feat: M4 demo executor daemon (long-running) + 6 rounds of codex review#1
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract public_books5_subscribe_message helper in bitget.rs (reuse from verify_public_ws_connects) and add run_public_ws_loop + apply_public_market_update in daemon.rs. The loop connects, subscribes books5, parses via parse_public_ws_message, and refreshes a shared Arc<Mutex<MarketCache>> with LOCAL-received timestamps; fixed 1s reconnect backoff (ponytail: exponential if disconnects become frequent). Not yet wired into run_daemon (Task 7). 75 tests green; clippy clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract the pending-intent loop out of run_once_or_loop into a reusable process_pending_intents_once so the upcoming daemon (Task 7) can drive the same code path. One-shot behavior is unchanged: same per-intent order, same freshness fail-fast, same events. Widen require_fresh_market to pub(crate) for daemon reuse; add a regression test on its error message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes code-review findings 4 and 5: the private WS loop now subscribes to the orders/positions/account channels after login, and parse_private_ws_message parses positions and account events (not just orders) and stops hard-coding the symbol via a config-backed instId→display-symbol resolver. Account snapshots are written through apply_private_ws_update via the existing insert_equity_snapshot helper. REST reconcile remains the source of truth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire run_daemon's real orchestration: startup reconcile-before-intents, spawned public/private WS loops on a shared watch shutdown channel, a 250ms poll loop that runs periodic reconcile + process_pending_intents_once, and ctrl_c / bounded-runtime exits. Loop errors (reconcile, intent-loop) are logged as events, never propagated — the daemon does not crash on a stale- market or flaky-REST tick. Add tokio signal feature + should_run_reconcile gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 8 (M4): /status /positions /orders /pnl /risk now map to read-only SQLite-backed replies via telegram_query::query_response. No side effects — these never write rows or move orders. /stop /resume /close_all are refused with "remote trading controls are not supported in M4" (M4 forbids remote trading control). query_response returns None for unrecognized commands so the loop doesn't reply to noise. run_telegram_query_loop polls Telegram getUpdates only when both bot token and chat_id are configured (else returns early — Telegram is not an execution dependency), filters to the operator's chat_id, and replies only on Some. All Telegram/network/SQLite errors are logged and swallowed so a flaky getUpdates or a transient DB lock never crashes the daemon. Spawned from run_daemon alongside the WS loops; cooperative shutdown extended to all three tasks via futures_util::future::join3, keeping the 200ms grace window + abort fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex finding 2: process_pending_intents_once called require_fresh_market
unconditionally per intent, blocking CLOSE/reduce/cancel on stale WS and
aborting the whole batch via ? on a stale open at the head (head-of-line
blocking). M4 spec (design line 95 / AC 8) requires stale market to block
ONLY new opening exposure; close/cancel/de-risk must stay allowed.
Add a pure market_gate(action, cache) -> {UseWs, DeferOpen, UseRestTicker}
helper and rewrite the per-intent loop body:
- DeferOpen: opening action + stale/missing WS -> leave pending, continue
(never fail; transient WS gap must not permanently reject an open).
- UseRestTicker: risk-reducing action -> fetch fresh REST ticker (and
market_cache.update so later opens in the batch see fresh data), then
process_one_intent, regardless of WS staleness.
- UseWs: opening action with fresh WS -> process_one_intent.
Per-intent error isolation: every arm continues, none aborts the batch
with ?. processed += 1 / 'processed intent' event fires only when
process_one_intent actually ran (not for a deferred open).
One-shot path unchanged in behavior: run_once_or_loop seeds the cache from
a fresh REST ticker immediately before the loop, so at one-shot time
latest_fresh returns Some -> market_gate(open)=UseWs, defer never triggers.
Verified by the python one-shot integration test (1 passed).
require_fresh_market is now dead code (its only consumer was the rewritten
loop; maker-retry/taker arms use fetch_market_snapshot directly); deleted it
and its two tests. market_gate_defers_open_when_cache_stale covers the
stale-don't-act behavior at the gate level where it now lives.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Set pragma journal_mode=wal on the daemon's main connection (idempotent;
matches src/prodigy/db.py) so concurrent Python/daemon/WS/Telegram access
uses WAL, not the default rollback journal.
- /pnl now explicitly states it is unrealized-only (M4); realized PnL is a
future-milestone concern.
- CLI/config demo-mode rejection wording is milestone-neutral ("prodigy
executor only supports --mode demo") — no longer says "third milestone".
Preserved the "only supports --mode demo" / "demo" substrings the tests
and the python live-reject test assert on.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
process_pending_intents_once now evaluates the market gate BEFORE fetching the REST account snapshot. A stale public WS defers an OPEN immediately (without a REST call), so the daemon doesn't hammer REST every 250ms tick while the WS is down. Close/reduce and fresh-WS opens still fetch the account + proceed as before. Also de-duplicates the two process_one_intent call sites (UseWs/UseRestTicker) into one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Previously the WAL pragma failure was swallowed with let _ =. Since M4 claims WAL-compatible behavior, a failure should be observable: write a warning event so the operator knows the DB isn't WAL (busy_timeout still serializes writers, so it isn't fatal). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vent flood)
Round-4 codex findings, each fixed at the root cause with regression tests:
1. (Critical) Lot-size rounding dropped whole lots at FP boundaries.
round_down_to_step used a bare (value/step).floor(); one lot of 0.01 is
stored as 0.009999999999999995, so 0.9999.../step floored to 0 — sizing
close orders to 0 (Bitget rejects "less than minimum order quantity") and
the same defect on the production path. Add a 1e-9 epsilon in lot-count
space: absorbs FP noise at a lot boundary (a value at/above an integer lot
keeps it) without bumping a genuine sub-lot remainder up. Stays round-DOWN
so placed size never exceeds approved notional. Demo close test now routes
the observed fill through the production sizer (format_size) instead of the
raw repr.
2. (Important) Private WS readiness was set true before the login ack.
The loop set private_ready=true right after sending subscribe, with no ack
wait, and the parser ignored event:error. Now: the parser surfaces
login_ack / auth_error; the loop waits for the login ack (bounded 10s)
before subscribing or marking ready, and emits a websocket_auth_failed
event (+ telegram) on auth failure. Caught-in-verification: Bitget
serializes the login `code` as a NUMBER ({"event":"login","code":0}), not a
string — the first cut mis-read every successful ack as an auth failure and
wedged the daemon. ws_code() normalizes number/string forms.
3. (Important) Accepted intent could wedge on a later infrastructure error.
process_one_intent accepts (pending->accepted) before driving the state
machine, and accept_intent only matches `pending`, so a bare `?` after
accept (sizing, retry/taker account snapshot, order-row writes) left the
intent accepted and invisible to every future tick — wedged forever. One
root-cause guard in the shared caller: on Err from process_one_intent,
fail_intent_after_infra_error flips it to `failed` (reconcile owns the real
order/position truth). Covers all current and future `?` paths.
4. (Minor) Stale-open defer event flooded SQLite every 250ms tick.
A WS outage wrote one "deferred open" row per tick per pending open.
Rate-limit per intent via executor_state (deferred_open:<id> = last-emit ms):
emit at most once per 10s; the intent defers either way. Pure
defer_event_recently_emitted helper for unit testing.
Verification (all green): cargo fmt --check; git diff --check main...HEAD;
cargo clippy --all-targets --all-features -- -D warnings; cargo test -q
(102 lib + 5 bin + 3 bitget_demo network); pytest (54 + daemon network test
passes when the phantom demo book lets the first tick complete).
Demo-only invariant holds; Telegram remote controls still refused.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…off, verify parser)
Round-5 codex findings, three private-WS stability fixes:
1. (Important) Post-ready {"event":"error",...} was silently ignored. After
private_ready.set(true), the inner read loop only checked
orders/fills/positions/account for emptiness; an auth_error (subscribe
rejected, key revoked mid-session) carries none of those, so the empty-skip
dropped it and private state stayed READY while the channel was broken —
new opens would proceed on a dead feed. Now: check update.auth_error BEFORE
the empty-check; on auth_error, write the websocket_auth_failed event, flip
private_ready false, and break to the outer reconnect loop (which backs off
1s + fresh login).
2. (Important) Auth-failure and ack-timeout branches did a bare `continue`,
skipping the 1s reconnect backoff at the bottom of the outer loop. A bad
key or dead socket would tight-loop reconnect and flood the events table /
Telegram. Both branches now sleep(1s) before continuing — same convention
the login-send-fail and subscribe-send-fail branches already use.
3. (Minor) verify_private_ws_connects (one-shot + daemon startup check) still
used string-contains probes for login/error while the daemon loop used the
new parser. Rewrote it to reuse parse_private_ws_message so both paths judge
login identically (incl. Bitget's numeric login code) and surface the real
code+msg on failure.
Refactor: extracted emit_websocket_auth_failed (event + best-effort demo
Telegram) shared by the pre-ready login-failure path and the new post-ready
auth-error path, so the two emission sites can't drift apart.
Verification (all green): cargo fmt --check; git diff --check main...HEAD;
cargo clippy --all-targets --all-features -- -D warnings; cargo test -q
(103 lib incl. new emit_websocket_auth_failed_writes_event_to_db + 5 bin + 3
bitget_demo network); pytest 55. Live daemon smoke: 0 websocket_auth_failed
events, 0 post-ready session errors, no tight-reconnect flood.
Demo-only invariant holds; Telegram remote controls still refused.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ile/private-WS Round-6 codex finding (Important, M4 spec violation): M4 spec: "Telegram delivery failure must not block execution, order management, or reconcile" (design line 141). send_telegram did Client::new().post(...).send().await with no request timeout. reqwest's default has no overall timeout, so a hung/slow Telegram POST would block every direct awaiter — the reconcile pass (reconcile.rs:320/468/596/...) and the private-WS auth-failure helper (daemon.rs emit_websocket_auth_failed). A stuck loop is exactly what the spec forbids. Minimal fix in ONE place: wrap the POST in tokio::time::timeout(3s). A timeout returns Err, which every caller already swallows (.ok() / let _ =), so Telegram being down degrades to "no notification" instead of "stuck loop". No per-caller timeouts scattered. tokio "time" feature is already enabled. Test: send_telegram_short_circuits_without_hanging pins the non-network short-circuit paths (suppressed kind, missing token, partial creds) return promptly — the most common callers. The 3s send() bound is a one-line constant verified by inspection + live daemon smoke (0 telegram errors, daemon starts clean); the real-network timeout can't be unit-tested without an injectable URL (the host is hard-coded), and abstracting the URL would be over-engineering a one-line fix. Verification (all green): cargo fmt --check; git diff --check main...HEAD; cargo clippy --all-targets --all-features -- -D warnings; cargo test -q (104 lib incl. new test + 5 bin + 3 bitget_demo network); pytest 55; live daemon smoke. Demo-only invariant holds; Telegram remote controls still refused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR evolves the demo executor from a one-shot runner into a long-running demo-only daemon, adding WS-driven caching/state updates, a periodic REST reconcile loop, and read-only Telegram status queries while preserving the existing --once flow.
Changes:
- Added
--daemonmode with public/private WS loops, reconcile signaling, bounded-runtime support for tests, and intent-loop orchestration. - Expanded Bitget WS parsing (login ack/auth errors, positions/account, symbol resolution) and added SQLite “refresh-from-WS” helpers that preserve reconcile-owned identity/ownership fields.
- Introduced read-only Telegram query formatting and bounded Telegram notification sending to avoid blocking core execution.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_executor_integration.py | Adds an integration test for daemon-mode processing and live-mode rejection. |
| crates/executor/tests/bitget_demo.rs | Uses shared lot-rounding (format_size) to avoid FP boundary rejects in demo close sizing. |
| crates/executor/src/types.rs | Extends private WS update types with login ack/auth error and optional account snapshot parsing. |
| crates/executor/src/telegram_query.rs | Implements SQLite-backed, read-only Telegram command responses with unit tests. |
| crates/executor/src/risk.rs | Refines risk gating to allow de-risking actions even when open-only gates (stale/ready/caps) would block. |
| crates/executor/src/notify.rs | Adds a 3s timeout around Telegram send to prevent hangs from blocking awaited call sites. |
| crates/executor/src/main.rs | Refactors CLI parsing to support --daemon and bounded runtime, and makes parsing unit-testable. |
| crates/executor/src/lib.rs | Exposes new daemon and telegram_query modules. |
| crates/executor/src/executor.rs | Adds daemon-friendly intent processing, market freshness gating behavior, infra-error fail-safe, and FP-safe lot rounding. |
| crates/executor/src/db.rs | Adds order/position refresh helpers for private-WS updates that preserve reconcile/executor-owned fields. |
| crates/executor/src/daemon.rs | Implements the long-running daemon loop, WS tasks, reconcile/private-ready signals, and Telegram polling loop. |
| crates/executor/src/config.rs | Updates demo-only rejection messaging and test naming. |
| crates/executor/src/bitget.rs | Adds WS login/subscribe message builders, improves private WS parsing (ack/errors/positions/account), and aligns verify with parser. |
| crates/executor/Cargo.toml | Enables Tokio signal feature for SIGTERM/SIGINT handling. |
| Cargo.lock | Updates lockfile for new Tokio signal dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+88
to
+93
| fn defer_event_recently_emitted(last_emit_ms: Option<i64>, now_ms: i64) -> bool { | ||
| match last_emit_ms { | ||
| Some(last) => now_ms - last < DEFER_EVENT_WINDOW_MS, | ||
| None => false, | ||
| } | ||
| } |
| assert order_count >= 1, "expected at least one demo order to be attempted" | ||
| assert event_count >= 1, "daemon must record startup + reconcile + intent events" | ||
| assert false_fills == 0, "an order must not be marked filled with no fill" | ||
| assert "daemon" in result.stdout or result.stderr == "" |
AaronL725
added a commit
that referenced
this pull request
Jul 4, 2026
… daemon Squashed in PR #1 earlier; re-merging with --no-ff to preserve the 24 granular commits (feat/fix across the M4 daemon work + 6 rounds of codex review). Same tree content as the prior squash merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upgrades the M3 one-shot demo executor (
prodigy-executor --once, preserved) into a demo-only long-running daemon (prodigy-executor --daemon): public WS maintains the MarketCache, private WS updates state/SQLite, an intent loop consumes SQLite pendingtrade_intents, periodic REST reconcile keeps exchange truth, and read-only Telegram queries (/status /positions /orders /pnl /risk) surface state.Architecture
MarketCache(fresh best bid/ask; stale-gates opens)pendingintents: freshness gate → account snapshot → risk check → maker→retry-maker→taker state machine with cancel-confirm + over-fill guardsReconcileSignal(reconnect triggers reconcile),PrivateStateReady(opens gated until private WS is logged in + acked)Six rounds of codex review — all findings addressed
0.009999999999999995) floored to 0, sizing closes to 0; fixed with an epsilon in lot-count space, routed through the sharedformat_size.true; now a real signal, gated on the login ack (incl. Bitget's numericcode:0), withwebsocket_auth_failedon auth failure.accept_intent(only matchespending) wedged the intent; one root-cause guard in the shared caller flips it tofailed.executor_state.Money-path safety
filled; phantom demo book → honestfailedterminal stateConstraints honored (Ponytail full)
No Redis/Kafka/FastAPI/actor/event-bus, no second execution service. New dep: tokio
signalfeature only. Reused M3 executor/SQLite/Bitget/Telegram.Verification — all gates green
cargo fmt --check✅git diff --check main...HEAD✅cargo clippy --all-targets --all-features -- -D warnings✅cargo test -q— 104 lib + 5 bin + 3 bitget_demo network ✅pytest— 55 ✅Invariants
TradingMode::Liverejected); demo WS host enforced/stop /resume /close_all)🤖 Generated with Claude Code