Skip to content

feat(scheduled): address schedule assignments by id for multi-instance scheduling#280

Merged
jeroenrinzema merged 5 commits into
mainfrom
feat/schedule-instance-id
Jun 29, 2026
Merged

feat(scheduled): address schedule assignments by id for multi-instance scheduling#280
jeroenrinzema merged 5 commits into
mainfrom
feat/schedule-instance-id

Conversation

@jeroenrinzema

@jeroenrinzema jeroenrinzema commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Why

Today a subject can hold only one schedule assignment per named schedule: the
user_schedules / organization_schedules rows are unique on (subject, schedule_id),
and since schedule_id is resolved from the name, submitting the same name again
overwrites the existing assignment. There was no way to schedule the same thing twice
for one user, or to reschedule/delete a specific instance.

What

Schedule assignments are now addressed by their own id:

  • Submit with an existing assignment id → upsert that row in place.
  • Submit without an id → create a new instance. A subject can now hold multiple
    assignments to the same named schedule.

The schedule definition (schedules, unique per project_id + name) is unchanged —
this only affects the per-subject assignment layer.

Changes

  • Migration 1784600000_schedule_multi_assignment drops the unique
    (user_id, schedule_id) / (organization_id, schedule_id) indexes (the non-unique
    lookup indexes are kept).
  • Store UpsertUserSchedule / UpsertOrganizationSchedule take an id and
    ON CONFLICT (id), guarded by user_id/schedule_id (and the org equivalent) so a
    supplied id cannot hijack another subject's row — returns ErrScheduleOwnershipMismatch
    otherwise. A nil id mints a fresh one.
  • Client API:
    • upsert requests gain an optional id; the 202 response returns the assignment
      id
      (matching its documented "scheduled instance" meaning), threaded via
      ScheduledMsg.assignment_id.
    • delete requests gain an optional id to remove a single instance
      (ownership-checked); without an id, delete still removes all assignments by name.
      name is now optional (either id or name required).
  • Management API: same optional id on upsert, so the console can target a specific
    instance.
  • Journeys: a schedule step now creates a new assignment per journey entry (each
    pass of a user through the journey), so re-entering schedules an additional instance.
    The id is derived deterministically from (journey_entry_id, step_id) so a
    redelivered/retried step message updates the same instance instead of duplicating it.
  • Regenerated client + management Go oapi and the console TS types.

Semantic changes to be aware of

  • The management PUT .../scheduled is no longer idempotent-by-name — without an id it
    creates a new instance
    . Idempotency is now by id (resubmit with the returned id).
    Tests updated (TestUpsert*ScheduledIdempotent now pass the id; added
    TestUpsertUserScheduledCreatesNewInstance).
  • The client 202 response id is now the assignment id rather than a throwaway
    pubsub-envelope id.

Console

No console code change required: the scheduled table is already keyed per-instance (no
dedup), so multiple same-named assignments render as separate rows, and per-instance
reschedule already exists (expand a row → edit the time → PATCH .../scheduled/{id}).
With this PR, "two schedules same name" + "reschedule one" work end-to-end.

Follow-up (separate repos)

  • External SDK clients need regenerating to expose the new id fields (tracked
    separately).

Tests

  • Store (testcontainers): upsert tests moved to the id model; added
    TestUpsertUserScheduleCreatesDistinctInstances and
    TestUpsertUserScheduleOwnershipMismatch. ✅
  • Management controller scheduled tests, plus
    TestHandleScheduleMultipleEntries (per-entry create + same-entry idempotency). ✅

…e scheduling

Schedule assignments are now addressed by their own id. Submitting with an
existing assignment id upserts that row; omitting it creates a new instance, so
a single subject can hold multiple assignments to the same named schedule
(e.g. several reminders of the same kind, or rescheduling a specific one).

- migration drops the unique (user_id, schedule_id) / (organization_id,
  schedule_id) indexes; the non-unique lookup indexes are kept
- UpsertUserSchedule / UpsertOrganizationSchedule take an id and conflict on
  (id), guarded by user/schedule so a supplied id cannot hijack another
  subject's row (ErrScheduleOwnershipMismatch otherwise)
- ScheduledMsg carries assignment_id; the client API exposes an optional id on
  the upsert requests and returns the assignment id in the 202 response; the
  management API exposes an optional id so the console can target an instance
- journey schedule steps derive a deterministic assignment id from
  (user, schedule) so re-runs/retries stay idempotent (no behaviour change)
- regenerated client + management Go oapi and the console TS types
…elete-by-id

Follow-up to the id-based assignment change:

- Journey schedule steps now create a NEW assignment per journey entry (a user's
  pass through the journey), so re-entering schedules an additional instance.
  The id is derived from (journey_entry_id, step_id) so a redelivered/retried
  step message stays idempotent instead of duplicating.
- Client API delete now accepts an optional id to remove a single assignment
  (ownership-checked); without an id it still deletes every assignment by name.
  name is now optional (either id or name required); regenerated client oapi.
The generated files were produced with v2.5.1 locally, which diverges from the
CI-pinned v2.7.0 and broke the generate check. Regenerated with v2.7.0; the diff
vs main is now scoped to the new schedule id fields only.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the scheduling assignment layer so per-subject schedule assignments are addressed by assignment id (rather than being uniquely keyed by (subject, schedule_id)), enabling multiple instances of the same named schedule per user/organization while still allowing safe idempotent updates when an assignment id is supplied.

Changes:

  • Drop the unique (user_id, schedule_id) / (organization_id, schedule_id) indexes to allow multiple assignments per named schedule.
  • Update store upserts to ON CONFLICT (id) with an ownership guard, returning ErrScheduleOwnershipMismatch when an id targets a different subject/schedule.
  • Thread optional assignment id through client + management APIs and pubsub messages; journeys derive deterministic assignment ids per (journey_entry_id, step_id).

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/store/subjects/scheduled.go Switch assignment upserts to be keyed by assignment id with ownership guard + new mismatch error.
internal/store/subjects/scheduled_test.go Update/add store tests for id-based upsert behavior, multi-instance creation, and ownership mismatch.
internal/store/subjects/migrations/1784600000_schedule_multi_assignment.up.sql Remove unique assignment-per-(subject,schedule) constraint to enable multiple instances.
internal/store/subjects/migrations/1784600000_schedule_multi_assignment.down.sql Recreate the unique indexes on rollback (with caveat about existing duplicates).
internal/pubsub/schemas/events.go Add assignment_id to scheduled pubsub message schema.
internal/pubsub/consumer/scheduled.go Consume assignment_id when upserting user/org assignments; treat ownership mismatch as permanent failure.
internal/journeys/schedule.go Generate deterministic assignment ids per journey entry + step to avoid duplicates on retries/redelivery.
internal/journeys/schedule_test.go Add coverage ensuring per-entry instance creation and same-entry idempotency.
internal/http/controllers/v1/management/scheduled.go Management upsert supports optional assignment id; returns 400 on ownership mismatch.
internal/http/controllers/v1/management/scheduled_test.go Update management tests for id-based idempotency; add “creates new instance without id” test.
internal/http/controllers/v1/management/oapi/resources.yml Extend management upsert request schemas with optional assignment id.
internal/http/controllers/v1/management/oapi/resources_gen.go Regenerated management Go types for optional assignment id.
internal/http/controllers/v1/client/scheduled.go Client endpoints mint/accept assignment ids, return assignment id in 202, and support delete-by-id vs delete-by-name.
internal/http/controllers/v1/client/oapi/resources.yml Extend client schemas with optional assignment id; make delete name optional.
internal/http/controllers/v1/client/oapi/resources_gen.go Regenerated client Go types reflecting optional id / optional name on delete.
console/src/oapi/management.generated.ts Regenerated console TS management types for optional assignment id on upsert.
Files not reviewed (1)
  • internal/http/controllers/v1/client/oapi/resources_gen.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/pubsub/consumer/scheduled.go Outdated
Comment thread internal/pubsub/consumer/scheduled.go Outdated
Comment thread internal/http/controllers/v1/client/oapi/resources.yml
…dentifier

