Skip to content

Start namespace-unique instances for scheduled workflows - #977

Merged
rmcdaniel merged 6 commits into
mainfrom
fix/874-schedule-instance-namespace
Nov 2, 2026
Merged

rmcdaniel merged 6 commits into
mainfrom
fix/874-schedule-instance-namespace

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

Fixes #874.

Problem

Workflow instance IDs are a global primary key, but schedule IDs are unique only within a namespace. The default schedule starter named instances schedule:{schedule_id}:{suffix} without the namespace. As a result, the same schedule ID in two namespaces competed for one instance ID for the same occurrence. The second namespace's backfill, tick or manual trigger then failed with Workflow instance [...] cannot be reused with a different namespace.

Change

PhpClassScheduleStarter::instanceId(): every new scheduled start, whatever the configured default namespace, now uses

schedule.ns:{sha256(namespace)[0..32]}:{byte length of schedule_id}:{schedule_id}:{suffix}

  • Valid: it uses only the characters WorkflowInstanceId allows (A-Z a-z 0-9 . _ : -).
  • No legacy collisions: it can never equal an original schedule:{schedule_id}:{suffix} ID; the two forms differ at the 9th character.
  • Unambiguous: the length prefix fixes where the schedule ID ends, even when it contains :.
  • Safe fallback: if the result isn't a valid instance ID (it exceeds 191 bytes, or the schedule ID uses other characters), the schedule ID is replaced by h:{sha256(schedule_id)}. h can't be read as a length.
  • Existing IDs: instances already stored under the original form stay readable. Occurrences that already fired stay deduplicated, because that check uses the schedule history (hasFiredOccurrence).

The metadata prepares Core 2.15.15 (Composer product-train and the changelog heading). The tag isn't created.

Tests

tests/Feature/V2/V2ScheduleNamespaceInstanceIdTest.php:

  • Backfill: the same occurrence backfilled in two namespaces starts one instance in each, and a repeated backfill is deduplicated.
  • Same millisecond: a manual trigger and a tick in the same millisecond, in both namespaces.
  • Default change: changing the configured default namespace, from tenant-a to tenant-b, doesn't produce a shared ID.
  • Pre-upgrade history: an older release's run under the original ID form, with its ScheduleTriggered history recorded, stays readable. Its occurrence stays deduplicated, and the next occurrence starts under the new form without colliding.
  • Format: IDs pass the validator; delimiters can't shift the tuple boundaries; long or out-of-alphabet schedule IDs produce distinct, valid, bounded IDs.

