Skip to content

fix(supervisor): read reports that outgrew the 8 MiB control-record cap - #69

Open
Huang-404-Q wants to merge 1 commit into
AMAP-ML:mainfrom
Huang-404-Q:fix-oversized-report-read
Open

Huang-404-Q wants to merge 1 commit into
AMAP-ML:mainfrom
Huang-404-Q:fix-oversized-report-read

Conversation

@Huang-404-Q

Copy link
Copy Markdown

What this fixes

report.json embeds the full transcript of every finished round, so a long run (up to MAX_ROUNDS=1000) can legally grow past the 8 MiB control-record cap that the supervisor uses to read JSON status files. _read_json (backed by control_bus._read_json_file) silently returns {} for any file larger than 8 MiB — it never reads the bytes and never complains.

Every lifecycle poller treats {} as no report. The consequences:

  • A completed run (exit 0) whose report grew past the cap is reclassified failed ("worker exited without a final report"), and
  • _persist_failure_report sees an empty report and overwrites the real report on disk with a synthetic failed stub — the completion evidence is physically destroyed, unrecoverable.
  • On a failed run, the report's own failure_reason is discarded in favour of the exit-code fallback text.

Fix

Add _read_report in supervisor/service.py: the same nofollow-anchored bounded read, with the ceiling raised to a 256 MiB hard cap (still bounded, so a hostile run directory cannot pin a parser on a multi-gigabyte file). Over-cap, missing, or non-dict files still return {}, so the fail-closed behaviour is unchanged for genuinely broken reports.

All nine report.json read sites in the lifecycle path switch to _read_report (refresh branches, stop/abort race reconciliation, finalize_attached_run, run listing, historical scan). The one exception is _resume_once's task-field fallback, which only reads the task key while the owner record and ledger remain the authoritative sources — keeping that on the small 8 MiB cap avoids paying for a large JSON parse where the value is never used.

Control records (owner.json, status.json, …) keep the 8 MiB cap; they are small by design and the cap is there deliberately.

Tests

New tests/supervisor/test_oversized_report.py (5 tests, all red on the previous code):

  • A 10 MiB completed report with exit 0 → run classified completed, no crash_report.json, report file byte-for-byte untouched (no stub overwrite).
  • A 10 MiB failed report with exit 1 → failure_reason taken from the report itself, not the exit-code fallback.
  • A sparse file past the 256 MiB ceiling (1 byte written, 256 MiB apparent size) → still treated as no report → failed + crash_report.json, confirming the cap is enforced and fail-closed.
  • _read_report unit tests: parses a >8 MiB report; returns {} for a missing path.
  • _read_report unit test: past-cap sparse file returns {} without reading it.

A finished report embeds every round, so a long run can legally exceed
the cap that the generic control-record reader enforces.  The reader
silently returns an empty dict for such files, every lifecycle poller
treats that as *no report*, and the failure path then overwrites the
real report with a synthetic stub.

Add _read_report with the same nofollow-anchored bounded read and a
256 MiB hard ceiling, and use it at the report read sites.  The
resume task-field fallback keeps the small cap: it only reads the
task key, and the owner record and ledger remain the source of truth.
prax211 added a commit to cogniziocompany/LongHorizon-Harness that referenced this pull request Sep 5, 2026
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
prax211 added a commit to cogniziocompany/LongHorizon-Harness that referenced this pull request Sep 6, 2026
…MAP-ML#68/AMAP-ML#69 merged, AMAP-ML#62 closed

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant