[WIP] Update schema approach for schedule enable/disable support

[Copilot]

• Inspect current PR changes and migration/test setup
• Replace create-table stub change with additive disabled_at migration stub
• Update/adjust tests for upgrade-safe migration behavior
• Run targeted tests, then full test suite validation
• Run final review/security validation and report results

Original prompt

Update pull request #7 in DirectoryTree/Cadence by changing the schema approach for schedule enable/disable support.

Context:

• The current PR implements schedule enable/disable support via a nullable disabled_at timestamp.
• Right now, the PR updates the existing migration stub database/migrations/create_schedules_table.php.stub to add the disabled_at column directly.
• For an existing package with current users, this only helps fresh installs and does not provide a clean upgrade path for existing installs.

Task:

• Modify the PR branch associated with PR Implement schedule enable/disable support via disabled_at #7 in DirectoryTree/Cadence.
• Replace the direct modification to database/migrations/create_schedules_table.php.stub with an additive migration strategy.
• Add a new migration stub that adds a nullable disabled_at timestamp column to the schedules table.
• Ensure the migration is reversible in down().
• Update any affected tests so they continue to reflect the intended package migration behavior.
• Keep the rest of the feature intact: model methods, query scopes, and runner behavior should remain unchanged unless necessary.

Constraints:

• Do not open a new PR.
• Work against the existing PR branch.
• Preserve the intent of the feature while making the migration strategy safer for upgrades.

Success criteria:

• Existing installs have a clear migration path for adding disabled_at.
• Fresh installs and tests still work correctly.
• The PR branch is updated with the requested changes.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Thoughts on this PR? Worth implementing in this package?

Assistant: Yes — this looks worth implementing, and the approach in the PR is sensible for this package.

Quick take

I’d rate it as a good fit with low-to-medium risk:

• It adds a capability users will reasonably expect from a scheduling package: temporarily disable a schedule without deleting it.
• The implementation is simple and idiomatic: a nullable disabled_at timestamp, model helpers, query scopes, and runner filtering.
• The behavior is coherent: disabling clears next_run_at, enabling recomputes it from “now.”
• The PR is small and focused: 6 files changed, 5 commits, +137 / -1, with tests added and no review objections yet.

Why it fits this package

For a package like Cadence, “enabled/disabled” is a natural lifecycle state for schedules. Using disabled_at instead of a boolean is a nice choice because it:

• supports simple enabled/disabled checks,
• preserves when a schedule was disabled,
• leaves room for future features like auditing or admin UI,
• maps cleanly to query scopes (enabled(), disabled()).

The runner change is also correct in spirit: only enabled schedules should be considered when dispatching due work.

What I like in the PR

• Data model is straightforward: disabled_at added to the schedules table.
• API is ergonomic: disable(), enable(), isDisabled(), isEnabled().
• Query API is clean: enabled() / disabled() scopes on the builder.
• Runtime behavior is consistent:
  • disabling sets disabled_at
  • disabling nulls next_run_at
  • enabling clears disabled_at
  • enabling recalculates next_run_at from the current time
• Tests were added for:
  • disabling,
  • enabling,
  • enabled/disabled scopes,
  • skipping disabled schedules during schedules:run.

My main caution

The only thing I’d want to think about before merging is schema/backward compatibility.

This PR updates the package’s migration stub to include disabled_at, which is fine for new installs, but existing users who already published or ran the old migration won’t get this column automatically. So if this package supports upgrades in-place, you’ll probably need one of these:

1. a documented upgrade step telling users to add disabled_at, or
2. a new upgrade migration path, if the package supports it.

That’s not a reason to reject the feature — just something to handle explicitly.

Minor review notes

These are not blockers, just things to consider:

• enable() recomputes from now, not from the previously missed cadence.
  I think that’s the right default, but it should be documented because some users may expect “resume where it left off.”
• disable() and enable() always save immediately.
  That’s probably fine and convenient, but if the package tends to keep model methods side-effect light, it’s worth checking consistency.
• Runner filtering is a bit redundant with nulling next_run_at, but still good.
  It makes intent explicit and protects against inconsistent data.

Merge readiness

From what I can see:

• PR status: open, mergeable, clean
• Reviews/comments: none yet
• Scope: small and contained
• Risk: low to medium, mostly around upgrade/migration handling rather than runtime logic

Recommendation

I’d lean yes: merge it, assuming you’re comfo...

This pull request was created from Copilot chat.