Replay corpus: schedule-namespace-instance-identity, plus an assertion keyed to that fixture in V2EmbeddedReplayRegressionCorpusTest. The starter sits under the src/V2/Support/*.php replay guard, and this backfill fails on main.

Evidence

Focused before/after (General's containers: PHP 8.3.36, PostgreSQL 17.11, Redis 7.4), covering V2ScheduleNamespaceInstanceIdTest, V2ScheduleTest, V2ScheduleTimezoneTest and V2EmbeddedReplayRegressionCorpusTest:

Code under test Tests Assertions Result
Unchanged main (b73a55e4) with the current tests 125 1832 4 failures, 2 errors: the real same-millisecond namespace collision throws, and the new helper API doesn't exist on main
Fix (df1bcc69) 125 2219 all pass

The backfill, default-change and corpus controls reproduce the original collision on main. Focused PHPStan is clean on the 3 changed PHP files. e240005f is an ECS-only formatting delta, and 230b6a93 adds the 2.15.15 metadata.

First candidate (cacc81bf), corrected before this head: 124 tests, 4 errors and 1 failure. The schedule@ form failed WorkflowInstanceId validation. The new tests used a Carbon Z timezone that the test clock rejects. And keeping the legacy form for the configured default namespace still collided after that default changed.

Normal PR checks and one full build on this exact head will follow.

🤖 Generated with Claude Code

rmcdaniel and others added 6 commits November 2, 2026 07:15
…#874)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Workflow instance IDs are global but schedule IDs are unique per namespace,
so the same schedule ID in two namespaces collided on the same occurrence.
Schedules outside the configured default namespace now use a reserved
schedule@ form with a namespace hash and a length-prefixed schedule ID,
bounded to the 191-byte instance key; default-namespace IDs are unchanged.

Refs #874

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use a valid schedule.ns:{namespace hash}:{length}:{schedule_id}:{suffix}
form for all new scheduled starts, independent of the configured default
namespace, and hash schedule IDs that exceed the bound or the instance ID
alphabet. Tests use the established UTC test clock and add validator,
default-namespace change and pre-upgrade schedule-history dedupe controls.

Refs #874

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Nov 2, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security bounded source/metadata review of exact head 230b6a9348b54ea91ee045799f7cfb8a57eadd56: source-clear, with no Security hold.

The new default-starter identity is structurally separate from the legacy schedule: form, binds the stored schedule namespace through a 128-bit SHA-256 prefix, and uses an explicit byte-length delimiter around a valid raw schedule ID. Invalid/oversized raw forms fall back to a full SHA-256 schedule digest under the same namespace prefix. The raw and hashed subforms are disjoint (<digits>: versus h:), the controlled timestamp/backfill suffix stays inside the existing instance-ID alphabet, and the final result is checked by the existing 191-byte validator. This closes the cross-namespace global-primary-key collision without exposing the namespace text.

The correction uses the schedule row namespace rather than mutable current configuration. The default-change, same-millisecond manual/tick, two-namespace backfill, delimiter, bounded fallback and repeat-backfill controls cover the important collision paths. Legacy IDs remain loadable because no reader changes; an already-fired legacy occurrence remains deduplicated by its real schedule-history event, while the next occurrence moves to the new form. The replay-corpus assertion exercises the same default starter rather than only inspecting a helper.

The final delta from executed source e240005f6b298f52a6bd75b3023e5692ef6ed4e0 is only CHANGELOG.md and composer.json; the src, tests, resources and .github tree IDs are identical. Product-train and changelog prepare 2.15.15 consistently, and no 2.15.15 tag or GitHub release exists. git diff --check passes.

General has recorded the focused PostgreSQL/Redis counterfactual and positive run. Current exact-head hosted checks are green except the still-running ordinary unit-contract aggregate; the one full build, protected merge/release, and installed published-artifact acceptance remain independent gates. This review creates no PHP Server release.

@rmcdaniel

Copy link
Copy Markdown
Member Author

General's corrected-source qualification passes at df1bcc69c81e4cf5b8466edb25a68d32f99f54d7, carried through the single ECS formatting line at e240005f and the mechanical Core 2.15.15 metadata at current head 230b6a9348b54ea91ee045799f7cfb8a57eadd56.

  • PHP 8.3.36, PostgreSQL 17.11, Redis 7.4: all 125 requested tests / 2,219 assertions pass, covering namespace backfill, same-millisecond trigger/tick, configured-default changes, valid bounded/delimited IDs, stored legacy IDs and real recorded-occurrence deduplication, plus the existing schedule/timezone and official replay corpus classes.
  • The same current test files against unchanged b73a55e4 complete 125 tests / 1,832 assertions, four failures and two errors. The corpus, backfill and configured-default cases expose the original collision; manual trigger throws the actual cross-namespace collision. The remaining error is the new helper API absent on base, so it is not separately claimed as runtime defect evidence. The legacy-read/history control also proves old IDs remain loadable while new admissions use the corrected form.
  • Focused PHPStan passes all three changed PHP files. ECS identified exactly one call-formatting line, now corrected. The runtime behavior is unchanged by that line or the metadata carry.

The first cacc81bf candidate was rejected after execution showed an invalid @ ID prefix, Carbon's rejected Z test timezone, and an executed mutable-default collision. The current source corrects all three, with no validator weakening or migration of existing IDs.

Normal PR checks and the single full build are the remaining source qualification gates. General retains protected merge, the routine Core patch publication and installed-package acceptance. No PHP Server release or native source change is included.

@codecov

codecov Bot commented Nov 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.00%. Comparing base (b73a55e) to head (230b6a9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #977   +/-   ##
=======================================
  Coverage   93.99%   94.00%           
=======================================
  Files         592      592           
  Lines       81998    82000    +2     
=======================================
+ Hits        77077    77082    +5     
+ Misses       4921     4918    -3     
Flag Coverage Δ
v2 94.00% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Status at exact head 230b6a9348b54ea91ee045799f7cfb8a57eadd56:

  • PR build 41466830677 succeeded: 19 checks passed and 7 were skipped by their event conditions. That includes the corpus preflight with schedule-namespace-instance-identity.
  • Full build 41466826959, attempt 1: every job passed except feature-postgresql (1) (job 125350264379, ChildWorkflowSignalingTest) and feature-postgresql (2) (job 125350264363, ContinueAsNewWorkflowTest). Both are v1 tests that hit the known transient harness failure: the queue worker exits because Testbench's vendor/composer/installed.json is missing. No schedule code is involved. This is the same failure class Security accepted as a flake on Fix repeated DST schedule occurrences for Core 2.15.13 #974.
  • I requested a scoped rerun of only those failed jobs (attempt 2), with no source change and no full rerun. That result is pending.

Next action: if attempt 2 passes, General takes review, merge, Core 2.15.15 publication and installed acceptance. If it fails again on these tests, treat it as a harness defect to investigate. Don't merge unchanged.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Full build 41466826959 attempt 2 succeeded at exact head 230b6a93. The scoped rerun covered only the PostgreSQL shards that failed in attempt 1 (jobs 125361815360 and 125361815597, plus their reshards), and all passed, as did the build aggregate (125366552174). Attempt 1's two failures stay recorded above as the transient Testbench installed.json harness failure. PR checks are green.

Source handoff: no further edits are planned at 230b6a9348b54ea91ee045799f7cfb8a57eadd56. General now owns review, merge, Core 2.15.15 publication and installed acceptance.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 2, 2026 15:28
@rmcdaniel
rmcdaniel merged commit a8a552b into main Nov 2, 2026
73 of 76 checks passed
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.

Start separate scheduled runs for the same schedule ID in different namespaces

1 participant