Skip to content

Fix Chrome supervisor post-done window starting before reporter_done - #1893

Closed
leehack wants to merge 1 commit into
mainfrom
test/1892-supervisor-timing
Closed

leehack wants to merge 1 commit into
mainfrom
test/1892-supervisor-timing

Conversation

@leehack

@leehack leehack commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Fixes #1892

Summary

  • Root cause: the supervisor set done_at = now, where now was read at the top of the loop before process_table(), so the post-done window started before the reporter_done event was logged. Recovery could fire up to one loop iteration (slow ps under load) less than post_done after the logged event: 0.598 vs 0.6 in CI. This was a real supervisor bug, not a test measurement error (both sides already use the monotonic clock).
  • Fix: take done_at from time.monotonic() right after recording reporter_done. The test compares at the events' millisecond rounding precision, so float rounding of the rounded timestamps cannot fail it.

Production-readiness scope

  • User-facing scope: None; CI tooling only.
  • Supported platforms/paths: Linux CI Chrome supervisor.
  • Unsupported or intentionally unavailable paths: N/A
  • Out of scope / follow-ups: None

Completeness checklist

  • Declared scope is fully implemented.
  • Unsupported platform/option combinations: N/A (no behavior surface change).
  • Public API docs, README/website docs, examples, support matrices, changelog: N/A, not user-visible.
  • Regression coverage: existing test test_recovery_happens_no_earlier_than_post_done_seconds_after_done; mutant below.
  • Validation coverage registry: N/A.
  • Security/privacy review: no secrets involved.
  • Follow-ups: None.

PR type guidance

  • Bugfix PR: root cause above; regression test is the existing one, which fails when post_done is halved in the supervisor (0.352 not >= 0.6).

Test Plan

  • dart run tool/prepare_workspace.dart
  • dart format --output=none --set-exit-if-changed .
  • dart analyze
  • dart test -p vm -j 1 --exclude-tags local-only (CI at 34f8911d: Test Linux VM 1–3 green)
  • dart test -p chrome --exclude-tags local-only N/A, CI tooling only
  • coverage-lib N/A, no lib/ change
  • Other: test_supervise_chrome_test.py (Linux-only) run in python:3.12-slim docker, repeated 30x with --cpus 1; macOS skips it.

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
high-risk tier (regressionPolicy surface) CI supervisor timing only Linux docker, --cpus 1 under stress-ng Pass Independent audit PASS at 34f8911d: head 80/80 pass under load; base 3/80 fail (reproduces the flake); old done_at = now 9/80 fail; halved post-done 80/80 fail. Evaluator exit 2.

Review Notes

  • Independent review status: independent audit PASS at 34f8911d, no blocking findings; follow-up (deterministic injected-delay regression test) logged on 0.19.x audit follow-ups (index) #1861
  • CI status / head SHA: green at 34f8911daaf2e8ff1ed29a82962d74cbd06b27c9 (33 pass, 8 skipped)

High-risk regression review

  • Classification: high-risk (surface: regressionPolicy)
  • Implementation task: this PR
  • Independent blocking QA task: fresh auditor session, PASS, 0 PR-caused P1, 0 unresolved threads
  • Exact head / current base: 34f8911d / main 77b01da1f
  • Production-branch deletion, bypass, or miswire proof: N/A
  • Affected-family real-model/artifact evidence: N/A, no model-dependent behavior
  • Explicit unavailable-family or other N/A evidence: N/A
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Chat app preview removed for leehack/llamadart-chat-pr-1893.

@leehack

leehack commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

Landed on main via batch integration PR #1895 (rebase merge, main ba4e0e1b; tree identical to the audited integration head 5ffd1224).

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.

chrome_test_supervisor_test timing flake: recovery measured at 0.598 s against a 0.6 s floor

1 participant