test(evolver): validate durable loops in isolated databases

[mirror29]

Summary

Database-dependent loop tests were previously skipped locally, and the existing development database contains synthetic event fixtures. Add a repeatable validation command that creates a fresh local PostgreSQL database, applies all migrations, runs durable-loop checks, writes logs/JUnit results, and removes only its disposable database. Reject non-local database URLs and never invoke paid model calls.

Document the observed validation results and real-data blockers in the current-state document. Existing five-generation development records use synthetic/test events and cannot establish real-market Forward or holdout readiness. Real news archives are not equivalent to extracted event facts.

Validation

• Default validation script: 28 passed, no skips, 3.46 seconds.
• Full script including the real five-generation evaluation engine: 30 passed, no skips, 120.65 seconds; disposable database cleanup confirmed.
• The initial database suite reported 28 passed and 2 failures from the five-generation 240-second wall-clock timeout. Those two tests passed in 71.8 seconds using the supported evaluation concurrency of 4 and one numerical-library thread per process. Assertions, test deadlines, resource limits, and production defaults were not changed.
• Non-local database URL rejected before connecting.
• Evolver Ruff and diff whitespace checks passed.
• Consistency: 19 passed, 0 failures, 120 existing warnings.

Scope

This validates PostgreSQL reuse, competing workers, abrupt worker exit, stale-lease fencing, checkpoints, handoff, budget accounting, and offline five-generation rejection behavior. It does not validate a paid end-to-end experiment, live Forward accumulation, or real-world holdout consumption. Existing development records and credentials were preserved; no service configuration or production behavior changed.

CR follow-up

• Reject connection-target query overrides and explicitly pin local host addresses, ports, and the generated test database for both libpq and SQLAlchemy. This also prevents ambient PGHOSTADDR from redirecting validation.
• Discover test_loop_*.py automatically; five-generation evaluation remains an explicit option.
• Updated to current main, preserving the wallet schema and Evolution UI changes.
• Validation after the fix: 15 offline guard/discovery regressions passed; full disposable-database validation passed 34 tests including five generations, with database cleanup confirmed. Ruff, whitespace and 19 consistency checks passed. No model calls or production writes were performed.

Latest CR follow-up: require the actual opt-in five-generation suite to exist and verify its repository discovery delta; use a bounded connect timeout, 120-second migration limit and 600-second test limit; unwind SIGTERM through cleanup. Document SIGKILL/host failure leftovers explicitly. Timeout cleanup and signal regressions pass (19 total unit tests).

[cloudflare-workers-and-pages]

Deploying inalpha-web with    Cloudflare Pages

Latest commit:	6cec331
Status:	✅  Deploy successful!
Preview URL:	https://51d223f2.inalpha-web.pages.dev
Branch Preview URL:	https://codex-e2-runtime-validation.inalpha-web.pages.dev

View logs

[github-actions]

Review base: e15417a

🤖 DeepSeek V4 Pro PR Review

Review

I rebuilt the intent from the diff: this PR adds services/evolver/scripts/validate_durable_loop.py, which spins up a randomly-named disposable local PostgreSQL database, migrates it, runs the test_loop_* suites against it (with the five-generation suite opt-in), and always drops the database; plus a unit test for the URL-routing guard and doc updates.

I traced the interaction between the URL normalization, the subprocess env, and the test suite. Findings below (nothing below is lint-level).

Findings

1. [major] validate_durable_loop.py:131-152 — admin connection timeout does not actually constrain migration/pytest subprocesses; a hung child keeps the script alive past its own timeouts
admin.execute("SET statement_timeout=15000") only bounds DDL/DML issued on this admin connection. The two subprocess.run(..., timeout=120/600) calls are what actually bound the children, and when they hit TimeoutExpired the handler returns 124 while the finally tries DROP DATABASE ... WITH (FORCE). If the pytest process has spawned the backtest subprocess trees the README advertises ("真实编译器、回测子进程和数据库配合"), WITH (FORCE) terminates the server-side backends, but the local orphaned child processes are never reaped, and subprocess.run's timeout path does not kill the child's process group. Over several CI runs this leaks processes and can leave the DB drop waiting. Evidence: general principle (resource lifecycle / error handling: timeout path must clean up the children it started, not just the database). Repro: make pytest spawn a long-lived worker subprocess, let the 600 s timeout fire, observe the DROP runs while the grandchild is still alive.

