Skip to content

fix(#769): drive retry-backoff tests with a simulated actor clock - #770

Merged
zgeoff merged 2 commits into
mainfrom
fix/769-deterministic-retry-clock
Jul 22, 2026
Merged

fix(#769): drive retry-backoff tests with a simulated actor clock#770
zgeoff merged 2 commits into
mainfrom
fix/769-deterministic-retry-clock

Conversation

@zgeoff

@zgeoff zgeoff commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Description

Closes #769

The submitter retry tests ran on real 5–20ms backoff timers, so CI load could slide an extra retry attempt (or one fewer) across an assertion window — the held-count test flaked both directions on GitHub runners. Retry timing is now driven by an injected xstate clock, so a backoff attempt fires only on an explicit clock.increment.

  • createCheckpointSubmitter takes a test-only clock option, forwarded to the parent actor; spawned per-activity children inherit it through the actor system.
  • The clock option is only forwarded when present — an options object carrying clock: undefined clobbers xstate's default system clock.
  • Both submission suites use SimulatedClock; the test-only retryTimings override is gone, tests advance past the production timings instead.
  • Real-time sleeps proving no-further-attempts became clock increments past the backoff cap.
  • Clock joins the readonly-parameter-types allow list: a two-method behaviour handle with no readonly form.

Testing

  • bun run typecheck passes
  • bun run test passes
  • bun run lint passes
  • New tests added for new functionality

The submitter's retry-loop tests ran on real 5-20ms backoff timers, so CI
load could slide an extra retry attempt (or one fewer) across an assertion
window - the checkpoint-submitter held-count test flaked both directions on
GitHub runners while passing locally.

Inject an xstate clock into createCheckpointSubmitter (spawned per-activity
children inherit it through the actor system) and drive both submission test
suites with SimulatedClock: a backoff attempt now fires only on an explicit
clock.increment, making attempt counts exact and removing the real-time
sleeps. The test-only retryTimings override on the submitter goes away -
tests use the production timings and advance the clock past them.

Closes #769
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b04149c6-17f5-4329-8aeb-d708e6be8674

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The submitter now accepts an optional XState clock, and retry-related tests use SimulatedClock advancement with fixed backoff constants instead of per-test real-time timing overrides.

Changes

Checkpoint retry timing

Layer / File(s) Summary
Submitter clock injection
libs/game/idle-client/src/submission/create-checkpoint-submitter.ts, .oxlintrc.json
The submitter accepts an optional actor clock, conditionally passes it to XState, and allowlists the Clock type for readonly-parameter linting.
Activity machine simulated-clock tests
libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts
Activity machine tests use a simulated clock, fixed retry timing input, explicit backoff advancement, and exact retry/state assertions.
Submitter retry-flow tests
libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts
Submitter retry, loop replacement, held checkpoint, failure, and shutdown tests use simulated backoff advancement instead of retryTimings overrides and real-time waits.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • zgeoff/vers#580: Related checkpoint submitter retry/backoff and flushHeld changes.
  • zgeoff/vers#599: Also modifies checkpoint submitter options and callback wiring.
  • zgeoff/vers#743: Related XState checkpoint submitter and retry timer behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: using a simulated actor clock for retry-backoff tests.
Description check ✅ Passed The description is directly related to the PR and accurately describes the simulated-clock retry test changes.
Linked Issues check ✅ Passed The PR addresses #769 by making retry-loop tests deterministic with explicit simulated-clock advancement.
Out of Scope Changes check ✅ Passed The changes stay focused on the retry-test flake fix and supporting clock plumbing, with no clear unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/769-deterministic-retry-clock

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts`:
- Line 47: Update the direct-machine test fixture’s retryTimings to reference
the imported production retry timing constants instead of duplicating literal
maxTimeout and minTimeout values. Keep the test aligned with the production
configuration and preserve the existing retry behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ed0275b3-fe36-4c4f-be04-23a49d5fd479

📥 Commits

Reviewing files that changed from the base of the PR and between 9752a5f and c0abf20.

📒 Files selected for processing (4)
  • .oxlintrc.json
  • libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts
  • libs/game/idle-client/src/submission/create-checkpoint-submitter.test.ts
  • libs/game/idle-client/src/submission/create-checkpoint-submitter.ts

Comment thread libs/game/idle-client/src/submission/checkpoint-activity-machine.test.ts Outdated
@zgeoff
zgeoff merged commit 093e6ea into main Jul 22, 2026
6 checks passed
@zgeoff
zgeoff deleted the fix/769-deterministic-retry-clock branch July 22, 2026 03:21
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.

checkpoint-submitter retry-loop test flakes under CI load

1 participant