feat(24_miracle): add isolated Judge adapter runtime - #10
Draft
thugongheng wants to merge 1 commit into
Draft
Annotations
10 errors and 2 warnings
|
Run blocking AI review
Process completed with exit code 1.
|
|
Run blocking AI review:
src/agentbench_frame/games/miracle/match_runner.py#L522
Caller-controlled identifiers are used as filesystem path components without validation%3A game_id becomes artifact filenames here%2C and the newly accepted Run.start run_id is joined verbatim in tracking/run.py. Absolute values or `..` segments can escape the configured roots and truncate or replace unrelated files. Suggestion%3A Require game_id%2C prefix%2C run_id%2C game%2C and agent to be safe single path segments%2C reject absolute paths and separators/dot segments%2C and verify resolved output paths remain under their configured roots before opening files.
|
|
Run blocking AI review:
src/agentbench_frame/games/miracle/runner.py#L42
Neither MatchAttempt nor attempt_to_outcome computes policy hashes%2C so every emitted `policy_source_sha256` value is null. Because evaluated and opponent directories are external and mutable%2C the resulting game records cannot identify which policy code or binary actually produced the measurements. Suggestion%3A Hash the resolved policy entry/binary or a deterministic source-tree manifest before execution%2C persist non-null hashes for both camps%2C and also record the authoritative Judge and vendor hashes in run provenance.
|
|
Run blocking AI review:
src/agentbench_frame/games/miracle/runner.py#L193
The runner calls `Run.finish()` before adding Miracle statistics. A crash or enrichment error in the following step leaves a finished summary.json containing framework defaults%2C including an incorrect 0.0 win rate when there were no valid games%2C despite the module's stated null/NO_VALID_GAMES contract. Suggestion%3A Compute statistics and h2h first and pass them through `Run.finish(extra_summary=...)`%2C so the first persisted finished summary is semantically complete; retain atomic writing inside Run if crash durability is required.
|
|
Run blocking AI review:
vendor/miracle_local/run_match.py#L111
Every AI read starts a new daemon thread. When a read times out%2C the old thread remains blocked on the same stdout pipe; a subsequent read starts another thread%2C so the stale thread can consume a later frame and discard it into an abandoned queue%2C corrupting framing and causing cascading timeouts. Suggestion%3A Use one persistent framed-reader thread and queue per AI%2C or terminate the AI immediately after timeout so no second reader is ever created for the same stream.
|
|
Run blocking AI review:
vendor/miracle_local/run_match.py#L147
Judge and AI stderr are connected to pipes but are not drained until after process cleanup. A strategy or Judge that writes more than the pipe capacity can block before producing its protocol response%2C causing a false timeout or wrapper kill instead of an attributable game result. Suggestion%3A Continuously drain each child's stderr to bounded on-disk evidence while the match runs%2C rather than retaining undrained PIPE handles until shutdown.
|
|
Run blocking AI review:
vendor/miracle_local/run_match.py#L281
The vendor's finally block references `judge_p`%2C `ai0_mp`%2C and `ai1_mp` even when an earlier spawn failed before those variables were assigned. Common startup failures therefore raise UnboundLocalError during cleanup and prevent the structured result-json from being written%2C collapsing the real startup error into result_json_missing. Suggestion%3A Initialize all process variables to None before the try block%2C drain only processes that were successfully created%2C and ensure result-json construction itself cannot be bypassed by partial startup.
|
|
Run blocking AI review:
src/agentbench_frame/games/miracle/match_runner.py#L231
A match can be classified valid even when the trace is missing or malformed%2C and the result-json `timeout` and `ai_error` flags are never consulted. Thus a result explicitly reporting an AI timeout/error can be counted as a capability result when its corresponding trace line is absent or corrupt. Suggestion%3A Require a parse-clean trace with a terminal match_end for valid games%2C reconcile result-json timeout/error flags with trace events%2C and classify missing evidence or any disagreement as an evidence error.
|
|
Run blocking AI review:
src/agentbench_frame/games/miracle/match_runner.py#L523
Collision detection checks only `<tag>.result.json`. Failed or timed-out attempts commonly lack that file while leaving stdout%2C stderr%2C trace%2C or replay evidence; a retry then reuses the tag%2C truncates stdout/stderr%2C appends to the old trace%2C and may overwrite the replay%2C destroying and contaminating audit evidence. Suggestion%3A Treat the existence of any artifact for a tag as a collision and reserve a fresh tag atomically%2C for example with an exclusive per-attempt directory or lock file.
|
|
Run blocking AI review:
src/agentbench_frame/games/miracle/proctree.py#L173
Descendants are reduced to bare PIDs and later force-killed after a grace period without rechecking their create times. If a child exits and its PID is reused during that window%2C cleanup can kill an unrelated process; additionally%2C cleanup_succeeded only checks the root%2C so surviving descendants can be reported as cleaned. Suggestion%3A Capture each descendant's PID and create_time%2C revalidate that identity immediately before every terminate/kill%2C and mark cleanup successful only after every identity-confirmed member of the captured tree is gone.
|
|
Complete job
Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
|
|
Run blocking AI review:
tools/miracle_precheck.py#L220
The precheck manifest hard-codes `g++ 15.2.0 (MinGW)` and `GNU Make 4.4.1` even though the tool executes and logs the locally installed versions. Running it on another machine therefore creates false environment provenance. Suggestion%3A Capture the actual compiler and make version outputs once and write those exact values into both the log and manifest.
|
background
wait
wait-all
cancel
parallel
Loading