schedulertest: scheduler invariants and deterministic tables#10727
Draft
davidporter-id-au wants to merge 7 commits into
Draft
schedulertest: scheduler invariants and deterministic tables#10727davidporter-id-au wants to merge 7 commits into
davidporter-id-au wants to merge 7 commits into
Conversation
This was referenced Jun 16, 2026
…g schedules The V1 scheduler's automatic CHASM-migration eligibility check read len(s.Info.RunningWorkflows) before the same run-loop iteration's processBuffer() reconciled it. processTimeRange sets NeedRefresh when it buffers a new action, but the refresh that prunes completed executions only runs inside processBuffer -- after the check, which also starts the replacement action. For a continuously-firing schedule the check therefore always saw the previous action as still running, so len(RunningWorkflows)==0 was never observed and, under the default EnableCHASMSchedulerMigrationWithRunningWorkflows=false guard, migration was deferred forever. Reconcile running-workflow status before the eligibility check so the check sees the genuine post-completion window and migrates before the next action starts. Gated behind a new SchedulerWorkflowVersion (RefreshBeforeMigrationCheck = 13) because refreshWorkflows issues recorded local activities and reordering it would otherwise break replay of existing histories. Tests: - workflow_test.go: TestAutoMigrateReconcilesRunningWorkflowBeforeCheck unit guard (fails without the version bump). - testdata/replay_migration_v1_to_v2.json.gz: new replay fixture exercising the v13 migration branch; TestReplays fails if the early-refresh block is reordered/removed. Regenerate via tests/schedule_migration_replay_fixture_gen_test.go (GENERATE_REPLAY_FIXTURE=1, inert in CI otherwise). - tests/schedule_migration_v1_to_v2_callback_compat_test.go: integration coverage; _AutomaticMigrationForActiveSchedule now passes. _NoUnsupportedHistoryEventOnRunningWorkflow stays skipped (separate admin-path callback-attach bug). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
davidporter-id-au
force-pushed
the
feature/chasm-scheduler-invariants
branch
2 times, most recently
from
July 18, 2026 20:15
a814fd8 to
fdf1ccd
Compare
Addresses the PR review notes on the callback-compat tests and clears the golangci failures on the changed files. Test unification (schedule_migration_v1_to_v2_callback_compat_test.go): - Extract shared helpers (createV1Schedule, awaitRunningAction, awaitAnyAction, awaitV1SchedulerCompleted, requireNoOptionsUpdatedEvent, requireV2ScheduleExists) to kill the ~4x-duplicated boilerplate. - Rename the admin-path test to TestScheduleMigrationV1ToV2_AdminMigratePreservesRunningWorkflowHistory (it is the tdbg/CLI poke-migration flow) and tighten the skip reason: the admin MigrateSchedule path still injects the unsupported event and is genuinely unfixed. - Merge the two dynamic-config tests into TestScheduleMigrationV1ToV2_RolloutMigration, which asserts both that a busy schedule migrates (regression guard for the ordering fix in this PR) and that no fired workflow gains EVENT_TYPE_WORKFLOW_EXECUTION_OPTIONS_UPDATED. The old single-action + refresh-nudge scaffolding existed only to sidestep the ordering bug this PR fixes, so it is dropped along with the stale "fails on main" framing and the local-path reference. Lint (golangci diff-mode now clean on the changed files): - Put ctx first in helpers (revive context-as-argument). - require.Eventually -> await.RequireTrue/RequireTruef in the rollout test and the replay-fixture generator (forbidigo). - s.NoError -> s.Require().NoError in the new workflow_test guard (testifylint require-error). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Test drivers that step a component forward need to invoke node-level task dispatch (EachPureTask/ExecuteSideEffectTask) and inspect accumulated physical tasks, which the Engine API did not expose. Add: - NodeForRef / BackendForRef accessors on the in-memory Engine. - WithNodeBackendDecorator option, applied to each execution's MockNodeBackend at creation time, so callers can supply handlers the default leaves unset (e.g. HandleGetNamespaceEntry, or a constant transition counter). All additive and default-nil; existing chasmtest users are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit aeddc4227acc0ec1c8a8b6b0034efa0d4c6bc42b)
Add a harness that builds a real CHASM scheduler on a chasmtest.Engine and steps it forward through every task it schedules for itself, advancing a controllable clock. Pure tasks dispatch via Node.EachPureTask and side-effect tasks via Node.ExecuteSideEffectTask (the production dispatch paths). The driver exposes Step/RunToQuiescence, Pause/Unpause/TriggerImmediately/MigrateToWorkflow operations, a Snapshot of observable state, and an AfterStep hook. This lives in a non-test package so property and migration tests can import it. library.go mirrors the in-package fixtures (NewTestLibrary, DefaultSchedule, ...) but exported and wired with the mock clients the side-effect handlers need. Smoke tests cover the three settled behaviors: a running interval schedule ticks forever, a paused schedule is held open and keeps ticking, and a schedule with no remaining actions idles then closes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 77342fdbbdf5ef7fadaa16553a87c496edb2db81)
- library.go: gofmt the NewSpecBuilder call indentation and import order (both flagged by CI's fmt/gci checks). - driver.go: drop the historyservice/historyservicemock import aliases -- .github/.golangci.yml's importas rule requires api/*service(mock) packages to be unaliased; these matched the package's own name so the alias was redundant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add CheckInvariants, evaluated after every settled transition via an AfterStep hook, encoding the legitimate-quiescence rules from Scheduler.isHeldOpen and getIdleExpiration: - no-stuck (primary): a non-closed schedule may have no pending task only when held open (paused or draining a backfill); any other taskless, non-closed state is the stuck-open bug class. - idle-not-while-held-open: a held-open schedule must not arm an idle close. - closed-is-terminal: a closed schedule never reopens. - high-water-mark monotonicity for the generator and invoker. Deterministic table tests run several schedules (running, paused, exhausted, short-interval) forward with the hook wired in, plus direct unit tests proving the predicate flags a stuck state and an HWM regression while allowing a legitimate held-open-without-task state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 1c2ac604e4638e60b8f2ca180ba25e671f0c936b)
Adds two invariants to CheckInvariants, with the driver instrumentation they need: - buffer-bound: total buffered starts must never exceed MaxBufferSize (the generator sizes each fill against this cap). - action-budget: a single InvokerExecuteTask must not take more actions (start + terminate + cancel RPCs) than MaxActionsPerExecution. Guards the double-spend class where the budget resets across the terminate/cancel/start phases. Driver now counts action RPCs via the mock clients, tracks the max taken by any single side-effect task dispatch, and exposes BufferedStartsCount / MaxBufferSize / MaxActionsObserved / MaxActionsPerExecution on Snapshot. Adds WithConfig so tests can set small Tweakables to exercise the caps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
davidporter-id-au
force-pushed
the
feature/chasm-scheduler-invariants
branch
from
July 18, 2026 23:16
fdf1ccd to
ac58125
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
CheckInvariants, evaluated after every settled transition via anAfterStephook, encoding the legitimate-quiescence rules fromScheduler.isHeldOpenandgetIdleExpiration:Deterministic table tests run several schedules (running, paused, exhausted, short-interval) forward with the hook wired in, plus direct unit tests proving the predicate flags a stuck state and an HWM regression while allowing a legitimate held-open-without-task state.
Stack
Stacked on #10726 (harness) → #10725 (bugfix). Diff against
mainis cumulative; net-new here isinvariants.go+invariants_test.go.Test plan
go test ./chasm/lib/scheduler/schedulertest/ -run 'TestInvariants|TestCheckInvariants'🤖 Generated with Claude Code