Addresses PR review:
- Consumer derives a deterministic assignment id from (subject, schedule) when a
  ScheduledMsg omits assignment_id, so producers that don't set one (user/org
  anniversary schedules) stay idempotent under at-least-once redelivery instead
  of minting a fresh id per delivery and duplicating the assignment. The client
  API still sends an explicit id, so its create-new-per-submit behaviour is
  unchanged.
- DeleteUserScheduledRequest now requires identifier in the schema (the handler
  already rejected requests without one); regenerated client oapi and updated the
  handler for the now non-pointer identifier.
- Added a unit test for resolveAssignmentID.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • internal/http/controllers/v1/client/oapi/resources_gen.go: Generated file

Comment thread internal/store/subjects/scheduled.go Outdated
Comment thread internal/store/subjects/scheduled.go Outdated
The UpsertUserSchedule / UpsertOrganizationSchedule doc comments still described
the old (subject, schedule) conflict model. Updated to reflect that assignments
are keyed by their own id (nil creates a new instance; an existing id updates in
place, guarded by ErrScheduleOwnershipMismatch).
@jeroenrinzema jeroenrinzema merged commit ca8282f into main Jun 29, 2026
6 checks passed
@jeroenrinzema jeroenrinzema deleted the feat/schedule-instance-id branch June 29, 2026 21:43
jeroenrinzema added a commit that referenced this pull request Jun 29, 2026
…e scheduling (#280)

* feat(scheduled): address schedule assignments by id for multi-instance scheduling

Schedule assignments are now addressed by their own id. Submitting with an
existing assignment id upserts that row; omitting it creates a new instance, so
a single subject can hold multiple assignments to the same named schedule
(e.g. several reminders of the same kind, or rescheduling a specific one).

- migration drops the unique (user_id, schedule_id) / (organization_id,
  schedule_id) indexes; the non-unique lookup indexes are kept
- UpsertUserSchedule / UpsertOrganizationSchedule take an id and conflict on
  (id), guarded by user/schedule so a supplied id cannot hijack another
  subject's row (ErrScheduleOwnershipMismatch otherwise)
- ScheduledMsg carries assignment_id; the client API exposes an optional id on
  the upsert requests and returns the assignment id in the 202 response; the
  management API exposes an optional id so the console can target an instance
- journey schedule steps derive a deterministic assignment id from
  (user, schedule) so re-runs/retries stay idempotent (no behaviour change)
- regenerated client + management Go oapi and the console TS types

* feat(scheduled): journeys schedule a new instance per entry; client delete-by-id

Follow-up to the id-based assignment change:

- Journey schedule steps now create a NEW assignment per journey entry (a user's
  pass through the journey), so re-entering schedules an additional instance.
  The id is derived from (journey_entry_id, step_id) so a redelivered/retried
  step message stays idempotent instead of duplicating.
- Client API delete now accepts an optional id to remove a single assignment
  (ownership-checked); without an id it still deletes every assignment by name.
  name is now optional (either id or name required); regenerated client oapi.

* fix(scheduled): regenerate oapi with pinned oapi-codegen v2.7.0

The generated files were produced with v2.5.1 locally, which diverges from the
CI-pinned v2.7.0 and broke the generate check. Regenerated with v2.7.0; the diff
vs main is now scoped to the new schedule id fields only.

* fix(scheduled): keep omitted-id messages idempotent; require delete identifier

Addresses PR review:
- Consumer derives a deterministic assignment id from (subject, schedule) when a
  ScheduledMsg omits assignment_id, so producers that don't set one (user/org
  anniversary schedules) stay idempotent under at-least-once redelivery instead
  of minting a fresh id per delivery and duplicating the assignment. The client
  API still sends an explicit id, so its create-new-per-submit behaviour is
  unchanged.
- DeleteUserScheduledRequest now requires identifier in the schema (the handler
  already rejected requests without one); regenerated client oapi and updated the
  handler for the now non-pointer identifier.
- Added a unit test for resolveAssignmentID.

* docs(scheduled): update upsert doc comments for id-based semantics

The UpsertUserSchedule / UpsertOrganizationSchedule doc comments still described
the old (subject, schedule) conflict model. Updated to reflect that assignments
are keyed by their own id (nil creates a new instance; an existing id updates in
place, guarded by ErrScheduleOwnershipMismatch).
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.

2 participants