2. [medium] validate_durable_loop.py:44 — duplicate/parsed query keys check happens before hostaddr is appended, so a user-supplied hostaddr in the source URL is a confusing reject rather than a normalized-away value
_SAFE_QUERY_OPTIONS rejects hostaddr, host, dbname, port, etc. That is intentional (the test asserts it). However the code appends its own ("hostaddr", address) at line ~64 after validation, and the source URL's host/port from netloc are kept verbatim in the rebuilt URL. For a source URL that already carries host=… in netloc, the effective connection is pinned by the appended hostaddr, which is correct — but the netloc host is still emitted and libpq treats hostaddr as authoritative only when it is a bare IP. If parsed.hostname == "localhost", host stays "localhost" in netloc and hostaddr=127.0.0.1; libpq will use hostaddr for the socket and host for cert/hostname verification. That is the desired pin, so this is fine. The real gap: a source URL of the form postgresql://localhost/dev?application_name=x&application_name=y is rejected, but ?connect_timeout=abc raises ValueError inside int(...) and is re-surfaced as the generic "without target overrides" message — a misleading error for an operator who simply typo'd the timeout. Evidence: general principle (error handling: fail with an accurate cause).

3. [medium] validate_durable_loop.py:96-117 — env overrides DATABASE_URL for the child but the script never verifies the child actually used the test URL; a misconfigured migration could target the dev DB
The script builds test_url, sets both DATABASE_URL and EVOLVER_TEST_DATABASE_URL, then runs Alembic from infra/migrations and pytest from services/evolver. If either tool reads a different env var (e.g. an Alembic sqlalchemy.url in alembic.ini, or the evolver settings reading a DATABASE_* family that is not DATABASE_URL), the migration would run against whatever the ambient config resolves to. The test test_migration_timeout_still_drops_created_database stubs subprocess.run, so it never exercises the real wiring — the guardrail "won't clear the dev DB" therefore rests entirely on the child honoring DATABASE_URL, which is asserted nowhere. Evidence: CLAUDE.md §3.1/§4 (dev-DB safety) + general principle (state/data flow: single source of truth for the target database is not proven). Suggest asserting the resolved target inside the child or failing closed.

**4. [medium] validate_durable_loop.py:1-160 / README— the "no LLM cost" claim for the five-generation run is asserted by docs but not enforced by the script** README: "外部模型为离线替身，不产生 LLM 费用". The script only sets thread-count env vars andCANDIDATE_EVALUATION_CONCURRENCY; it does not force any offline-model flag. If the developer's root .env(which the script happily reads viadotenv_values) has real model credentials/endpoints enabled, the opt-in run *would* make paid calls. Since .envis the documented source and the docs promise "no purchases", the script should pin the offline stub explicitly (or refuse to run--include-five-generations` when a paid endpoint is configured). Evidence: CLAUDE.md §3 (面向全球用户 / cost safety) + general principle (safety: untrusted/unintended external input reaching a paid path).

5. [medium] test_validate_durable_loop.py:56-82 — credential-preservation test only covers a password with @; a URL with user:pass@ where pass contains : or / is not covered, and the rsplit("@", 1) split can mis-split on encoded @
credentials = parsed.netloc.rsplit("@", 1)[0] keeps user:pass@. parsed.netloc has already had %40 decoded by urlsplit to user:pass@ in the netloc only if the raw @ was percent-encoded — actually urlsplit splits on the literal last @, so an encoded %40 stays encoded and rsplit("@",1) finds the real delimiter. That path is safe. But parsed.port/parsed.hostname accesses can raise ValueError for malformed netloc (e.g. user@host:notaport) and that is caught — fine. The uncovered risk is credentials containing a literal @ in the user portion: urlsplit("postgresql://a@b:pw@localhost/db") … this is ambiguous by URL spec and rsplit("@",1) will treat b:pw as part of credentials and a as user, silently shifting the boundary. Evidence: general principle (correctness/contract: parser edge cases for secrets should be tested or the code made explicit). Low confidence — flagging because credentials are involved and a silent mis-parse would connect with wrong creds.

6. [medium] docs/04-current-state.md:596-616 — doc claims a reproducible 30/0 in 120.65 s, but the script's five-generation path is opt-in and the default --evaluation-concurrency is 2, so a bare uv run python scripts/validate_durable_loop.py will not reproduce the documented run
The recorded numbers ("30 项通过、0 跳过，120.65 秒", "以现有配置允许的评估并发 4") come from --include-five-generations --evaluation-concurrency 4, but the README lists the plain invocation first and the doc does not tie the numbers to the exact flags. The script's --evaluation-concurrency choices=range(1,5) caps at 4, matching the doc, but nothing in the doc records the command line. This is a verifiability gap: the next reader cannot reproduce the recorded evidence. Evidence: CLAUDE.md §3 (docs must not overstate) + general principle (reproducibility).

Not flagged

• The PGHOSTADDR pinning via appended hostaddr is correct and well-tested.
• discover_tests fail-closed on empty and on missing opt-in suite is correct.
• Thread-count pinning (OMP_NUM_THREADS etc.) matches the concurrency-4 claim.
• terminate_validation SIGTERM unwinding is correct; SIGKILL residue is documented.

Net: the isolation core (local-only guard, random DB, forced drop) is sound. The gaps are (a) child-process/DB cleanup on timeout, (b) the child's actual DB target and offline-model guarantee being asserted rather than proven, and (c) reproduction/error-message quality. I would not block on 4–6 alone, but 1 and 3 are worth addressing before merge.