diff --git a/.gitignore b/.gitignore index 5313a0c8..5568869b 100644 --- a/.gitignore +++ b/.gitignore @@ -73,6 +73,7 @@ fuckyeah.md .anustimes/ .brutal_reviews/ .pr-reviews/ +.deep-qa/ # Local orchestration scaffolding — never committed .plan-and-delegate/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a101881..3d129856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,32 @@ The project follows [Semantic Versioning](https://semver.org/): patch = bug fixe ## [Unreleased] +### Fixed + +- **Broker symbol suffixes are no longer appended blindly** (`mt5api/backtest/handler.py` `_normalize_symbol`, new `mt5api/symbol_cache.py`). `symbol_suffix` used to be appended to every `[Tester].Symbol` that did not already end with it, but brokers rarely suffix their whole book — Eightcap Global carries 56 suffixed FX pairs (`EURUSD.i`) alongside 785 bare metals, indices and crypto (`XAUUSD`, `BTCUSD`, `ASX200`), so every non-FX backtest there asked the tester for a symbol that does not exist and came back empty. The suffix is now skipped when the broker's symbol list has the bare name and lacks the suffixed one. Brokers that carry both forms (BlackBull lists `AUDUSD` *and* `AUDUSDp`) still get the suffix, so `symbol_suffix: p` keeps meaning "use the prime variant". + + A `mode: backtest` terminal never attaches the MT5 SDK and `Bases//symbols/*.dat` is encrypted, so there is no way to enumerate symbols at INI-build time. `GET /symbols` (unfiltered only) now persists what it saw to `/mt5api-symbols.json`, and the INI builder reads that back. With no cache the old append-always behaviour is used unchanged, so this cannot regress a terminal that has never been primed. Tests: `tests/test_symbol_suffix_remap.py`. + + The cache has a finite trust window, enforced inside `load()` itself: past `symbol_cache_max_age` (default 7 days; `SYMBOL_CACHE_MAX_AGE` in the environment), or with a missing/malformed `updated` stamp, it counts as absent and the append-always fallback applies. Without that, the cache was authoritative forever — a broker moving a symbol between bare and suffixed stayed wrongly normalized until someone happened to call `GET /symbols`. + +- **The documented cache-priming request no longer wedges the backtest terminal.** `GET /symbols` is `@with_mt5`, so calling it on a `mode: backtest` terminal — which never attaches the SDK at startup — fell through `ensure_initialized()` into a full `mt5.initialize()`, spawning `terminal64.exe` and holding the tester's single-instance data-dir lock for the rest of that terminal's life. Every backtest submitted to it afterward exited clean with an empty report. `GET /symbols` is now refused with 409 on a `mode: backtest` terminal before any SDK call. The new `POST /symbols/import` (`mt5api/handlers/symbols.py`) is the safe replacement: it writes a caller-supplied symbol list straight to `symbol_cache` and never calls `mt5.*` or acquires the MT5 lock, so it is the priming path for a broker/account that has no live terminal to call `GET /symbols` on. Test: `tests/test_symbol_import.py` asserts against the real Flask app that neither route reaches `ensure_initialized()` when `MODE == "backtest"`. + + That refusal is also no longer queued behind the global MT5 lock. `@with_mt5` enters `session()` and blocks on `_mt5_lock` for up to `SESSION_ACQUIRE_TIMEOUT` *before* the handler body runs, so a backtest terminal already sitting behind a stuck SDK request answered with a 503 a minute later instead of the 409 — a refusal that needs no SDK at all. `list_symbols` now runs the `MODE` check undecorated and delegates the live path to a `@with_mt5` `_list_symbols_live`, so only the branch that touches the SDK waits for it. Tests hold `_mt5_lock` from another thread and assert the backtest request still returns 409 promptly without acquiring it, and that the live listing still does wait. + + `config/config.yaml.example` documented the old, wedging flow — prime and refresh with `GET ///symbols` — which a `mode: backtest` reader now cannot follow at all. It carries the `POST /symbols/import` workflow instead, with a concrete request body and where to source the list (a live terminal on the same broker/account, or the broker's own documentation). Covered behaviorally rather than by asserting on the file's text: the test walks the documented steps against the real app and asserts the outcome the comment promises — that the INI builder stops appending the suffix to a symbol the broker carries bare. + +- **`POST /symbols/import` now bounds what it will accept** (`mt5api/handlers/symbols.py`, `mt5api/config.py`). The endpoint had no input limits at all: one `symbols` item of 2,097,153 bytes was accepted with a `200` and persisted into `/mt5api-symbols.json`, a file the backtest INI builder reads and parses on every run inside a fixed-disk Windows VM. Three per-request caps now apply, each configurable at the top level of `config/config.yaml` or under the same name uppercased in the environment, and each clamped to a minimum of 1 rather than raising — config.py is imported by the whole API, so a typo in one endpoint's tuning value must not stop trading, nor disable the only offline path a `mode: backtest` terminal has for priming its cache. + + `symbol_import_max_body_bytes` (default 2 MiB) is checked against the declared `Content-Length` **before** `request.get_json()` runs; parsing first would already have paid the memory cost the cap exists to prevent, so the placement is the point and is covered by a test that sends an over-cap `Content-Length` with a body that is not valid JSON — only an ordering where the gate runs first can answer `413`. A body with no declared length but a transfer encoding cannot be bounded up front and is refused with `411` rather than waved past the cap; a request with no body at all carries no transfer encoding and keeps the `400` it has always had. `symbol_import_max_symbols` (default 20000, ~20x the widest broker book seen in this fleet) is counted on the raw array before deduplication, because the cap's job is to bound the strip/dedupe/sort pass. `symbol_import_max_symbol_length` (default 64, double MT5's own 31-character symbol-name limit) is measured on the *normalized* name, so whitespace padding can neither fail a legal name nor smuggle an illegal one through. Count and length violations return `400`; nothing is written to the cache when a request is refused. Documented in `config/config.yaml.example` and `docs/market-data.md`; tests in `tests/test_symbol_import.py` cover the max and max-plus-one boundary of each cap, the pre-parse ordering, and the reported 2 MB payload at shipped defaults. + +- **Every request body is bounded now, not just `POST /symbols/import`'s** (`mt5api/server.py`, `mt5api/config.py`). Capping the one endpoint that was reported left six others parsing whatever arrived — `POST /orders`, `PUT /orders/`, `PUT` and `DELETE /positions/`, `POST /symbols//rates/ta`, `POST /backtest/build-ini` and `/backtest/build-set` — and a per-endpoint check only bounds the endpoint someone remembered, so the next route added is the next hole. The cap now lives once in the `before_request` hook every route already passes through: `max_request_body_bytes` (4 MiB) for anything that is not a file upload, `max_upload_body_bytes` (25 MiB) for multipart, since `POST /backtest` carries a compiled `.ex5` plus its `.set` and `.ini`. The upload cap matches the `client_max_body_size` nginx already enforces in front of the API, so reaching the port directly answers the same as coming through the proxy. Both are refused with 413 from the declared `Content-Length` before the body is parsed, and an endpoint with a tighter cap of its own still reports that one, because it is checked inside the handler. Tests in `tests/test_request_body_cap.py` cover every body-reading route, the pre-parse ordering on each, both caps' boundaries, and that the endpoint-specific cap still wins. + +- **The unified `endpoints` MCP tool is tested through the protocol now, not by parsing its source.** `tests/test_mcp_tool_parity.py` asserted the route catalog by AST-parsing `_ROUTE_CATALOG` out of `mcpunifier/mcp_server.py` — an implementation-source assertion that stays green while MCP tool registration, transport or response serialization breaks. It is replaced by black-box tests in `tests/integration/test_mcpunifier.py`, which boot the shipped unifier image, call the public `endpoints` tool over MCP, and compare the routes it returns against the real `mt5api.server.app.url_map`. Demonstrated by renaming the tool's response key: the old test still passed, the new ones fail. The comparison is equality in both directions — a missing entry hides a real route from agents, a surplus entry sends them at a 404 — with Werkzeug's converter prefix stripped from the Flask side (`` against the catalog's ``), since the converter is a routing-layer detail with no presence on the wire while the parameter name is what an agent reads. `requirements-test.txt` grows `flask`, `flask-compress` and `psutil` so the host-side integration run can import the real router instead of a hand-copied list. + +- **Terminal journals are now bounded by size as well as age** (`scripts/rotate-logs.sh`). The retention window added in v4.13.1 deletes dated journals once they are `RETAIN_DAYS` old, which cannot help with the actual hazard: a high-frequency grid strategy logs every order placement, modification and cancellation, so one backtest writes tens of gigabytes into *today's* journal — the disk fills a week before that file is even eligible for the age pass. Any journal still inside the retention window is now truncated once it exceeds `MAX_LOG_BYTES` (default 2 GiB), skipping anything written within `IDLE_MINUTES` (default 30) so a running backtest never loses its own diagnostics. Truncated in place rather than deleted, because `terminal64.exe` holds the journal open and unlinking the inode would leave it writing to a deleted file with the space unreclaimed until it exited. Both knobs are set on the `log-rotator` service in both the generated and single-VM Compose files alongside the existing retention window, and validated at startup like `RETAIN_DAYS`. Tests in `tests/test_rotate_logs.py` cover each journal location, inode preservation, the active-run exemption, files under the cap, and startup validation. + + Sizing uses `stat` rather than `wc -c`: busybox `wc` reads the whole file to count bytes — roughly 18s per 3 GB journal in the `alpine:3.20` image the sidecar runs in, repeated every `INTERVAL` on precisely the files the cap exists for. The mtime is preserved across the truncation, so a just-emptied journal cannot outrank the journal a running job is writing in `_tail_dir_log`'s newest-by-mtime selection and blank out `GET /backtest//tail`. + ## [v4.13.1]: 2026-09-10 ### Changed diff --git a/config/config.yaml.example b/config/config.yaml.example index 79bddb69..1255476a 100644 --- a/config/config.yaml.example +++ b/config/config.yaml.example @@ -66,6 +66,85 @@ accounts: # See docs/multi-vm-setup.md for multi-VM setup. # symbol_suffix: optional broker-specific suffix appended to [Tester].Symbol # when missing. Examples: "p", ".p", "-mini". Use "" for no suffix. +# Most brokers suffix only part of their book — Eightcap Global has 56 +# suffixed FX pairs (EURUSD.i) and 785 bare metals/indices/crypto (XAUUSD, +# ASX200) — so the suffix is skipped for any symbol the broker carries bare +# and does NOT carry suffixed. That check needs a symbol list, which a +# backtest-mode terminal cannot fetch (no SDK attached), so it reads the +# cache at /mt5api-symbols.json. Until that cache exists the +# suffix is appended unconditionally, as it always was. +# +# Priming the cache, by mode: +# mode: live — GET ///symbols writes it as a side +# effect of the unfiltered listing. Nothing else to do. +# mode: backtest — GET /symbols is REFUSED with 409 there: it would +# attach the SDK, spawn terminal64.exe and hold the +# tester's single-instance data-dir lock, leaving the +# next backtest run with an empty report. Prime it with +# POST /symbols/import instead, which touches no SDK: +# +# curl -X POST -H "Authorization: Bearer $MT5_API_TOKEN" \ +# -H 'Content-Type: application/json' \ +# -d '{"symbols": ["EURUSD", "GBPUSD", "XAUUSD"]}' \ +# http://127.0.0.1:8888///symbols/import +# +# Source that list from a live terminal on the SAME broker/account — +# `curl $MT5_API_URL/symbols` against it returns exactly the JSON array +# this endpoint accepts under the "symbols" key — or from the broker's own +# symbol documentation. +# +# The cache is trusted for symbol_cache_max_age (default 7d, also +# SYMBOL_CACHE_MAX_AGE in the environment); past that it counts as absent +# again — refresh it the same way you primed it — so a broker moving a +# symbol between bare and suffixed cannot be papered over by a years-old +# list. +# +# symbol_cache_max_age is read at the TOP LEVEL of this file, not per +# terminal — it applies to every terminal. Uncomment it here, at the same +# indentation as `terminals:` below; setting it inside a terminal entry is +# silently ignored. +# symbol_cache_max_age: "7d" + +# Per-request caps on POST /symbols/import. Also read at the TOP LEVEL, and +# also settable in the environment under the uppercase name. Each is clamped to +# a minimum of 1 rather than raising, so a typo here cannot take the only +# priming path a backtest terminal has offline. +# +# symbol_import_max_body_bytes (default 2097152, 2 MiB) — an oversized body is +# refused with 413 from its declared Content-Length, before any of it is +# parsed. A request with no Content-Length at all (chunked) is refused with +# 411. Sized to admit the largest legitimate payload, which is the count cap +# times the length cap plus JSON quoting (~1.4 MB). +# symbol_import_max_symbols (default 20000) — maximum entries in the 'symbols' +# array, counted before deduplication; over the cap is a 400. No MT5 broker +# publishes anything close: the widest books seen here are low thousands. +# symbol_import_max_symbol_length (default 64) — maximum characters per symbol, +# measured on the NORMALIZED (whitespace-stripped) name; over the cap is a +# 400. MT5 itself caps a symbol name at 31 characters, so this is double the +# platform's own limit. +# symbol_import_max_body_bytes: 2097152 +# symbol_import_max_symbols: 20000 +# symbol_import_max_symbol_length: 64 + +# Global per-request body caps, applied to EVERY route in one place +# (server.py's before_request hook) rather than endpoint by endpoint. A +# per-endpoint cap only bounds the endpoint someone remembered to add one to; +# this bounds the rest, including routes added later. An endpoint with a +# tighter cap of its own — symbol_import_max_body_bytes above — still reports +# that one, because it is checked inside the handler. +# +# Both are refused with 413 from the declared Content-Length, before the body +# is parsed, and both take the uppercase name in the environment. +# +# max_request_body_bytes (default 4194304, 4 MiB) — everything that is not a +# file upload. Well above the largest legitimate JSON this API takes. +# max_upload_body_bytes (default 26214400, 25 MiB) — multipart, i.e. +# POST /backtest, which carries a compiled .ex5 plus its .set and .ini. +# Matched to the client_max_body_size nginx already enforces in front of +# this API, so reaching the port directly gives the same answer as coming +# through the proxy. +# max_request_body_bytes: 4194304 +# max_upload_body_bytes: 26214400 terminals: - broker: ftmo account: tenkchallenge diff --git a/docker-compose.yml.example b/docker-compose.yml.example index fe217220..5c580c5b 100644 --- a/docker-compose.yml.example +++ b/docker-compose.yml.example @@ -87,6 +87,8 @@ services: LOG_DIR: /logs TERMINALS_DIR: /terminals RETAIN_DAYS: "7" + MAX_LOG_BYTES: "2147483648" + IDLE_MINUTES: "30" INTERVAL: "3600" volumes: - ./data/shared/logs:/logs diff --git a/docker-compose.yml.j2 b/docker-compose.yml.j2 index 7d115ca7..fefa451c 100644 --- a/docker-compose.yml.j2 +++ b/docker-compose.yml.j2 @@ -116,6 +116,8 @@ services: LOG_DIR: /logs TERMINALS_DIR: /terminals RETAIN_DAYS: "7" + MAX_LOG_BYTES: "2147483648" + IDLE_MINUTES: "30" INTERVAL: "3600" volumes: - /data/mt5-shared/logs:/logs diff --git a/docs/market-data.md b/docs/market-data.md index 2150b282..3d8bf3a3 100644 --- a/docs/market-data.md +++ b/docs/market-data.md @@ -15,7 +15,8 @@ Find symbols, inspect the contract details that brokers love making weird, pull | Method | Endpoint | Description | | ------ | ------------------------ | ----------------------------------------- | -| GET | `/symbols` | List symbols (`?group=*USD*`) | +| GET | `/symbols` | List symbols (`?group=*USD*`). **Refused with 409 on a `mode: backtest` terminal** — it never attaches the SDK; see below. | +| POST | `/symbols/import` | Prime the symbol cache from a JSON body without touching the SDK — the safe path on a `mode: backtest` terminal. | | GET | `/symbols/:symbol` | Symbol details | | GET | `/symbols/:symbol/tick` | Latest tick | | GET | `/symbols/:symbol/rates` | OHLCV candles (`?timeframe=H1&count=100`, `?timeframe=H1&from=&count=-100`, or `?timeframe=H1&from=&to=`) | @@ -28,6 +29,44 @@ Find symbols, inspect the contract details that brokers love making weird, pull ["EURUSD", "GBPUSD", "ADAUSD", "BTCUSD", "..."] ``` +On success (unfiltered, i.e. no `?group=`) this also persists the list to that +terminal's symbol cache, which the backtest INI builder reads to decide +whether a broker's `symbol_suffix` applies (see [Backtesting](backtesting.md)). +`GET /symbols` calls the MT5 SDK, so on a `mode: backtest` terminal — which +never attaches the SDK — it is refused with 409 instead of triggering a full +`mt5.initialize()` that would spawn `terminal64.exe` and hold the tester's +single-instance lock, leaving the next backtest run with an empty report. + +**POST `/symbols/import`** — prime a terminal's symbol cache directly, with no +MT5 SDK call at all. This is the only supported way to prime a `mode: backtest` +terminal: get the list from a live terminal on the same broker/account (`GET +/symbols` there) or from the broker's own documentation, then post it here. + +```bash +curl -X POST -H "Authorization: Bearer $MT5_API_TOKEN" \ + -H 'Content-Type: application/json' \ + -d '{"symbols": ["EURUSD", "GBPUSD", "XAUUSD"]}' \ + "$MT5_API_URL/symbols/import" +``` + +```json +{"imported": 3} +``` + +The body is bounded on three axes, each configurable at the top level of +`config/config.yaml` or under the same name uppercased in the environment: + +| Setting | Default | Enforced | +| ------- | ------- | -------- | +| `symbol_import_max_body_bytes` | `2097152` (2 MiB) | `413` from the declared `Content-Length`, before the body is parsed. A request with no `Content-Length` at all (chunked) is refused with `411`. | +| `symbol_import_max_symbols` | `20000` | `400` — entries in `symbols`, counted before deduplication. | +| `symbol_import_max_symbol_length` | `64` | `400` — characters per symbol, measured on the whitespace-stripped name. | + +The defaults sit far above any real broker book — the widest seen in this fleet +is 841 symbols, and MT5 itself caps a symbol name at 31 characters — so they +only fire on input that was never going to be a usable symbol list. Nothing is +written to the cache when a request is refused. + **GET `/symbols/:symbol`** — full symbol info: ```json diff --git a/docs/operations.md b/docs/operations.md index c16226ea..a153e46d 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -490,4 +490,19 @@ Override defaults via `docker-compose.yml`: Truncation is in-place (the archive is a copy, then the original is `:>`-truncated) so the Python API's open log handle keeps writing without reopening. +#### Size cap on terminal journals + +Retention alone does not bound the terminal journals, because it will not consider a journal until it is `RETAIN_DAYS` old. A high-frequency grid strategy logs every order placement, modification and cancellation, so a single backtest can write tens of gigabytes into *today's* journal — and the disk fills a week before that file is even eligible for the age pass. + +So a journal still inside the retention window is reclaimed by size instead, once it goes quiet: + +- `MAX_LOG_BYTES` (default `2147483648`, 2 GiB) - truncate any single journal larger than this +- `IDLE_MINUTES` (default `30`) - never touch a journal written more recently + +Both are set on the `log-rotator` service in `docker-compose.yml` alongside `RETAIN_DAYS`. All three are validated at startup: a non-positive or non-numeric value exits non-zero rather than silently falling back to a default and quietly pruning on the wrong terms. + +**What this does not do:** `IDLE_MINUTES` deliberately exempts a journal a backtest is actively writing, because truncating it destroys the diagnostics for the run producing it. So the cap reclaims the space *after* the run goes quiet — it will not stop a single runaway backtest filling the disk while it is still going. If that is your failure mode, the lever is the strategy's own logging, or a larger volume; the rotator only guarantees the space comes back afterwards instead of never. A journal over the cap and still active is logged as `over cap but still active, left alone` on each pass, so it is visible in `docker compose logs log-rotator` rather than silently skipped. + +Oversized journals are truncated in place rather than deleted: the terminal holds them open, so unlinking the inode would leave `terminal64.exe` writing to a deleted file and the space would not come back until it exited. The same exclusions as the age pass apply — MQL5 expert logs, MetaEditor logs, reports and backtest jobs are never touched. + When shit breaks, check these first. diff --git a/mcpunifier/mcp_server.py b/mcpunifier/mcp_server.py index dcf445cd..98e4eab8 100644 --- a/mcpunifier/mcp_server.py +++ b/mcpunifier/mcp_server.py @@ -39,6 +39,7 @@ ("POST", "/terminal/restart"), ("GET", "/account"), ("GET", "/symbols"), + ("POST", "/symbols/import"), ("GET", "/symbols/"), ("GET", "/symbols//tick"), ("GET", "/symbols//rates"), diff --git a/mt5api/backtest/handler.py b/mt5api/backtest/handler.py index 895ed5b3..c7afa10c 100644 --- a/mt5api/backtest/handler.py +++ b/mt5api/backtest/handler.py @@ -28,6 +28,7 @@ from flask import Response, abort, jsonify, request, send_file +from mt5api import symbol_cache from mt5api.backtest import cache_parser, ini_builder, jobs, optimization_parser, set_builder from mt5api.config import ( ACCOUNT, @@ -214,6 +215,25 @@ def _normalize_symbol(parser): return remapped = f"{symbol}{suffix}" + + # Few brokers suffix their whole book. Eightcap Global suffixes 56 FX pairs + # (EURUSD.i) and leaves 785 metals/indices/crypto bare (XAUUSD, ASX200), so + # appending unconditionally asks the tester for a symbol that cannot exist + # and the run comes back empty. Only skip the remap on positive evidence: + # a cached symbol list that HAS the bare name and LACKS the suffixed one. + # Absent cache => append, exactly as this did before the cache existed. + known = symbol_cache.load(TERMINAL_DIR) + if known is not None and remapped not in known and symbol in known: + log.info( + "backtest symbol kept unsuffixed broker=%s account=%s %s " + "(broker has no %s)", + BROKER, + ACCOUNT, + symbol, + remapped, + ) + return + tester["Symbol"] = remapped log.info( "backtest symbol remap broker=%s account=%s %s -> %s", @@ -254,6 +274,52 @@ def _tail(text, limit=DIAGNOSTIC_TAIL_CHARS): return text if len(text) <= limit else text[-limit:] +#: How much of a log file a tail is allowed to touch. A terminal writing a +#: multi-year backtest grows its Tester log to gigabytes WHILE the run is +#: polled, so tailing must stay O(tail), never O(file) — a whole-file read of +#: one of those seizes the process for a minute per call (whole-file bytes + +#: a decoded str copy + a splitlines() list, all while holding the GIL), which +#: starves every other request including /ping and the container healthcheck. +TAIL_MAX_BYTES = 256 * 1024 + + +def _read_tail_text(path, max_bytes=TAIL_MAX_BYTES): + """Decode at most the final ``max_bytes`` of ``path``. + + Encoding is sniffed from the file's first two bytes: a BOM means + UTF-16-LE (how MT5 writes its logs), and so does a NUL second byte — + UTF-16-LE of any ASCII-leading text, which covers BOM-less UTF-16 logs. + Anything else decodes as UTF-8 (run.log). The read then seeks to the + final window, aligned to a 2-byte boundary so UTF-16 code units stay + intact. When the window starts mid-file, everything up to the first + newline is dropped — a truncated first line reads as garbage, and a tail + endpoint never needs it. + """ + try: + with open(path, "rb") as handle: + head = handle.read(2) + utf16 = head == b"\xff\xfe" or (len(head) == 2 and head[1] == 0) + size = handle.seek(0, os.SEEK_END) + offset = max(0, size - max_bytes) + if utf16 and offset % 2: + offset -= 1 + handle.seek(offset) + raw = handle.read() + except OSError: + return "" + + if utf16: + text = raw.decode("utf-16-le", errors="replace") + else: + text = raw.decode("utf-8", errors="replace") + + if offset > 0: + first_break = text.find("\n") + if first_break != -1: + text = text[first_break + 1:] + return text + + def _tail_terminal_log(lines=20): """Tail of the terminal's most recently written run log. @@ -286,12 +352,7 @@ def _tail_terminal_log(lines=20): except OSError: return "" - latest_path = newest.path - try: - with open(latest_path, "r", encoding="utf-16-le", errors="replace") as handle: - content = handle.read() - except OSError: - return "" + content = _read_tail_text(newest.path) tail_lines = [line.strip() for line in content.splitlines() if line.strip()] if not tail_lines: @@ -832,19 +893,42 @@ def get_log(job_id): def _tail_dir_log(log_dir, lines): - """Return (path_used, last N non-empty lines) from the newest .log in log_dir.""" + """Return (path_used, last N non-empty lines) from the newest .log in log_dir. + + Newest by MODIFICATION TIME, excluding `metaeditor.log` — the same two + rules `_tail_terminal_log` already applies, and for the same reason: the + logs are `.log` files plus a `metaeditor.log` that sorts after all + of them ("m" > "2") and never changes, so an alphabetical pick returned a + stale compile log instead of the run being polled. + + Bounded read (`_read_tail_text`): this runs on the live /tail endpoint, + which the backend polls once a minute for every running job, against a + Tester log that reaches gigabytes mid-run. The prior whole-file read took + 45-65 s per call on such a log, starving every thread in the process — + /ping and the container healthcheck included — which made a healthy + terminal look wedged from the outside. + """ if not os.path.isdir(log_dir): return None, "" try: - candidates = sorted(f for f in os.listdir(log_dir) if f.lower().endswith(".log")) + candidates = [ + entry + for entry in os.scandir(log_dir) + if entry.is_file() + and entry.name.lower().endswith(".log") + and entry.name.lower() != "metaeditor.log" + ] except OSError: return None, "" if not candidates: return None, "" - path = os.path.join(log_dir, candidates[-1]) - content = _read_text_best_effort(path) + try: + newest = max(candidates, key=lambda entry: entry.stat().st_mtime) + except OSError: + return None, "" + content = _read_tail_text(newest.path) tail_lines = [ln.strip() for ln in content.splitlines() if ln.strip()] - return path, "\n".join(tail_lines[-lines:]) + return newest.path, "\n".join(tail_lines[-lines:]) def get_tail(job_id): @@ -865,11 +949,13 @@ def get_tail(job_id): if job is None: return jsonify({"error": f"Backtest job not found: {job_id}"}), 404 - # run.log — stdout/stderr of terminal64.exe (sparse but useful on errors) + # run.log — stdout/stderr of terminal64.exe. Usually sparse, but "usually" + # is not a bound: a chatty terminal can grow it without limit, and this + # endpoint is polled — same O(tail) rule as _tail_dir_log. run_log = "" log_path = job.get("logPath") if log_path: - content = _read_text_best_effort(log_path) + content = _read_tail_text(log_path) run_tail = [ln.strip() for ln in content.splitlines() if ln.strip()] run_log = "\n".join(run_tail[-50:]) diff --git a/mt5api/config.py b/mt5api/config.py index 1338f4c5..db0e0eda 100644 --- a/mt5api/config.py +++ b/mt5api/config.py @@ -251,6 +251,84 @@ def load_terminal_config(): MODE = str(_MODE_RAW).strip().lower() or "live" if MODE not in ("live", "backtest"): MODE = "live" +def _symbol_cache_max_age(): + """Finite trust window for the persisted broker symbol list, in seconds. + + Clamped rather than raised on a bad value - config.py is imported by the + whole API, and a typo here must not stop trading. The floor keeps "0" or a + negative from making every cache read stale and silently re-enabling the + append-always behaviour the cache exists to fix. + """ + raw = os.environ.get("SYMBOL_CACHE_MAX_AGE") or load_yaml_config().get( + "symbol_cache_max_age" + ) + default = 7 * 24 * 3600 + if raw in (None, ""): + return default + try: + parsed = parse_duration_to_seconds(raw) + except (TypeError, ValueError): + return default + return max(60, parsed or default) + + +SYMBOL_CACHE_MAX_AGE_SECONDS = _symbol_cache_max_age() + + +def _positive_int_setting(env_name, yaml_key, default): + """A positive integer setting, clamped rather than raised on a bad value. + + Same call as every other numeric setting in this module: config.py is + imported by the whole API, so a typo in one endpoint's tuning value must + not stop trading and backtesting. The floor keeps a bad value (0, a + negative, "none") from silently disabling /symbols/import outright, which + is the only way to prime a mode: backtest terminal's symbol cache. + + Environment overrides the top-level config.yaml key, matching + _symbol_cache_max_age above. + """ + raw = os.environ.get(env_name) or load_yaml_config().get(yaml_key) + if raw in (None, ""): + return default + try: + value = int(str(raw).strip()) + except (TypeError, ValueError): + return default + return max(1, value) + + +# Per-request caps for POST /symbols/import. Without them one authenticated +# request could hand the JSON parser an unbounded body, then persist the result +# into /mt5api-symbols.json — a file the backtest INI builder reads +# and parses on every run, inside a Windows VM with a fixed disk. All three are +# checked in handlers/symbols.py BEFORE the resource they bound is spent. +# +# SYMBOL_IMPORT_MAX_BODY_BYTES (2 MiB): refused from the declared +# Content-Length, before request.get_json() pulls the body into memory. Sized +# to comfortably admit the largest legitimate payload — the count cap times +# the length cap plus JSON quoting is ~1.4 MB — while keeping a single +# request inside what the VM can buffer without paging. +# SYMBOL_IMPORT_MAX_SYMBOLS (20000): no MT5 broker publishes anything close. +# The widest books seen in this fleet are low thousands (Eightcap Global: +# 841), so this is ~20x the real maximum: generous enough that no broker's +# full book is ever refused, small enough to bound the dedupe/sort and the +# cache file. +# SYMBOL_IMPORT_MAX_SYMBOL_LENGTH (64): MT5 itself caps a symbol name at 31 +# characters (CustomSymbolCreate), and real broker names run to ~15 +# ("USDCNH.raw_ecn"). Double the platform's own limit leaves room for any +# suffix convention while rejecting the megabyte "symbol" that motivated +# this cap. Applied to the NORMALIZED (stripped) name, which is what +# actually reaches the cache. +SYMBOL_IMPORT_MAX_BODY_BYTES = _positive_int_setting( + "SYMBOL_IMPORT_MAX_BODY_BYTES", "symbol_import_max_body_bytes", 2 * 1024 * 1024 +) +SYMBOL_IMPORT_MAX_SYMBOLS = _positive_int_setting( + "SYMBOL_IMPORT_MAX_SYMBOLS", "symbol_import_max_symbols", 20000 +) +SYMBOL_IMPORT_MAX_SYMBOL_LENGTH = _positive_int_setting( + "SYMBOL_IMPORT_MAX_SYMBOL_LENGTH", "symbol_import_max_symbol_length", 64 +) + SYMBOL_SUFFIX_CONFIGURED = "symbol_suffix" in _terminal_config _SYMBOL_SUFFIX_RAW = _terminal_config.get("symbol_suffix") SYMBOL_SUFFIX = "" if _SYMBOL_SUFFIX_RAW is None else str(_SYMBOL_SUFFIX_RAW) @@ -340,3 +418,32 @@ def load_terminal_config(): "SPECIFIED": mt5.ORDER_TIME_SPECIFIED, "SPECIFIED_DAY": mt5.ORDER_TIME_SPECIFIED_DAY, } + + +# Global per-request body caps, enforced once in server.py's before_request +# hook rather than per handler. +# +# Every JSON endpoint used to hand request.get_json() whatever arrived: +# POST /orders, PUT /orders/, PUT|DELETE /positions/, +# POST /symbols//rates/ta, POST /backtest/build-ini and +# /backtest/build-set all parsed an unbounded body. POST /symbols/import was +# fixed with its own cap, but a per-endpoint check has to be remembered on +# every route added afterwards, and the one that gets forgotten is the hole. +# This is the backstop that cannot be forgotten; endpoints keep their own +# tighter caps where the payload shape justifies one, and those fire first. +# +# MAX_REQUEST_BODY_BYTES (4 MiB) covers everything that is not a file upload. +# Comfortably above the largest legitimate JSON this API takes — the +# /symbols/import cap is 2 MiB and every other body is a handful of KB — so +# it bounds the parser without second-guessing any endpoint. +# MAX_UPLOAD_BODY_BYTES (25 MiB) covers multipart, i.e. POST /backtest, which +# carries a compiled .ex5 plus its .set and .ini. Matched to the +# client_max_body_size nginx already enforces in front of this API, so a +# caller reaching the port directly gets the same answer as one coming +# through the proxy instead of a larger one. +MAX_REQUEST_BODY_BYTES = _positive_int_setting( + "MAX_REQUEST_BODY_BYTES", "max_request_body_bytes", 4 * 1024 * 1024 +) +MAX_UPLOAD_BODY_BYTES = _positive_int_setting( + "MAX_UPLOAD_BODY_BYTES", "max_upload_body_bytes", 25 * 1024 * 1024 +) diff --git a/mt5api/handlers/symbols.py b/mt5api/handlers/symbols.py index c0d55196..46814c89 100644 --- a/mt5api/handlers/symbols.py +++ b/mt5api/handlers/symbols.py @@ -7,7 +7,13 @@ import MetaTrader5 as mt5 +from mt5api import symbol_cache from mt5api.config import ( + MODE, + SYMBOL_IMPORT_MAX_BODY_BYTES, + SYMBOL_IMPORT_MAX_SYMBOL_LENGTH, + SYMBOL_IMPORT_MAX_SYMBOLS, + TERMINAL_DIR, TIMEFRAME_MAP, TIMEFRAME_SECONDS, WICKWORKS_TIMEOUT_SECONDS, @@ -80,13 +86,148 @@ def _parse_anchor(s): return None -@with_mt5 def list_symbols(): + """Refuse on a backtest terminal, otherwise serve the live SDK listing. + + Deliberately NOT @with_mt5: that decorator enters session(), which blocks + on the global MT5 lock for up to SESSION_ACQUIRE_TIMEOUT before the + handler body runs at all. A backtest terminal sitting behind a stuck SDK + request would then answer this refusal with a 503 a minute later instead + of an immediate 409 — the refusal needs no SDK, so it must not queue for + one. The live path keeps the lock, via _list_symbols_live below. + """ + # mode: backtest never attaches the SDK — ensure_initialized() finding no + # terminal_info() would fall through to a full mt5.initialize(), which + # spawns terminal64.exe and holds the tester's single-instance lock. That + # used to be the documented way to prime this terminal's symbol cache; + # instead it wedged the terminal, so it is refused before any SDK call. + # Use POST /symbols/import to prime the cache without touching MT5. + if MODE == "backtest": + return jsonify({ + "error": ( + "GET /symbols is unsafe on a mode:backtest terminal: it " + "would initialize the MT5 SDK and hold the tester's " + "single-instance lock, leaving the next backtest run with " + "an empty report. Use POST /symbols/import to prime the " + "cache without touching MT5." + ), + }), 409 + return _list_symbols_live() + + +@with_mt5 +def _list_symbols_live(): if not ensure_initialized(): return jsonify({"error": "MT5 not initialized"}), 503 group = request.args.get("group") syms = m(mt5.symbols_get, group=group) if group else m(mt5.symbols_get) - return jsonify([s.name for s in syms] if syms else []) + names = [s.name for s in syms] if syms else [] + # Persist only the UNFILTERED list: the backtest INI builder uses this to + # decide a symbol does not exist, and a group-filtered subset would make it + # draw that conclusion about every symbol the filter excluded. + if not group and names: + symbol_cache.save(TERMINAL_DIR, names) + return jsonify(names) + + +def import_symbols(): + """Prime this terminal's symbol cache without touching the MT5 SDK. + + Deliberately NOT @with_mt5 and calls no mt5.* function: this is the safe + priming path for a mode:backtest terminal, which must never need an SDK + request (see list_symbols above). Feed it a symbol list sourced elsewhere + — GET /symbols against a live terminal on the same broker/account, or the + broker's own symbol documentation. + + Bounded on three axes, all configurable (see mt5api/config.py) and all + checked before the resource they bound is spent: the raw body, the number + of entries, and the length of each normalized name. + """ + # ── Body size, BEFORE request.get_json() ────────────────────────── + # get_json() pulls the entire body into memory and builds a Python object + # graph from it, so a check placed after it has already paid the cost the + # cap exists to prevent. Content-Length is the only thing available before + # a single byte is parsed, so that is what this gate reads. + declared = request.content_length + if declared is None and request.headers.get("Transfer-Encoding"): + # No declared length AND a transfer encoding: a streamed body whose + # size is unknowable until it has all been read, so there is nothing + # for this gate to check. Deliberately refused rather than waved + # through — waving it through is precisely the hole the cap closes. + # This endpoint's body is one small JSON object and every real client + # (curl -d, requests, the unifier's `request` tool) sends it with a + # Content-Length; 411 is the code RFC 9110 defines for exactly this. + # (content_length is also None for a request with no body at all, + # which carries no Transfer-Encoding and needs no bounding — it falls + # through to the same 400 an empty body has always produced.) + return jsonify({ + "error": ( + "request must declare a Content-Length; chunked bodies are " + "not accepted on this endpoint" + ), + }), 411 + if declared is not None and declared > SYMBOL_IMPORT_MAX_BODY_BYTES: + log.warning( + "symbols/import: rejected %d-byte body (cap %d)", + declared, SYMBOL_IMPORT_MAX_BODY_BYTES, + ) + return jsonify({ + "error": ( + f"request body is {declared} bytes; this server accepts at " + f"most {SYMBOL_IMPORT_MAX_BODY_BYTES} " + "(SYMBOL_IMPORT_MAX_BODY_BYTES)" + ), + }), 413 + + body = request.get_json(silent=True) + if not isinstance(body, dict): + # get_json(silent=True) happily returns a bare list/number/string for + # syntactically valid JSON that isn't an object — body.get() below + # would then raise AttributeError instead of a clean 400. + return jsonify({"error": "request body must be a JSON object with a 'symbols' array"}), 400 + names = body.get("symbols") + if not isinstance(names, list) or not all(isinstance(n, str) for n in names): + return jsonify({"error": "request body must include a 'symbols' array of strings"}), 400 + # Counted on the raw array, not the deduplicated result: this bounds the + # strip/dedupe/sort work below, which happens before any dedupe can shrink + # the list, and "items in 'symbols'" is what the caller actually sent. + if len(names) > SYMBOL_IMPORT_MAX_SYMBOLS: + log.warning( + "symbols/import: rejected %d symbols (cap %d)", + len(names), SYMBOL_IMPORT_MAX_SYMBOLS, + ) + return jsonify({ + "error": ( + f"'symbols' has {len(names)} entries; this server accepts at " + f"most {SYMBOL_IMPORT_MAX_SYMBOLS} (SYMBOL_IMPORT_MAX_SYMBOLS)" + ), + }), 400 + # This is operator/copy-paste input, unlike the SDK-sourced names + # GET /symbols persists — strip stray whitespace so it cannot silently + # defeat the exact-match lookup in backtest.handler._normalize_symbol. + cleaned = sorted({n.strip() for n in names if n.strip()}) + if not cleaned: + return jsonify({"error": "'symbols' must contain at least one non-empty name"}), 400 + # Measured on the NORMALIZED name — the stripped form is what lands in the + # cache and what the INI builder matches against, so padding a legal name + # with whitespace must not fail, and padding an illegal one must not pass. + too_long = [n for n in cleaned if len(n) > SYMBOL_IMPORT_MAX_SYMBOL_LENGTH] + if too_long: + log.warning( + "symbols/import: rejected %d oversized symbol name(s) (cap %d)", + len(too_long), SYMBOL_IMPORT_MAX_SYMBOL_LENGTH, + ) + return jsonify({ + "error": ( + f"{len(too_long)} symbol name(s) exceed " + f"{SYMBOL_IMPORT_MAX_SYMBOL_LENGTH} characters " + "(SYMBOL_IMPORT_MAX_SYMBOL_LENGTH); longest is " + f"{max(len(n) for n in too_long)}" + ), + }), 400 + if not symbol_cache.save(TERMINAL_DIR, cleaned): + return jsonify({"error": "failed to write symbol cache"}), 500 + return jsonify({"imported": len(cleaned)}) @with_mt5 diff --git a/mt5api/server.py b/mt5api/server.py index 1aca624a..f68e275f 100644 --- a/mt5api/server.py +++ b/mt5api/server.py @@ -1,10 +1,14 @@ import os import time -from flask import Flask, abort, g, request +from flask import Flask, abort, g, jsonify, request from flask_compress import Compress from mt5api.backtest import handler as backtest_handler -from mt5api.config import API_TOKEN +from mt5api.config import ( + API_TOKEN, + MAX_REQUEST_BODY_BYTES, + MAX_UPLOAD_BODY_BYTES, +) from mt5api.handlers import account, history, orders, positions, symbols, terminal from mt5api.logger import log @@ -27,11 +31,52 @@ def _start_request(): g.req_id, request.method, request.full_path, _client_ip(), request.headers.get("User-Agent", "-"), ) - if not API_TOKEN: - return - auth = request.headers.get("Authorization", "") - if auth != f"Bearer {API_TOKEN}": - abort(401) + if API_TOKEN: + auth = request.headers.get("Authorization", "") + if auth != f"Bearer {API_TOKEN}": + abort(401) + return _refuse_oversized_body() + + +def _refuse_oversized_body(): + """Bound the request body before any handler parses it. + + Deliberately here and not in each handler. A per-endpoint cap has to be + remembered on every route added afterwards, and the one that gets forgotten + is the hole — which is how POST /symbols/import came to accept a 2 MB + symbol name while six other JSON routes had no bound at all. An endpoint + that needs a tighter cap still declares one and it fires first, because it + is checked inside the handler; this only catches what nothing else bounded. + + Content-Length is what makes this a PRE-PARSE gate: it is the one thing + known before a byte is read. A body with no declared length is left to the + endpoint (POST /symbols/import refuses it outright) and, in production, to + waitress, which de-chunks and supplies a length before Flask sees the + request at all. + + Runs after the auth check so an unauthenticated caller cannot probe the + limits, and answers JSON because the client treats a non-JSON body as a + broken host. + """ + declared = request.content_length + if declared is None: + return None + multipart = (request.mimetype or "").startswith("multipart/") + limit = MAX_UPLOAD_BODY_BYTES if multipart else MAX_REQUEST_BODY_BYTES + if declared <= limit: + return None + setting = "MAX_UPLOAD_BODY_BYTES" if multipart else "MAX_REQUEST_BODY_BYTES" + log.warning( + "%s rejected %d-byte body on %s %s (cap %d)", + getattr(g, "req_id", "--------"), declared, + request.method, request.path, limit, + ) + return jsonify({ + "error": ( + f"request body is {declared} bytes; this server accepts at most " + f"{limit} ({setting})" + ), + }), 413 @app.after_request @@ -71,6 +116,7 @@ def _end_request(response): # ── Symbols ────────────────────────────────────────────────────── app.get("/symbols")(symbols.list_symbols) +app.post("/symbols/import")(symbols.import_symbols) app.get("/symbols/")(symbols.get_symbol) app.get("/symbols//tick")(symbols.get_tick) app.get("/symbols//rates")(symbols.get_rates) diff --git a/mt5api/symbol_cache.py b/mt5api/symbol_cache.py new file mode 100644 index 00000000..7e80ac88 --- /dev/null +++ b/mt5api/symbol_cache.py @@ -0,0 +1,145 @@ +"""Persisted broker symbol names, so the backtest INI builder can tell a real +symbol from one that still needs the broker's suffix. + +`_normalize_symbol` in mt5api/backtest/handler.py appends `symbol_suffix` to +`[Tester].Symbol`. Brokers rarely suffix everything: Eightcap Global carries +56 suffixed FX pairs (`EURUSD.i`) alongside 785 bare ones (`XAUUSD`, `BTCUSD`, +`ASX200`), so appending unconditionally invents names that do not exist. + +A `mode: backtest` terminal never attaches the MT5 SDK — mt5api/main.py skips +init so the tester can own the data dir — so there is no live way to ask the +broker which names exist at INI-build time. `Bases//symbols/*.dat` is +encrypted and unreadable. This module is the workaround: GET /symbols (on a +terminal that DOES attach the SDK) writes the full list it saw, and the INI +builder reads it back. + +GET /symbols itself calls the SDK (`ensure_initialized()`), so it is refused +on a `mode: backtest` terminal instead of triggering a full `mt5.initialize()` +that would spawn `terminal64.exe` and hold the tester's single-instance lock. +POST /symbols/import (mt5api/handlers/symbols.py) is the safe priming path for +those terminals: it calls symbol_cache.save() directly from a caller-supplied +list and never touches mt5.*. + +The cache is only ever used to SUPPRESS a remap that would invent a symbol. +When it is missing, stale or unreadable the builder falls back to appending, +which is the behaviour that shipped before it existed. Staleness is enforced +in load() itself — a cache older than MAX_AGE_SECONDS, or one without a valid +``updated`` stamp, is treated exactly like no cache — so no caller can forget +the check and treat an ancient list as current. +""" +from __future__ import annotations + +import json +import os +import tempfile +import time + +from mt5api.config import SYMBOL_CACHE_MAX_AGE_SECONDS +from mt5api.logger import log + +CACHE_BASENAME = "mt5api-symbols.json" + +#: Module-level so tests (and an operator poking at a live process) can see +#: and override what load() enforces. Config default: 7 days, via +#: SYMBOL_CACHE_MAX_AGE / symbol_cache_max_age. +MAX_AGE_SECONDS = SYMBOL_CACHE_MAX_AGE_SECONDS + + +def cache_path(terminal_dir): + return os.path.join(terminal_dir, CACHE_BASENAME) + + +def save(terminal_dir, names): + """Persist the broker's full symbol list. Best-effort: never raises. + + Written atomically — the INI builder reads this on every backtest, and a + torn file would silently degrade every remap decision until overwritten. + """ + names = sorted({str(n) for n in names if n}) + if not names: + return False + path = cache_path(terminal_dir) + payload = {"updated": int(time.time()), "symbols": names} + try: + os.makedirs(terminal_dir, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=terminal_dir, prefix=".symbols-", suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(payload, handle) + os.replace(tmp, path) + except BaseException: + # Leaving a .tmp behind on every failed write would slowly fill the + # terminal dir; the replace above is what makes this safe to drop. + try: + os.unlink(tmp) + except OSError: + pass + raise + except OSError as exc: + log.warning("symbol cache write failed at %s: %s", path, exc) + return False + return True + + +def load(terminal_dir, max_age_seconds=None): + """Return the cached symbol set, or None when there is no usable cache. + + None means "no opinion" — callers must treat it as unknown, not empty. + + A cache past ``max_age_seconds`` (default: MAX_AGE_SECONDS) is unusable, + and so is one whose ``updated`` stamp is missing or malformed. This is + what keeps the cache from being AUTHORITATIVE forever: a broker that + moves a symbol between bare and suffixed would otherwise keep being + normalized against a years-old list until someone happened to call + GET /symbols. Stale degrades to the append-always fallback — the + conservative behaviour that shipped before the cache existed — never to + a wrong answer presented as a current one. + """ + limit = MAX_AGE_SECONDS if max_age_seconds is None else max_age_seconds + path = cache_path(terminal_dir) + try: + with open(path, encoding="utf-8") as handle: + payload = json.load(handle) + except FileNotFoundError: + return None + except (OSError, ValueError) as exc: + log.warning("symbol cache unreadable at %s: %s", path, exc) + return None + if not isinstance(payload, dict): + log.warning("symbol cache at %s has no symbols list", path) + return None + updated = payload.get("updated") + if not isinstance(updated, int) or isinstance(updated, bool) or updated <= 0: + log.warning( + "symbol cache at %s has a missing or invalid 'updated' stamp; " + "treating as stale", path, + ) + return None + age = int(time.time()) - updated + if age > limit: + # Almost every reader of this cache is a mode:backtest terminal, where + # GET /symbols is refused with 409 — so naming it here would send the + # operator at the one call that cannot work. + log.warning( + "symbol cache at %s is %ds old (max %ds); treating as stale — " + "refresh it with POST /symbols/import on this terminal, or " + "GET /symbols if this terminal is mode:live", path, age, limit, + ) + return None + symbols = payload.get("symbols") + if not isinstance(symbols, list) or not symbols: + log.warning("symbol cache at %s has no symbols list", path) + return None + return {str(s) for s in symbols} + + +def age_seconds(terminal_dir): + """Seconds since the cache was written, or None when absent/unreadable.""" + path = cache_path(terminal_dir) + try: + with open(path, encoding="utf-8") as handle: + payload = json.load(handle) + updated = int(payload["updated"]) + except (OSError, ValueError, KeyError, TypeError): + return None + return max(0, int(time.time()) - updated) diff --git a/requirements-test.txt b/requirements-test.txt index 9294c398..a784d17c 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -12,3 +12,16 @@ pytest==9.0.3 pytest-cov==7.1.0 pyyaml==6.0.3 testcontainers==4.14.2 + +# Enough of the API's own runtime to import mt5api.server on the host and read +# its Flask url_map. test_mcpunifier.py compares the routes the unifier's +# `endpoints` MCP tool actually returns against that router, so the router has +# to be the real one — a hand-copied list on this side would assert nothing. +# MetaTrader5 is Windows-only and stays out: tests/conftest.py stubs it. +# +# Deliberately unpinned, unlike the four above: requirements-api.txt is the +# source of truth for these and pins nothing, so pinning here would let the +# version the test imports drift away from the version the VM installs. +flask +flask-compress +psutil diff --git a/scripts/config_helper.py b/scripts/config_helper.py index 92c360b4..5de65780 100644 --- a/scripts/config_helper.py +++ b/scripts/config_helper.py @@ -164,7 +164,17 @@ def main(): sys.exit(1) if cmd == "terminals": + # Scope to this VM's group. start.bat launches a terminal and starts an + # API process for every line this prints, so without the filter each VM + # in a multi-VM install prepares and serves ALL terminals — including + # another VM's live-mode terminal, whose data dir is on the same shared + # mount, so the two fight over MT5's single-instance lock. + # check_health.py already applies exactly this filter; the two must + # agree or the status loop probes ports this VM never started. + allowed = _vm_group_filter() for t in cfg.get("terminals") or []: + if not _in_group(t, allowed): + continue utc = t.get("utc_offset") utc = "0" if utc is None else str(utc).replace(" ", "") mode = (t.get("mode") or "live").strip().lower() or "live" diff --git a/scripts/measure-broker-offsets.py b/scripts/measure-broker-offsets.py new file mode 100755 index 00000000..95ebd160 --- /dev/null +++ b/scripts/measure-broker-offsets.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +"""Measure each terminal's broker-clock offset from real UTC. + +config.yaml's per-terminal `utc_offset` is a STATIC number that mt5api +subtracts from every broker timestamp (mt5client.broker_to_utc_seconds). +Nothing in the stack is DST-aware, so a value that is right in August is +one hour wrong after the autumn rollover. Re-run this then and update +config.yaml with whatever it prints. + +Reads the latest tick and compares its broker timestamp to local UTC, so +it only works while the market is open and the terminal can log in. + +WARNING: hitting an SDK route on a `mode: backtest` terminal makes mt5api +launch terminal64.exe, and POST /terminal/shutdown only detaches the SDK +client — it does not close the terminal. A terminal left running holds +MT5's single-instance lock on the data dir, and the next backtest there +spawns a second terminal64.exe that exits silently with code 0, producing +an empty "Bars=0 Ticks=0 Symbols=0" report. Run this only when you can +follow it with `docker compose down && ./run.sh`. + +Usage: + python3 scripts/measure-broker-offsets.py [broker/account ...] + +With no arguments it measures every terminal in config.yaml. +""" +import json +import os +import sys +import time +import urllib.error +import urllib.request + +import yaml + +HERE = os.path.dirname(os.path.abspath(__file__)) +CONFIG = os.path.join(HERE, os.pardir, "config", "config.yaml") +BASE = os.environ.get("MT5_HTTPAPI_BASE_URL", "http://127.0.0.1:8888") +TIMEOUT = int(os.environ.get("MEASURE_TIMEOUT", "120")) +# Broker clocks sit on quarter-hour boundaries; snapping absorbs tick latency. +QUANTUM = 900 +PREFERRED = ("EURUSD", "BTCUSD", "XAUUSD") + + +def load_config(): + with open(CONFIG, encoding="utf-8") as fh: + return yaml.safe_load(fh) + + +def api(token, path): + req = urllib.request.Request( + f"{BASE}/{path}", headers={"Authorization": f"Bearer {token}"} + ) + with urllib.request.urlopen(req, timeout=TIMEOUT) as resp: + return json.load(resp) + + +def pick_symbol(token, route): + """Shortest name matching a liquid, near-24h instrument.""" + symbols = api(token, f"{route}/symbols") + if not isinstance(symbols, list): + raise RuntimeError(f"unexpected /symbols payload: {symbols!r}") + for prefix in PREFERRED: + matches = [s for s in symbols if s.upper().startswith(prefix)] + if matches: + return sorted(matches, key=len)[0] + raise RuntimeError("no EURUSD/BTCUSD/XAUUSD variant in the symbol list") + + +def measure(token, route): + symbol = pick_symbol(token, route) + before = time.time() + tick = api(token, f"{route}/symbols/{symbol}/tick") + after = time.time() + broker_time = tick.get("time") + if not broker_time: + raise RuntimeError(f"tick carried no time: {tick!r}") + # The route already subtracts the CONFIGURED offset, so add it back to + # recover the raw broker clock — otherwise this reports 0 once the config + # is correct instead of confirming it. + configured = api(token, f"{route}/terminal").get("broker_utc_offset_seconds", 0) + raw = (broker_time + configured) - (before + after) / 2 + offset = round(raw / QUANTUM) * QUANTUM + return symbol, offset, raw, configured + + +def main(): + cfg = load_config() + token = cfg.get("api_token") or "" + routes = sys.argv[1:] + if not routes: + seen, routes = set(), [] + for term in cfg.get("terminals", []): + route = f"{term['broker']}/{term['account']}" + if route not in seen: + seen.add(route) + routes.append(route) + + failures = 0 + print(f"{'terminal':<32}{'symbol':<14}{'measured':>10}{'configured':>12}") + for route in routes: + try: + symbol, offset, raw, configured = measure(token, route) + except (urllib.error.URLError, urllib.error.HTTPError, RuntimeError) as exc: + failures += 1 + print(f"{route:<32}FAILED: {exc}") + continue + flag = "" if offset == configured else " <-- MISMATCH" + print( + f"{route:<32}{symbol:<14}{offset / 3600:>+9.2f}h" + f"{configured / 3600:>+11.2f}h{flag} (raw {raw:.1f}s)" + ) + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/rotate-logs.sh b/scripts/rotate-logs.sh index f6be3cd7..6ce6763d 100755 --- a/scripts/rotate-logs.sh +++ b/scripts/rotate-logs.sh @@ -16,6 +16,11 @@ LOG_DIR="${LOG_DIR:-/logs}" TERMINALS_DIR="${TERMINALS_DIR:-/terminals}" RETAIN_DAYS="${RETAIN_DAYS:-7}" INTERVAL="${INTERVAL:-3600}" +# Terminal journals need a size bound as well as an age one — see +# cap_journal_size. IDLE_MINUTES is the "a backtest is still writing this" +# guard; nothing touched more recently is truncated. +MAX_LOG_BYTES="${MAX_LOG_BYTES:-2147483648}" +IDLE_MINUTES="${IDLE_MINUTES:-30}" log() { printf '[%s] [rotator] %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$*" @@ -29,6 +34,54 @@ is_positive_integer() { [ "$1" -gt 0 ] } +# Age alone cannot bound these. A high-frequency strategy logs every order +# placement, modification and cancellation, so a single backtest can write tens +# of gigabytes into TODAY's journal — which the retention window deliberately +# will not touch for RETAIN_DAYS, long after the disk has filled. +# +# This reclaims that space once the run goes quiet; it does NOT bound a journal +# while its own backtest is still writing (see IDLE_MINUTES below), because +# truncating a running job's log destroys the diagnostics for the very run +# producing them. +# +# Truncate in place rather than delete: the terminal holds the journal open, so +# unlinking the inode would leave the writer pointed at a deleted file and the +# space would not come back until the terminal exited. +cap_journal_size() { + journal=$1 + + # stat, not `wc -c`: busybox wc READS the whole file to count bytes, which + # costs ~18s on a 3 GB journal (measured in alpine:3.20) and would run for + # every in-window journal every INTERVAL — on exactly the multi-gigabyte + # files this function exists for. stat is one fstat on busybox and GNU + # alike. The test image has GNU coreutils, where `wc -c` is already O(1), + # so this cost is invisible to the suite and only appears in production. + size=$(stat -c %s "$journal" 2>/dev/null || echo 0) + [ "$size" -gt "$MAX_LOG_BYTES" ] || return 0 + + # Anything written inside the idle window belongs to a running backtest; + # truncating it would destroy the diagnostics for the very run producing + # them. Let it exceed the cap until it goes quiet. + if [ -n "$(find "$journal" -mmin "-${IDLE_MINUTES}" 2>/dev/null)" ]; then + log "over cap but still active, left alone (${size}B) $journal" + return 0 + fi + + # Restore the mtime afterwards. _tail_dir_log picks the newest .log in a + # directory by mtime, so bumping this one to now would make a just-emptied + # journal outrank the journal a running job is actually writing, and + # GET /backtest//tail would answer with nothing until that job's next + # write. Truncating is this script's housekeeping, not the terminal + # logging, so it should not look like the most recent activity. + mtime=$(stat -c %Y "$journal" 2>/dev/null || echo "") + if : >"$journal"; then + if [ -n "$mtime" ]; then + touch -d "@$mtime" "$journal" 2>/dev/null || true + fi + log "truncated oversized terminal journal (${size}B) $journal" + fi +} + prune_journal_dir() { journal_dir=$1 cutoff=$2 @@ -47,7 +100,11 @@ prune_journal_dir() { if [ "$journal_date" -lt "$cutoff" ]; then rm -f "$journal" log "pruned terminal journal $journal" + continue fi + + # Still inside the retention window — bound it by size instead. + cap_journal_size "$journal" done } @@ -113,7 +170,17 @@ if ! is_positive_integer "$RETAIN_DAYS"; then exit 1 fi -log "starting (log_dir=$LOG_DIR terminals_dir=$TERMINALS_DIR retain_days=$RETAIN_DAYS interval=${INTERVAL}s)" +if ! is_positive_integer "$MAX_LOG_BYTES"; then + log "MAX_LOG_BYTES must be a positive integer, got: $MAX_LOG_BYTES" + exit 1 +fi + +if ! is_positive_integer "$IDLE_MINUTES"; then + log "IDLE_MINUTES must be a positive integer, got: $IDLE_MINUTES" + exit 1 +fi + +log "starting (log_dir=$LOG_DIR terminals_dir=$TERMINALS_DIR retain_days=$RETAIN_DAYS max_log=${MAX_LOG_BYTES}B idle_min=$IDLE_MINUTES interval=${INTERVAL}s)" while true; do if ! rotate_once; then log "rotate_once failed (continuing)" diff --git a/tests/integration/test_mcpunifier.py b/tests/integration/test_mcpunifier.py index c4955c7d..33219871 100644 --- a/tests/integration/test_mcpunifier.py +++ b/tests/integration/test_mcpunifier.py @@ -13,6 +13,7 @@ import base64 import json +import re import time import urllib.error import urllib.request @@ -252,6 +253,94 @@ def test_a_mismatched_broker_account_pair_is_refused(unifier): assert "unknown terminal" in json.dumps(result) +# ── `endpoints` vs the real Flask router ───────────────────────────── +# +# `endpoints` is the only route discovery an agent on the unified endpoint +# gets: it is the catalog the `request` escape hatch is driven from, so a route +# the Flask app serves but the catalog omits is invisible to every such agent. +# POST /symbols/import was exactly that. +# +# The unifier runs out-of-process and cannot read Flask's url_map, so its +# catalog is hand-maintained — which is why it needs a test. This one is +# black-box on the MCP side: it boots the shipped image, speaks the protocol, +# and reads what the public tool returns, so it also fails if tool registration, +# the transport, or response serialization breaks. (The version it replaced +# AST-parsed mcpunifier/mcp_server.py for the `_ROUTE_CATALOG` literal, which +# stayed green through all three of those.) + +# Flask reports these on every rule; they are not part of the callable surface. +_NON_API_METHODS = frozenset({"HEAD", "OPTIONS"}) +# Werkzeug spells a placeholder with its converter, ``; the catalog +# spells it ``. Normalize the Flask side to the catalog's form rather +# than the reverse: what `endpoints` describes is a path an agent substitutes a +# value into, and the converter is a routing-layer detail with no presence on +# the wire. Stripping it keeps the parameter NAME, which agents do read, so +# renaming `` to `` still fails this test. +_CONVERTER_PREFIX = re.compile(r"<[a-zA-Z_][a-zA-Z0-9_]*:") + + +def _flask_routes(): + """(method, path) for every route the real mt5api Flask app registers. + + Imported here rather than at module scope so the MT5 stub tests/conftest.py + installs is in place first — the SDK wheel is Windows-only. + + Note what is deliberately NOT in url_map and so not expected in the + catalog: mt5api/main.py mounts the per-terminal MCP app at /mcp through + werkzeug's DispatcherMiddleware, outside Flask's router entirely. The + catalog lists the REST surface `request` can call, which is the same set. + """ + from mt5api.server import app + + routes = set() + for rule in app.url_map.iter_rules(): + if rule.endpoint == "static": + continue + path = _CONVERTER_PREFIX.sub("<", str(rule.rule)) + for method in rule.methods - _NON_API_METHODS: + routes.add((method, path)) + return routes + + +def test_the_endpoints_tool_returns_exactly_the_real_flask_routes(unifier): + """Call the public `endpoints` tool over MCP and compare what comes back to + the Flask router every terminal actually serves. + + Equality, not containment, in both directions: a missing entry hides a real + route from agents, and a surplus entry sends them at a 404. Every route in + mt5api/server.py is registered unconditionally, so the two sides are + comparable exactly as they stand. If a route is ever registered behind a + config flag, this has to build the app under the configuration the catalog + documents rather than be relaxed to a subset check — a subset check would + pass the empty catalog. + """ + payload = json.loads(_tool_text(_call_tool(unifier, "endpoints", {}))) + + reported = { + (entry["method"], entry["path"]) for entry in payload["endpoints"] + } + expected = _flask_routes() + + assert reported == expected, ( + "`endpoints` and the Flask router disagree.\n" + f" only in the Flask app (invisible to agents): {sorted(expected - reported)}\n" + f" only in the catalog (agents would 404): {sorted(reported - expected)}" + ) + + +def test_the_endpoints_tool_lists_the_backtest_cache_priming_route(unifier): + """The specific regression the catalog missed, pinned by name. + + GET /symbols answers 409 on a mode: backtest terminal and tells the caller + to use POST /symbols/import instead. An agent that can only reach the API + through `request` has no way to find that route if `endpoints` omits it, so + the advice in the 409 is unfollowable. + """ + payload = json.loads(_tool_text(_call_tool(unifier, "endpoints", {}))) + + assert {"method": "POST", "path": "/symbols/import"} in payload["endpoints"] + + def test_the_endpoint_is_still_healthy_after_those_failures(unifier): """Ordered last on purpose: it asserts the failures above left the process serving rather than wedged. diff --git a/tests/test_backtest_log_tail.py b/tests/test_backtest_log_tail.py index d6f39574..d1d9473b 100644 --- a/tests/test_backtest_log_tail.py +++ b/tests/test_backtest_log_tail.py @@ -81,3 +81,114 @@ def test_tail_is_limited_to_the_requested_line_count(terminal_logs): _write_utf16(terminal_logs / "20260808.log", "".join(f"line {i}\n" for i in range(50))) tail = handler._tail_terminal_log(lines=5) assert tail.splitlines() == [f"line {i}" for i in range(45, 50)] + + +# ── Bounded reads (_read_tail_text) ───────────────────────────────────── +# +# Tailing must stay O(tail), never O(file): the live /tail endpoint is polled +# once a minute per running job against a Tester log that reaches gigabytes +# mid-run. A whole-file read of one of those took 45-65 s per call — decode +# and splitlines hold the GIL, so every thread in the process stalled, +# /ping and the container healthcheck included, and a healthy terminal +# looked wedged from the outside. + + +def test_read_tail_text_reads_only_the_final_window(tmp_path): + path = tmp_path / "big.log" + body = "".join(f"line {i:07d}\n" for i in range(200_000)) # ~2.6 MB utf-8 + path.write_text(body, encoding="utf-8") + + text = handler._read_tail_text(str(path), max_bytes=64 * 1024) + + lines = text.splitlines() + assert lines[-1] == "line 0199999" + assert len(text.encode("utf-8")) <= 64 * 1024 + # The window starts mid-file: the truncated first line must be dropped, + # so every surviving line is complete. + assert all(ln.startswith("line ") and len(ln) == 12 for ln in lines) + + +def test_read_tail_text_keeps_utf16_code_units_aligned(tmp_path): + path = tmp_path / "terminal.log" + body = "".join(f"запись {i:06d}\n" for i in range(50_000)) # force odd offsets + with open(path, "w", encoding="utf-16-le") as fh: + fh.write("") + fh.write(body) + + text = handler._read_tail_text(str(path), max_bytes=32 * 1024 + 1) + + lines = text.splitlines() + assert lines[-1] == "запись 049999" + # A misaligned seek shifts every code unit by one byte and turns the + # whole tail to mojibake — one intact line proves alignment held. + assert all(ln.startswith("запись ") for ln in lines) + + +def test_read_tail_text_small_file_is_returned_whole(tmp_path): + path = tmp_path / "run.log" + path.write_text("first\nsecond\n", encoding="utf-8") + assert handler._read_tail_text(str(path)) == "first\nsecond\n" + + +def test_read_tail_text_missing_file_is_empty(tmp_path): + assert handler._read_tail_text(str(tmp_path / "absent.log")) == "" + + +def test_tail_terminal_log_is_bounded(terminal_logs): + huge = "".join(f"entry {i:08d}\n" for i in range(300_000)) # ~9.7 MB utf-16 + _write_utf16(terminal_logs / "20260829.log", huge) + + tail = handler._tail_terminal_log(lines=5) + + assert tail.splitlines() == [f"entry {i:08d}" for i in range(299_995, 300_000)] + + +# ── _tail_dir_log (the live /tail endpoint's picker) ──────────────────── +# +# Same two rules as _tail_terminal_log — newest by mtime, never +# metaeditor.log — which this helper predated and never received. + + +def test_tail_dir_log_never_picks_metaeditor_log(tmp_path): + log_dir = tmp_path / "logs" + log_dir.mkdir() + _write_utf16(log_dir / "20260808.log", "Tester\tautomatic testing started\n") + _write_utf16(log_dir / "metaeditor.log", "compiling ancient stuff\n") + _age(log_dir / "20260808.log", 3600) + _age(log_dir / "metaeditor.log", 1) + + path, tail = handler._tail_dir_log(str(log_dir), 20) + + assert path.endswith("20260808.log") + assert "ancient" not in tail + + +def test_tail_dir_log_picks_newest_by_mtime_not_name(tmp_path): + log_dir = tmp_path / "logs" + log_dir.mkdir() + _write_utf16(log_dir / "20261231.log", "last year\n") + _write_utf16(log_dir / "20270101.log", "this year\n") + _age(log_dir / "20261231.log", 5) # older name, newer mtime + _age(log_dir / "20270101.log", 86400) + + path, tail = handler._tail_dir_log(str(log_dir), 20) + + assert path.endswith("20261231.log") + assert tail == "last year" + + +def test_tail_dir_log_is_bounded_on_a_large_log(tmp_path): + log_dir = tmp_path / "logs" + log_dir.mkdir() + huge = "".join(f"tick {i:08d}\n" for i in range(300_000)) + _write_utf16(log_dir / "20260829.log", huge) + + _, tail = handler._tail_dir_log(str(log_dir), 3) + + assert tail.splitlines() == [f"tick {i:08d}" for i in range(299_997, 300_000)] + + +def test_tail_dir_log_empty_dir(tmp_path): + log_dir = tmp_path / "logs" + log_dir.mkdir() + assert handler._tail_dir_log(str(log_dir), 20) == (None, "") diff --git a/tests/test_config_generation.py b/tests/test_config_generation.py index 49c14283..b7bb0e04 100644 --- a/tests/test_config_generation.py +++ b/tests/test_config_generation.py @@ -296,6 +296,11 @@ def test_log_rotator_mounts_only_logs_and_terminal_journals(tmp_path, monkeypatc "LOG_DIR": "/logs", "TERMINALS_DIR": "/terminals", "RETAIN_DAYS": "7", + # Age alone leaves today's journal unbounded, and one backtest can + # write tens of gigabytes into it — so the rotator needs the size cap + # wired up too, not just the retention window. + "MAX_LOG_BYTES": "2147483648", + "IDLE_MINUTES": "30", "INTERVAL": "3600", } assert rotator["volumes"] == [ @@ -316,6 +321,11 @@ def test_default_compose_exposes_terminal_retention_to_the_rotator(): "LOG_DIR": "/logs", "TERMINALS_DIR": "/terminals", "RETAIN_DAYS": "7", + # Age alone leaves today's journal unbounded, and one backtest can + # write tens of gigabytes into it — so the rotator needs the size cap + # wired up too, not just the retention window. + "MAX_LOG_BYTES": "2147483648", + "IDLE_MINUTES": "30", "INTERVAL": "3600", } assert compose["services"]["wickworks"]["image"] == WICKWORKS_IMAGE diff --git a/tests/test_live_api_contract.py b/tests/test_live_api_contract.py index 7d3baab0..18805202 100644 --- a/tests/test_live_api_contract.py +++ b/tests/test_live_api_contract.py @@ -227,9 +227,14 @@ def test_error_endpoint_returns_the_last_sdk_error(api_client, patch_handler): # ── tests/real/test_symbols.py ─────────────────────────────────────────── -def test_list_symbols_returns_the_tradeable_names(api_client, patch_handler): +def test_list_symbols_returns_the_tradeable_names( + api_client, patch_handler, monkeypatch, tmp_path +): recorder = patch_handler(symbols_handler) recorder.set("symbols_get", [Symbol(name=SYMBOL), Symbol(name="OTHERUSD")]) + # An unfiltered listing persists the symbol cache as a side effect; without + # this it writes into the real TERMINAL_DIR under the working directory. + monkeypatch.setattr(symbols_handler, "TERMINAL_DIR", str(tmp_path)) resp = api_client.get("/symbols") diff --git a/tests/test_request_body_cap.py b/tests/test_request_body_cap.py new file mode 100644 index 00000000..ff86f2ec --- /dev/null +++ b/tests/test_request_body_cap.py @@ -0,0 +1,111 @@ +"""The global per-request body cap in mt5api/server.py. + +POST /symbols/import was reported as accepting a 2 MB symbol name, and fixed +with its own cap. But six other routes parsed an unbounded JSON body the same +way -- POST /orders, PUT /orders/, PUT and DELETE /positions/, +POST /symbols//rates/ta, POST /backtest/build-ini and +/backtest/build-set. A per-endpoint cap only bounds the endpoint someone +remembered; these tests pin the backstop that bounds the ones nobody did, +including routes added after this was written. +""" +from __future__ import annotations + +import pytest + +from mt5api import config +from mt5api.server import app + +# Every route that reads a request body, with a minimal valid content type. +# POST /backtest is multipart and gets the larger cap, so it is tested apart. +JSON_BODY_ROUTES = ( + ("POST", "/orders"), + ("PUT", "/orders/1"), + ("PUT", "/positions/1"), + ("DELETE", "/positions/1"), + ("POST", "/symbols/EURUSD/rates/ta"), + ("POST", "/backtest/build-ini"), + ("POST", "/backtest/build-set"), +) + + +def _client(): + app.config["TESTING"] = True + return app.test_client() + + +def _send(method, path, payload, content_type="application/json"): + return _client().open( + path, method=method, data=payload, content_type=content_type + ) + + +@pytest.mark.parametrize("method,path", JSON_BODY_ROUTES) +def test_every_json_route_refuses_a_body_over_the_cap(method, path): + """Each of these used to hand request.get_json() whatever arrived.""" + payload = b"x" * (config.MAX_REQUEST_BODY_BYTES + 1) + + resp = _send(method, path, payload) + + assert resp.status_code == 413, f"{method} {path} accepted an oversized body" + assert "MAX_REQUEST_BODY_BYTES" in resp.get_json()["error"] + + +@pytest.mark.parametrize("method,path", JSON_BODY_ROUTES) +def test_the_cap_runs_before_the_body_is_parsed(method, path): + """The payload is over the cap and is NOT valid JSON. Only a gate placed + before the parser can answer 413; one placed after answers 400 (or 500) + because parsing fails first. This is what makes the cap worth having -- + refusing after get_json() has already built the object graph pays exactly + the cost the cap exists to avoid. + """ + payload = b"{" + b"x" * (config.MAX_REQUEST_BODY_BYTES + 1) + + resp = _send(method, path, payload) + + assert resp.status_code == 413 + + +def test_a_body_exactly_at_the_cap_is_not_refused_by_it(): + """Boundary: the cap must not fire at exactly the limit. This asserts the + cap does NOT trigger, so it cannot be made red by removing the cap -- its + job is to pin the off-by-one against a future tightening. + """ + payload = b"x" * config.MAX_REQUEST_BODY_BYTES + + resp = _send("POST", "/backtest/build-ini", payload) + + assert resp.status_code != 413 + + +def test_multipart_uploads_get_the_larger_cap(): + """POST /backtest carries a compiled .ex5 plus its .set and .ini, so the + JSON cap would refuse legitimate submissions.""" + assert config.MAX_UPLOAD_BODY_BYTES > config.MAX_REQUEST_BODY_BYTES + payload = b"x" * (config.MAX_REQUEST_BODY_BYTES + 1) + + resp = _send("POST", "/backtest", payload, content_type="multipart/form-data; boundary=x") + + assert resp.status_code != 413, "a multipart upload was held to the JSON cap" + + +def test_multipart_over_its_own_cap_is_still_refused(): + payload = b"x" * (config.MAX_UPLOAD_BODY_BYTES + 1) + + resp = _send("POST", "/backtest", payload, content_type="multipart/form-data; boundary=x") + + assert resp.status_code == 413 + assert "MAX_UPLOAD_BODY_BYTES" in resp.get_json()["error"] + + +def test_the_endpoint_cap_still_wins_where_one_is_tighter(): + """POST /symbols/import declares 2 MiB against the global 4 MiB. The + endpoint's own message must be what a caller sees, so the tighter bound is + the one reported rather than being masked by the backstop. + """ + assert config.SYMBOL_IMPORT_MAX_BODY_BYTES < config.MAX_REQUEST_BODY_BYTES + payload = b"x" * (config.SYMBOL_IMPORT_MAX_BODY_BYTES + 1) + + resp = _send("POST", "/symbols/import", payload) + + assert resp.status_code == 413 + assert "SYMBOL_IMPORT_MAX_BODY_BYTES" in resp.get_json()["error"] diff --git a/tests/test_rotate_logs.py b/tests/test_rotate_logs.py index 71227928..61a911b8 100644 --- a/tests/test_rotate_logs.py +++ b/tests/test_rotate_logs.py @@ -43,12 +43,21 @@ def _rotated_log_name(name: str) -> str: return name + "." + _journal_name(1).removesuffix(".log") -def _run_rotator(log_dir: Path, terminals_dir: Path, retain_days: str, ready) -> None: +def _run_rotator( + log_dir: Path, + terminals_dir: Path, + retain_days: str, + ready, + max_log_bytes: str = "2147483648", + idle_minutes: str = "30", +) -> None: environment = { "PATH": os.environ.get("PATH", "/usr/bin:/bin"), "LOG_DIR": str(log_dir), "TERMINALS_DIR": str(terminals_dir), "RETAIN_DAYS": retain_days, + "MAX_LOG_BYTES": max_log_bytes, + "IDLE_MINUTES": idle_minutes, "INTERVAL": "3600", } process = subprocess.Popen( @@ -198,6 +207,168 @@ def test_journal_cleanup_is_idempotent(tmp_path): assert not journal.exists() +# ── Size cap ───────────────────────────────────────────────────────── +# +# The retention window above deliberately will not touch a journal until it is +# RETAIN_DAYS old. A high-frequency strategy can write tens of gigabytes into +# today's journal during a single backtest, so age alone lets a disk fill long +# before the first prune is even eligible to fire. + + +def _age(path: Path, minutes: int) -> None: + stamp = time.time() - minutes * 60 + os.utime(path, (stamp, stamp)) + + +@pytest.mark.parametrize("location", _JOURNAL_LOCATIONS) +def test_truncates_an_oversized_idle_journal_inside_the_retention_window( + tmp_path, location +): + """Truncated in place, not deleted: the terminal holds the journal open, so + unlinking the inode would leave the writer pointed at a deleted file and + the space would not come back until the terminal exited. + """ + log_dir = tmp_path / "shared-logs" + terminals_dir = tmp_path / "terminals" + # Today's journal — the age pass will not consider it for another 7 days. + journal = _terminal_dir(terminals_dir) / location / _journal_name(0) + _write(journal, "x" * 4096) + _age(journal, 120) + inode_before = journal.stat().st_ino + + _run_rotator( + log_dir, + terminals_dir, + "7", + lambda: journal.exists() and journal.stat().st_size == 0, + max_log_bytes="1024", + ) + + assert journal.exists(), "journal was deleted; it must be truncated in place" + assert journal.stat().st_size == 0 + assert journal.stat().st_ino == inode_before, "inode changed; the writer is orphaned" + + +def test_truncating_a_journal_does_not_make_it_look_freshly_written(tmp_path): + """`_tail_dir_log` picks the newest .log in a directory by mtime. If + truncation bumped the mtime to now, a just-emptied journal would outrank + the one a running job is writing and GET /backtest//tail would answer + with nothing until that job's next write — the same stale-wrong-log failure + the mtime selection was introduced to fix. + """ + log_dir = tmp_path / "shared-logs" + terminals_dir = tmp_path / "terminals" + journal_dir = _terminal_dir(terminals_dir) / _JOURNAL_LOCATIONS[0] + oversized = journal_dir / _journal_name(1) + _write(oversized, "x" * 4096) + _age(oversized, 120) + mtime_before = oversized.stat().st_mtime + # The journal a running job would be writing: newer, and under the cap. + live = journal_dir / _journal_name(0) + _write(live, "x" * 16) + + _run_rotator( + log_dir, + terminals_dir, + "7", + lambda: oversized.exists() and oversized.stat().st_size == 0, + max_log_bytes="1024", + ) + + assert oversized.stat().st_mtime == pytest.approx(mtime_before, abs=1) + newest = max((live, oversized), key=lambda p: p.stat().st_mtime) + assert newest == live, "the emptied journal outranks the live one by mtime" + + +def test_oversized_journal_is_left_alone_while_a_backtest_is_writing_it(tmp_path): + """Truncating the log of a run in progress destroys the diagnostics for the + very backtest producing them. Over the cap but recently written wins. + """ + log_dir = tmp_path / "shared-logs" + terminals_dir = tmp_path / "terminals" + terminal = _terminal_dir(terminals_dir) + active = terminal / _JOURNAL_LOCATIONS[0] / _journal_name(0) + _write(active, "x" * 4096) + # A second, idle journal gives the pass an observable finishing line that + # does not depend on the active one being touched. + idle = terminal / _JOURNAL_LOCATIONS[1] / _journal_name(0) + _write(idle, "x" * 4096) + _age(idle, 120) + + _run_rotator( + log_dir, + terminals_dir, + "7", + lambda: idle.exists() and idle.stat().st_size == 0, + max_log_bytes="1024", + idle_minutes="30", + ) + + assert active.stat().st_size == 4096 + + +def test_journal_under_the_size_cap_is_untouched(tmp_path): + log_dir = tmp_path / "shared-logs" + terminals_dir = tmp_path / "terminals" + terminal = _terminal_dir(terminals_dir) + small = terminal / _JOURNAL_LOCATIONS[0] / _journal_name(0) + _write(small, "x" * 100) + _age(small, 120) + oversized = terminal / _JOURNAL_LOCATIONS[1] / _journal_name(0) + _write(oversized, "x" * 4096) + _age(oversized, 120) + + _run_rotator( + log_dir, + terminals_dir, + "7", + lambda: oversized.exists() and oversized.stat().st_size == 0, + max_log_bytes="1024", + ) + + assert small.read_text(encoding="utf-8") == "x" * 100 + + +@pytest.mark.parametrize( + "overrides", + ( + {"max_log_bytes": "0"}, + {"max_log_bytes": "-1"}, + {"max_log_bytes": "not-a-number"}, + {"idle_minutes": "0"}, + {"idle_minutes": "not-a-number"}, + ), +) +def test_invalid_size_cap_settings_fail_before_touching_journals(tmp_path, overrides): + log_dir = tmp_path / "shared-logs" + terminals_dir = tmp_path / "terminals" + journal = _terminal_dir(terminals_dir) / _JOURNAL_LOCATIONS[0] / _journal_name(0) + _write(journal, "x" * 4096) + _age(journal, 120) + + environment = { + "PATH": os.environ.get("PATH", "/usr/bin:/bin"), + "LOG_DIR": str(log_dir), + "TERMINALS_DIR": str(terminals_dir), + "RETAIN_DAYS": "7", + "MAX_LOG_BYTES": overrides.get("max_log_bytes", "1024"), + "IDLE_MINUTES": overrides.get("idle_minutes", "30"), + "INTERVAL": "3600", + } + result = subprocess.run( + ["sh", str(_SCRIPT)], + cwd=_REPO, + env=environment, + capture_output=True, + text=True, + timeout=_DEFAULT_TIMEOUT_SECONDS, + check=False, + ) + + assert result.returncode != 0 + assert journal.stat().st_size == 4096 + + @pytest.mark.parametrize("retain_days", ("-1", "0", "not-a-number")) def test_invalid_retention_fails_before_deleting_journals(tmp_path, retain_days): log_dir = tmp_path / "shared-logs" diff --git a/tests/test_symbol_import.py b/tests/test_symbol_import.py new file mode 100644 index 00000000..911680c9 --- /dev/null +++ b/tests/test_symbol_import.py @@ -0,0 +1,520 @@ +"""HTTP-level regression tests for the safe symbol-cache priming path. + +GET /symbols is @with_mt5 and calls ensure_initialized(), which on a terminal +that never attached the SDK (mode: backtest) falls through to a full +mt5.initialize() -- spawning terminal64.exe and holding the tester's +single-instance data-dir lock for the rest of that terminal's life. That used +to be the documented way to prime this terminal's symbol cache; it wedged the +terminal instead. These tests prove, against the real Flask app: + + 1. GET /symbols on a mode: backtest terminal is refused BEFORE + ensure_initialized() or any mt5.* SDK call is ever reached. + 2. POST /symbols/import -- the safe replacement -- writes the cache without + calling ensure_initialized() or any mt5.* SDK call, regardless of mode. + 3. That refusal does not queue behind the global MT5 lock, and the live + listing still does. + 4. The priming workflow config/config.yaml.example documents actually + produces the cache the backtest INI builder reads. + 5. The three per-request caps hold: body bytes (refused BEFORE the body is + parsed), entry count, and per-symbol length. +""" +from __future__ import annotations + +import json +import threading +import time + +import pytest + +import mt5api.handlers.symbols as h +from mt5api import config, mt5client, symbol_cache +from mt5api.backtest import handler as backtest_handler +from mt5api.server import app + + +def _forbidden(*_args, **_kwargs): + raise AssertionError("ensure_initialized() must not be called here") + + +def _client(): + app.config["TESTING"] = True + return app.test_client() + + +def test_list_symbols_refused_on_backtest_mode_without_sdk_call(monkeypatch): + monkeypatch.setattr(h, "MODE", "backtest") + monkeypatch.setattr(h, "ensure_initialized", _forbidden) + h.mt5.symbols_get.reset_mock() + + resp = _client().get("/symbols") + + assert resp.status_code == 409 + body = resp.get_json() + assert body is not None and "error" in body + assert "backtest" in body["error"].lower() + h.mt5.symbols_get.assert_not_called() + + +def test_list_symbols_still_works_live_mode(monkeypatch, tmp_path): + monkeypatch.setattr(h, "MODE", "live") + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "ensure_initialized", lambda: True) + fake = type("S", (), {"name": "EURUSD"})() + monkeypatch.setattr(h.mt5, "symbols_get", lambda **kw: [fake]) + + resp = _client().get("/symbols") + + assert resp.status_code == 200 + assert resp.get_json() == ["EURUSD"] + assert symbol_cache.load(str(tmp_path)) == {"EURUSD"} + + +def test_import_symbols_writes_cache_without_sdk_call(monkeypatch, tmp_path): + monkeypatch.setattr(h, "MODE", "backtest") + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "ensure_initialized", _forbidden) + h.mt5.initialize.reset_mock() + h.mt5.terminal_info.reset_mock() + + resp = _client().post("/symbols/import", json={"symbols": ["EURUSD", "XAUUSD", "EURUSD"]}) + + assert resp.status_code == 200 + assert resp.get_json() == {"imported": 2} + assert symbol_cache.load(str(tmp_path)) == {"EURUSD", "XAUUSD"} + h.mt5.initialize.assert_not_called() + h.mt5.terminal_info.assert_not_called() + + +def test_import_symbols_strips_stray_whitespace(monkeypatch, tmp_path): + # This is operator copy-paste input, not the SDK-sourced list GET /symbols + # persists -- a stray space would silently defeat the exact-match lookup + # in backtest.handler._normalize_symbol. + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={"symbols": [" EURUSD ", "XAUUSD\n", " "]}) + + assert resp.status_code == 200 + assert resp.get_json() == {"imported": 2} + assert symbol_cache.load(str(tmp_path)) == {"EURUSD", "XAUUSD"} + + +def test_import_symbols_rejects_non_list_body(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={"symbols": "EURUSD"}) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_non_string_entries(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={"symbols": ["EURUSD", 123]}) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_empty_list(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={"symbols": []}) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_missing_body(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={}) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_bare_json_array_body(monkeypatch, tmp_path): + # get_json(silent=True) returns the bare list as-is for syntactically + # valid JSON that isn't an object -- a naive body.get("symbols") on that + # raises AttributeError (500) instead of the clean 400 every other bad + # shape gets. Regression for that crash. + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json=["EURUSD", "XAUUSD"]) + + assert resp.status_code == 400 + body = resp.get_json() + assert body is not None and "error" in body + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_bare_json_scalar_body(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json=42) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_no_body_at_all(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import") + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +# ── The refusal must not queue behind the global MT5 lock ──────────── + +@pytest.fixture +def mt5_lock_held(): + """Hold the global MT5 lock from another thread, as an in-flight SDK + request would, and shorten the acquire timeout so a handler that queues + for it fails in milliseconds instead of the production minute. + """ + acquired = threading.Event() + release = threading.Event() + + def holder(): + mt5client._mt5_lock.acquire() + acquired.set() + release.wait(timeout=30) + mt5client._mt5_lock.release() + + thread = threading.Thread(target=holder, daemon=True) + thread.start() + assert acquired.wait(timeout=5), "lock holder thread never started" + original = mt5client.SESSION_ACQUIRE_TIMEOUT + mt5client.SESSION_ACQUIRE_TIMEOUT = 2.0 + try: + yield + finally: + mt5client.SESSION_ACQUIRE_TIMEOUT = original + release.set() + thread.join(timeout=5) + + +def test_backtest_refusal_does_not_wait_on_the_mt5_lock(monkeypatch, mt5_lock_held): + """A backtest terminal whose SDK request is stuck must still refuse GET + /symbols immediately. While @with_mt5 wrapped this handler the decorator + entered session() and blocked on the lock before the MODE check ever ran, + so the caller got a 503 after the full acquire timeout -- 60s in + production -- instead of the 409 that needs no SDK at all. + """ + monkeypatch.setattr(h, "MODE", "backtest") + monkeypatch.setattr(h, "ensure_initialized", _forbidden) + + started = time.monotonic() + resp = _client().get("/symbols") + elapsed = time.monotonic() - started + + assert resp.status_code == 409, ( + f"expected an immediate 409, got {resp.status_code} -- the refusal " + "queued for the MT5 lock" + ) + assert elapsed < 1.0, f"refusal took {elapsed:.3f}s; it waited on the lock" + # The holder still owns it: the request neither took nor stole the lock. + assert mt5client._mt5_lock.locked() + + +def test_live_listing_still_waits_on_the_mt5_lock(monkeypatch, mt5_lock_held): + """The other half of the split: only the backtest branch skips the lock. + The live path still touches the SDK, so it must stay serialized behind it + rather than racing other handlers. + """ + monkeypatch.setattr(h, "MODE", "live") + monkeypatch.setattr(h, "ensure_initialized", _forbidden) + + resp = _client().get("/symbols") + + assert resp.status_code == 503 + assert "could not acquire MT5 lock" in resp.get_json()["error"] + + +# ── The workflow config/config.yaml.example documents ──────────────── + +def test_documented_backtest_priming_workflow_primes_the_suffix_decision( + monkeypatch, tmp_path +): + """Walk the flow config/config.yaml.example tells a backtest-mode operator + to run, and assert the outcome it promises: after priming, the INI builder + stops appending the suffix to a symbol the broker carries bare. + + Source of the list is a live terminal on the same broker/account, so the + payload here is exactly what GET /symbols returns there. + """ + terminal = tmp_path / "terminal" + terminal.mkdir() + monkeypatch.setattr(h, "TERMINAL_DIR", str(terminal)) + monkeypatch.setattr(backtest_handler, "TERMINAL_DIR", str(terminal)) + monkeypatch.setattr(backtest_handler, "SYMBOL_SUFFIX", ".i") + monkeypatch.setattr(backtest_handler, "SYMBOL_SUFFIX_CONFIGURED", True) + client = _client() + + # Step 1: the old instruction -- GET /symbols -- is refused here. + monkeypatch.setattr(h, "MODE", "backtest") + monkeypatch.setattr(h, "ensure_initialized", _forbidden) + assert client.get("/symbols").status_code == 409 + assert symbol_cache.load(str(terminal)) is None + + # Step 2: prime with the documented POST body instead. + resp = client.post( + "/symbols/import", + json={"symbols": ["EURUSD.i", "GBPUSD.i", "XAUUSD"]}, + ) + assert resp.status_code == 200 + assert resp.get_json() == {"imported": 3} + + # Step 3: the builder now reads that cache. XAUUSD is carried bare and not + # suffixed, so it keeps its name; EURUSD is suffix-only and still remaps. + assert _tester_symbol("XAUUSD") == "XAUUSD" + assert _tester_symbol("EURUSD") == "EURUSD.i" + + +def _tester_symbol(symbol): + import configparser + + parser = configparser.RawConfigParser() + parser.optionxform = str + parser["Tester"] = {"Symbol": symbol} + backtest_handler._normalize_symbol(parser) + return parser["Tester"]["Symbol"] + + +# ── Per-request bounds ─────────────────────────────────────────────── +# +# The endpoint writes caller-supplied text straight into +# /mt5api-symbols.json, which the backtest INI builder reads and +# parses on every run inside a fixed-disk Windows VM. Before these caps a +# single authenticated request could hand the JSON parser an unbounded body +# and persist whatever came out: one `symbols` item of 2,097,153 bytes was +# accepted with a 200 and cached. All three caps are configurable through +# mt5api/config.py; the tests below pin the max / max-plus-one boundary of +# each, and the body cap's placement BEFORE request.get_json(). + + +def _body(symbols): + return json.dumps({"symbols": symbols}).encode("utf-8") + + +def test_import_symbols_accepts_a_body_exactly_at_the_byte_cap(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + payload = _body(["EURUSD", "XAUUSD"]) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_BODY_BYTES", len(payload)) + + resp = _client().post( + "/symbols/import", data=payload, content_type="application/json" + ) + + assert resp.status_code == 200 + assert resp.get_json() == {"imported": 2} + assert symbol_cache.load(str(tmp_path)) == {"EURUSD", "XAUUSD"} + + +def test_import_symbols_rejects_a_body_one_byte_over_the_cap(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + payload = _body(["EURUSD", "XAUUSD"]) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_BODY_BYTES", len(payload) - 1) + + resp = _client().post( + "/symbols/import", data=payload, content_type="application/json" + ) + + assert resp.status_code == 413 + body = resp.get_json() + assert body is not None and "SYMBOL_IMPORT_MAX_BODY_BYTES" in body["error"] + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_rejects_an_oversized_body_before_parsing_it( + monkeypatch, tmp_path +): + """The body cap must be enforced BEFORE request.get_json(). + + get_json() reads the whole body into memory and builds an object graph + from it, so a cap checked afterwards has already paid the cost it exists + to prevent. This request declares a Content-Length over the cap while + carrying a body that is not valid JSON at all: only an ordering where the + length gate runs first can answer 413. Parse first and the silent parser + returns None, which is the generic 400 -- so a 400 here is the failure + signal, not a pass. + """ + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_BODY_BYTES", 1024) + + resp = _client().post( + "/symbols/import", + data=b"this is not json", + content_type="application/json", + environ_overrides={"CONTENT_LENGTH": "1025"}, + ) + + assert resp.status_code == 413, ( + f"expected 413, got {resp.status_code} -- the body cap ran after " + "get_json() instead of before it" + ) + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_refuses_a_body_with_no_declared_length(monkeypatch, tmp_path): + """A chunked body has no length to check, so it cannot be bounded up front + and is refused outright (411) rather than waved past the cap. A request + with no body at all also has no Content-Length, but carries no transfer + encoding either -- it keeps the plain 400 it has always had, asserted by + test_import_symbols_rejects_no_body_at_all above. + """ + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post( + "/symbols/import", + data=_body(["EURUSD"]), + content_type="application/json", + headers={"Transfer-Encoding": "chunked"}, + ) + + assert resp.status_code == 411 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_accepts_the_maximum_entry_count(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_SYMBOLS", 5) + + resp = _client().post( + "/symbols/import", json={"symbols": [f"SYM{i}" for i in range(5)]} + ) + + assert resp.status_code == 200 + assert resp.get_json() == {"imported": 5} + + +def test_import_symbols_rejects_one_entry_over_the_maximum(monkeypatch, tmp_path): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_SYMBOLS", 5) + + resp = _client().post( + "/symbols/import", json={"symbols": [f"SYM{i}" for i in range(6)]} + ) + + assert resp.status_code == 400 + body = resp.get_json() + assert body is not None and "SYMBOL_IMPORT_MAX_SYMBOLS" in body["error"] + assert symbol_cache.load(str(tmp_path)) is None + + +def test_entry_count_is_measured_before_deduplication(monkeypatch, tmp_path): + """Counted on what the caller sent, not on what survives the dedupe. + + The cap's job is to bound the strip/dedupe/sort pass, which runs over the + raw array -- so a million repetitions of "EURUSD" has to be refused even + though it would deduplicate to one entry. + """ + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_SYMBOLS", 5) + + resp = _client().post("/symbols/import", json={"symbols": ["EURUSD"] * 6}) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_import_symbols_accepts_a_symbol_exactly_at_the_length_cap( + monkeypatch, tmp_path +): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_SYMBOL_LENGTH", 8) + + resp = _client().post("/symbols/import", json={"symbols": ["E" * 8]}) + + assert resp.status_code == 200 + assert symbol_cache.load(str(tmp_path)) == {"E" * 8} + + +def test_import_symbols_rejects_a_symbol_one_character_over_the_cap( + monkeypatch, tmp_path +): + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_SYMBOL_LENGTH", 8) + + resp = _client().post("/symbols/import", json={"symbols": ["EURUSD", "E" * 9]}) + + assert resp.status_code == 400 + body = resp.get_json() + assert body is not None and "SYMBOL_IMPORT_MAX_SYMBOL_LENGTH" in body["error"] + # Nothing is persisted: one bad name rejects the whole import rather than + # silently caching a partial book, which the INI builder would read as the + # broker's complete symbol list. + assert symbol_cache.load(str(tmp_path)) is None + + +def test_symbol_length_is_measured_after_whitespace_is_stripped(monkeypatch, tmp_path): + """The cap applies to the NORMALIZED name -- the form that reaches the + cache and that the INI builder matches against. A legal name padded with + whitespace must still be accepted. + """ + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + monkeypatch.setattr(h, "SYMBOL_IMPORT_MAX_SYMBOL_LENGTH", 8) + + resp = _client().post("/symbols/import", json={"symbols": [" " + "E" * 8 + " "]}) + + assert resp.status_code == 200 + assert symbol_cache.load(str(tmp_path)) == {"E" * 8} + + +def test_the_reported_two_megabyte_symbol_is_refused_at_shipped_defaults(tmp_path, monkeypatch): + """Verbatim reproduction of the review finding, against the real defaults. + + One `symbols` item of 2,097,153 bytes used to return 200 and land in the + cache. No monkeypatched limits here on purpose -- this asserts the values + the server actually ships with are the ones that refuse it. + """ + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={"symbols": ["A" * 2_097_153]}) + + assert resp.status_code == 413 + assert symbol_cache.load(str(tmp_path)) is None + + +def test_a_symbol_under_the_body_cap_but_over_the_length_cap_is_still_refused( + tmp_path, monkeypatch +): + """The second half of that finding: shrink the payload under the 2 MiB body + cap and the per-symbol cap is what has to catch it. Without it, a 1 KB + "symbol" would still be cached as a broker symbol name. + """ + monkeypatch.setattr(h, "TERMINAL_DIR", str(tmp_path)) + + resp = _client().post("/symbols/import", json={"symbols": ["A" * 1024]}) + + assert resp.status_code == 400 + assert symbol_cache.load(str(tmp_path)) is None + + +# ── The knobs themselves ───────────────────────────────────────────── + + +def test_symbol_import_limits_read_the_environment(monkeypatch): + monkeypatch.setenv("SYMBOL_IMPORT_MAX_SYMBOLS", "123") + + assert config._positive_int_setting( + "SYMBOL_IMPORT_MAX_SYMBOLS", "symbol_import_max_symbols", 20000 + ) == 123 + + +@pytest.mark.parametrize("bad", ["", "abc", "0", "-5", " "]) +def test_symbol_import_limits_clamp_instead_of_raising(monkeypatch, bad): + """config.py is imported by the whole API: a typo in one endpoint's tuning + value must not stop trading, and must not disable the only offline path a + backtest terminal has for priming its symbol cache either. + """ + monkeypatch.setenv("SYMBOL_IMPORT_MAX_SYMBOLS", bad) + + value = config._positive_int_setting( + "SYMBOL_IMPORT_MAX_SYMBOLS", "symbol_import_max_symbols", 20000 + ) + + assert value >= 1 diff --git a/tests/test_symbol_suffix_remap.py b/tests/test_symbol_suffix_remap.py new file mode 100644 index 00000000..e2364a4b --- /dev/null +++ b/tests/test_symbol_suffix_remap.py @@ -0,0 +1,234 @@ +"""Tests for the suffix remap in mt5api.backtest.handler._normalize_symbol +and the symbol cache it consults. + +The remap exists because config.yaml carries one `symbol_suffix` per terminal, +but real brokers suffix only part of their book. The cache is what lets the +builder tell those apart; without it the remap must keep its old behaviour. +""" +from __future__ import annotations + +import configparser +import json +import os + +import pytest + +from mt5api import symbol_cache +from mt5api.backtest import handler + + +def _parser(symbol): + parser = configparser.RawConfigParser() + parser.optionxform = str + parser["Tester"] = {"Symbol": symbol} + return parser + + +@pytest.fixture +def terminal_dir(monkeypatch, tmp_path): + d = tmp_path / "terminal" + d.mkdir() + monkeypatch.setattr(handler, "TERMINAL_DIR", str(d)) + monkeypatch.setattr(handler, "BROKER", "eightcapglobal") + monkeypatch.setattr(handler, "ACCOUNT", "live-raw") + return d + + +def _configure(monkeypatch, suffix): + monkeypatch.setattr(handler, "SYMBOL_SUFFIX", suffix) + monkeypatch.setattr(handler, "SYMBOL_SUFFIX_CONFIGURED", True) + + +def _remap(symbol): + parser = _parser(symbol) + handler._normalize_symbol(parser) + return parser["Tester"]["Symbol"] + + +# ── Without a cache: unchanged legacy behaviour ────────────────────── + + +def test_appends_suffix_when_no_cache_exists(terminal_dir, monkeypatch): + _configure(monkeypatch, ".i") + assert _remap("EURUSD") == "EURUSD.i" + + +def test_appends_suffix_when_cache_is_corrupt(terminal_dir, monkeypatch): + _configure(monkeypatch, ".i") + symbol_cache.cache_path(str(terminal_dir)) + with open(symbol_cache.cache_path(str(terminal_dir)), "w") as fh: + fh.write("{not json") + assert _remap("XAUUSD") == "XAUUSD.i" + + +def test_no_suffix_configured_leaves_symbol_alone(terminal_dir, monkeypatch): + monkeypatch.setattr(handler, "SYMBOL_SUFFIX", "") + monkeypatch.setattr(handler, "SYMBOL_SUFFIX_CONFIGURED", True) + assert _remap("EURUSD") == "EURUSD" + + +def test_already_suffixed_symbol_is_not_double_suffixed(terminal_dir, monkeypatch): + _configure(monkeypatch, ".i") + symbol_cache.save(str(terminal_dir), ["EURUSD.i"]) + assert _remap("EURUSD.i") == "EURUSD.i" + + +# ── With a cache: the actual fix ───────────────────────────────────── + + +def test_bare_only_symbol_keeps_its_name(terminal_dir, monkeypatch): + """Eightcap: XAUUSD exists, XAUUSD.i does not — appending invents a symbol.""" + _configure(monkeypatch, ".i") + symbol_cache.save(str(terminal_dir), ["EURUSD.i", "XAUUSD", "ASX200", "BTCUSD"]) + assert _remap("XAUUSD") == "XAUUSD" + assert _remap("ASX200") == "ASX200" + assert _remap("BTCUSD") == "BTCUSD" + + +def test_suffixed_symbol_is_still_remapped(terminal_dir, monkeypatch): + """Eightcap: EURUSD does not exist, EURUSD.i does.""" + _configure(monkeypatch, ".i") + symbol_cache.save(str(terminal_dir), ["EURUSD.i", "XAUUSD"]) + assert _remap("EURUSD") == "EURUSD.i" + + +def test_suffix_wins_when_broker_carries_both_forms(terminal_dir, monkeypatch): + """BlackBull lists AUDUSD and AUDUSDp. `symbol_suffix: p` means prime, so + the suffixed variant must still win — this is why the guard requires the + suffixed form to be ABSENT, not merely the bare form to be present.""" + _configure(monkeypatch, "p") + monkeypatch.setattr(handler, "BROKER", "blackbull") + symbol_cache.save(str(terminal_dir), ["AUDUSD", "AUDUSDp", "XAUUSD"]) + assert _remap("AUDUSD") == "AUDUSDp" + + +def test_unknown_symbol_falls_back_to_appending(terminal_dir, monkeypatch): + """Neither form cached — a stale cache must not block a valid new listing.""" + _configure(monkeypatch, ".i") + symbol_cache.save(str(terminal_dir), ["EURUSD.i", "XAUUSD"]) + assert _remap("NEWPAIR") == "NEWPAIR.i" + + +def test_empty_symbol_is_left_alone(terminal_dir, monkeypatch): + _configure(monkeypatch, ".i") + assert _remap("") == "" + + +# ── Cache module ───────────────────────────────────────────────────── + + +def test_save_then_load_roundtrips(tmp_path): + d = str(tmp_path) + assert symbol_cache.save(d, ["EURUSD.i", "XAUUSD"]) is True + assert symbol_cache.load(d) == {"EURUSD.i", "XAUUSD"} + + +def test_load_returns_none_when_absent(tmp_path): + assert symbol_cache.load(str(tmp_path)) is None + + +def test_save_refuses_an_empty_list(tmp_path): + """An empty write would turn 'unknown' into 'nothing exists' and suppress + every remap.""" + assert symbol_cache.save(str(tmp_path), []) is False + assert symbol_cache.load(str(tmp_path)) is None + + +def test_save_leaves_no_temp_files_behind(tmp_path): + d = str(tmp_path) + symbol_cache.save(d, ["EURUSD.i"]) + leftovers = [n for n in os.listdir(d) if n.endswith(".tmp")] + assert leftovers == [] + + +def test_save_is_atomic_over_an_existing_cache(tmp_path): + d = str(tmp_path) + symbol_cache.save(d, ["OLD"]) + symbol_cache.save(d, ["NEW1", "NEW2"]) + assert symbol_cache.load(d) == {"NEW1", "NEW2"} + + +def test_cache_records_when_it_was_written(tmp_path): + d = str(tmp_path) + symbol_cache.save(d, ["EURUSD.i"]) + with open(symbol_cache.cache_path(d)) as fh: + payload = json.load(fh) + assert isinstance(payload["updated"], int) + assert symbol_cache.age_seconds(d) is not None + assert symbol_cache.age_seconds(d) < 5 + + +# ── Staleness: the cache must not be authoritative forever ─────────── + + +def _age_cache(d, updated): + """Rewrite the cache's updated stamp in place.""" + path = symbol_cache.cache_path(d) + with open(path) as fh: + payload = json.load(fh) + payload["updated"] = updated + with open(path, "w") as fh: + json.dump(payload, fh) + + +def test_a_stale_cache_is_no_cache(tmp_path): + """load() itself enforces the age. Enforcing it anywhere else means a + caller can forget to, which is exactly what production did: age_seconds() + existed and nothing called it.""" + d = str(tmp_path) + symbol_cache.save(d, ["XAUUSD"]) + _age_cache(d, 1) # epoch: ~1.7 billion seconds old + assert symbol_cache.load(d) is None + + +def test_a_cache_missing_its_timestamp_is_no_cache(tmp_path): + d = str(tmp_path) + symbol_cache.save(d, ["XAUUSD"]) + path = symbol_cache.cache_path(d) + with open(path) as fh: + payload = json.load(fh) + del payload["updated"] + with open(path, "w") as fh: + json.dump(payload, fh) + assert symbol_cache.load(d) is None + + +@pytest.mark.parametrize("updated", ["yesterday", None, -5, 0, 3.7, True]) +def test_a_cache_with_a_malformed_timestamp_is_no_cache(tmp_path, updated): + d = str(tmp_path) + symbol_cache.save(d, ["XAUUSD"]) + _age_cache(d, updated) + assert symbol_cache.load(d) is None + + +def test_a_cache_within_the_window_is_served(tmp_path): + d = str(tmp_path) + symbol_cache.save(d, ["XAUUSD"]) + assert symbol_cache.load(d) == {"XAUUSD"} + + +def test_the_age_limit_is_configurable_per_call(tmp_path, monkeypatch): + import time as _time + d = str(tmp_path) + symbol_cache.save(d, ["XAUUSD"]) + _age_cache(d, int(_time.time()) - 120) + assert symbol_cache.load(d, max_age_seconds=60) is None + assert symbol_cache.load(d, max_age_seconds=3600) == {"XAUUSD"} + + +def test_a_stale_bare_symbol_cache_falls_back_to_the_suffix( + terminal_dir, monkeypatch +): + """The integration-level case from review, through the real INI builder + path: a stale cache that knows XAUUSD as bare must NOT keep suppressing + the remap — the broker may have moved the symbol since. Stale means the + conservative append-always fallback, same as no cache at all.""" + _configure(monkeypatch, ".i") + symbol_cache.save(str(terminal_dir), ["XAUUSD"]) + + # Fresh cache first, proving the suppression is live and the staleness is + # what flips it — not some other reason to append. + assert _remap("XAUUSD") == "XAUUSD" + + _age_cache(str(terminal_dir), 1) # epoch + assert _remap("XAUUSD") == "XAUUSD.i" diff --git a/tests/test_terminal_instances.py b/tests/test_terminal_instances.py index 4f78c0d6..feadee76 100644 --- a/tests/test_terminal_instances.py +++ b/tests/test_terminal_instances.py @@ -334,3 +334,50 @@ def test_container_healthcheck_probes_only_this_vms_ports(): ) assert "awk" in src and "groupfile=" in src, "healthcheck.sh no longer wires a groupfile into awk" + + + +def test_terminals_command_is_scoped_to_this_vms_group( + vm_grouped_terminals_config, tmp_path, monkeypatch, capsys +): + """`terminals` drives what start.bat actually launches, so it must honour + the group file the same way check_health.py does. + + Unfiltered, every VM in a multi-VM install prepares a data dir and starts an + API process for ALL terminals — including another VM's live-mode terminal, + whose portable dir sits on the same shared mount, so the two terminal64.exe + fight over MT5's single-instance lock and one exits silently with code 0. + Regression: the filter hooks shipped for check_health.py but this command + never called them. + """ + helper = _load_config_helper_module() + shared = _write_group_file(tmp_path, "darwinex live\ndarwinex live b\n") + monkeypatch.setattr(helper, "CONFIG_PATH", str(vm_grouped_terminals_config)) + monkeypatch.setattr(helper, "_SHARED_DIR", str(shared)) + + monkeypatch.setattr("sys.argv", ["config_helper.py", "terminals"]) + helper.main() + out = capsys.readouterr().out.strip().splitlines() + + # Only the fast VM's two terminals, and specifically NOT the bulk VM's. + assert out == [ + "darwinex live default 6001 0 live", + "darwinex live b 6002 0 live", + ] + + +def test_terminals_command_unfiltered_without_a_group_file( + vm_grouped_terminals_config, tmp_path, monkeypatch, capsys +): + """No group file is a single-VM install: every terminal stays in scope, the + same no-filter fallback _vm_group_filter() documents.""" + helper = _load_config_helper_module() + shared = _write_group_file(tmp_path, None) + monkeypatch.setattr(helper, "CONFIG_PATH", str(vm_grouped_terminals_config)) + monkeypatch.setattr(helper, "_SHARED_DIR", str(shared)) + + monkeypatch.setattr("sys.argv", ["config_helper.py", "terminals"]) + helper.main() + out = capsys.readouterr().out.strip().splitlines() + + assert len(out) == 4