Skip to content

[P0] Replace uncontrolled wall-clock assertions in watch and refresh tests with deterministic scheduling #689

Description

@mohanagy

Parent: #654
Target branch: next
Blocks: #654 completion, merge of #681 and #680, and start of #657

Outcome

Make watcher, refresh, and backoff tests prove policy and state transitions deterministically instead of depending on narrow real-time windows controlled by the OS scheduler, runner load, timer jitter, or process contention.

The immediate known failure is the idle-backoff assertion in tests/unit/watch.test.ts near the previously reported line 478. The test requires the system to reach a specific backoff rung inside a fixed wall-clock window while also remaining below a reconciliation-count ceiling. That combination is scheduler-sensitive and cannot be made trustworthy merely by increasing the timeout.

Confirmed evidence

During repeated protected-CI qualification for #654:

  • one six-lane matrix failed on the watch.test.ts idle-backoff timing assertion;
  • the test must reach the 80 ms backoff rung inside a fixed 190 ms window;
  • it simultaneously constrains the number of reconciliations to at most five;
  • widening the outer timeout does not solve the contradictory timing dependency;
  • related failures observed during the same roadmap wave were also wall-clock assertions over timing the test did not control.

This is distinct from the local forks-worker startup symptom. Do not conflate the two failure classes.

Scope

1. Reproduce and model the failing contract

For the known watch.test.ts case, document:

  • exact test name and current assertions;
  • production state machine or backoff policy being exercised;
  • timers, sleeps, polling, filesystem events, and callbacks involved;
  • lower and upper wall-clock bounds;
  • reconciliation-count assumptions;
  • why a slow or fast scheduler can fail the test without a product defect;
  • whether the failure reproduces under CPU contention, timer jitter, fake time, or controlled scheduler input.

2. Separate policy tests from integration tests

Backoff and scheduling policy should be tested as deterministic state transitions.

Prefer one of these evidence-supported designs:

  • a pure backoff/transition function;
  • an injected clock;
  • an injected scheduler/timer interface;
  • Vitest fake timers with explicit advancement;
  • explicit event/state barriers rather than fixed sleeps.

Do not add an abstraction merely for architectural style. Use the smallest seam that makes the behavior deterministic and understandable.

3. Audit the related test corridor

Audit tests and helpers involving:

  • Date.now() / performance.now();
  • real setTimeout / setInterval;
  • fixed sleeps or delay helpers;
  • polling windows;
  • retry/backoff rung timing;
  • watcher idle transitions;
  • auto-refresh scheduling;
  • background refresh;
  • stdio refresh;
  • reconciliation-count ceilings;
  • process-start deadlines where the assertion is about policy rather than real integration latency.

At minimum inspect:

  • tests/unit/watch.test.ts;
  • watcher/background-refresh tests;
  • stdio auto-refresh tests;
  • directly shared timer/backoff helpers.

Classify every discovered test as:

  1. deterministic policy test;
  2. event-driven integration test;
  3. true latency/performance test;
  4. unrelated and out of scope.

Do not rewrite all asynchronous tests indiscriminately.

4. Preserve real integration coverage

Deterministic policy tests must not eliminate proof that real watcher/refresh integration works.

Keep a small number of integration tests that:

  • wait for explicit observable state or events;
  • use generous safety timeouts only as deadlock protection;
  • do not assert narrow scheduler-dependent durations;
  • clean up timers, watchers, handles, child processes, and temporary files.

Explicit non-goals

  • Do not increase timeouts merely to hide timing instability.
  • Do not remove reconciliation/backoff semantics from the assertions.
  • Do not force the full suite to one worker.
  • Do not add retries around failing tests.
  • Do not skip, quarantine, or mark tests flaky.
  • Do not change graph, retrieval, context-pack, extraction, MCP, installer, or release semantics.
  • Do not alter user-visible watcher behavior unless a separately evidenced product defect is found.
  • Do not mix the absorbed forks-worker detection work from the sibling [P0] Stabilize the complete Vitest suite and protected CI merge gate #654 child issue into this PR.

Required characterization tests

Before changing behavior, add or retain evidence for:

  • initial backoff state;
  • successive failure/idle transitions;
  • exact backoff rung sequence;
  • maximum backoff cap;
  • reset after a successful or active transition;
  • cancellation/cleanup;
  • no extra reconciliation after stop;
  • deterministic result independent of real elapsed time;
  • one real integration path reaching the expected observable state.

When a clock or scheduler is introduced, test:

  • monotonic advancement;
  • same-deadline ordering;
  • cancellation;
  • pending-task cleanup;
  • no hidden real timer remains in the deterministic policy test.

Validation

At minimum run:

npm ci
npm run typecheck
npm run build
npx vitest run tests/unit/watch.test.ts --maxWorkers=1
npx vitest run tests/unit/watch.test.ts --maxWorkers=4
npm run test:run
npm run test:coverage
npm run verify:pack-parity
npm pack --dry-run
npm run registry:validate
npm run release:verify

Also run the affected test repeatedly under:

  • normal local load;
  • controlled CPU contention where practical;
  • supported Node 20 and Node 22 lines through protected CI;
  • Ubuntu, macOS, and Windows protected lanes.

Capture raw logs. A green summary is insufficient when the log contains worker-start failure signatures.

Acceptance criteria

  • The known watch.test.ts failure is reproduced or its scheduler-sensitive contract is proven directly.
  • Backoff policy assertions no longer depend on narrow real-time windows.
  • The expected backoff sequence and reconciliation semantics remain fully asserted.
  • Policy and integration responsibilities are separated explicitly.
  • Real watcher/refresh integration remains covered through observable state or event barriers.
  • No timeout-only workaround, retry, skip, quarantine, or global worker reduction is introduced.
  • The related timing-assumption audit is attached to the PR.
  • Every changed test passes repeatedly under supported local and protected environments.
  • No unexplained timer, watcher, child-process, or file handle remains.
  • Complete protected CI is green on the exact head.
  • Raw logs contain zero known worker-start failure signatures.
  • No unrelated product or roadmap scope enters the diff.

Rollback

Revert the deterministic test seam and tests together. Do not restore the narrow wall-clock assertion as the permanent gate; if the new design proves invalid, return to investigation with the retained reproduction evidence.

Agent handoff

Start from current origin/next in an isolated worktree. Begin with the exact watch.test.ts state machine and a timing-assumption inventory. Keep the PR focused on deterministic scheduling and related tests. Return the reproduction, architecture choice, files changed, before/after assertions, handle cleanup, repeated results, raw-log scans, and remaining uncertainty. Stop after this issue.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions