From bc436196f7779b380dbf29994cebf6d6f36fe0a4 Mon Sep 17 00:00:00 2001 From: NinjaLikesCheez Date: Sun, 18 Oct 2026 12:45:17 +0200 Subject: [PATCH] Count /schedule edit timing from the edit; refuse dead one-offs A timing edit (when, repeat or days) kept the old lastRunAt, so the runner could post an occurrence of the new timing that fell up to an hour before the edit, right away, while the reply said the next one. Moving the time now sets lastRunAt to max(stored, now), as resume already does. An edit that turns a repeat into a one-off whose start has passed, and a resume of a one-off whose time passed while paused, are now refused with a clear message instead of keeping a schedule that can never post. The `when` option description names the configured PIXEL_TIMEZONE instead of a hard-coded Europe/Amsterdam. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 ++ docs/architecture.md | 2 +- docs/commands.md | 4 +- src/features/schedules/index.ts | 57 +++++++--- src/features/schedules/schedules.test.ts | 129 +++++++++++++++++++++-- 5 files changed, 177 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f5410c..e7feb4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ Until 1.0.0, releases stay on 0.x: `Added` bumps minor, `Fixed`/`Changed` (and t ## [Unreleased] +### Fixed + +- `/schedule edit`: changing `when`, `repeat` or `days` now counts from the edit. Before, a weekly post that last went out on Monday and was moved to Thursday at 19:20 could post straight away for that Thursday 19:00, while the reply said next week. +- `/schedule edit` refuses to turn a repeat into a one-off whose start has passed (add a new `when`), and `/schedule resume` refuses a one-off whose time passed while it was paused (it says to delete it). Both used to keep a schedule that could never post. +- `/schedule` `when` option: its description names the configured `PIXEL_TIMEZONE` instead of always saying Europe/Amsterdam. Re-register the commands (`just register`) for a zone other than Europe/Amsterdam to see it. + ## [0.17.0] - 2026-10-15 ### Added diff --git a/docs/architecture.md b/docs/architecture.md index 24fb7b9..0464028 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -338,7 +338,7 @@ The [spaceapi.io directory](https://api.spaceapi.io/openapi.json) was considered - **Where:** a text or announcement channel picked with the command (a new core `channel` option type). People can only schedule into channels where **they** can post, polls only where they can create polls, and `mentions` only where they can ping everyone (from the permissions Discord reports for the person in that channel). Pixel also checks its own permissions there before saving, and again when posting. - **What:** a plain **message** (up to 2000 characters of markdown; pings are off unless `mentions` is on), or a native **Discord poll** (a question, 2–10 answers of up to 55 characters, open for 1 hour to 2 weeks, single or multiple choice). Discord counts the votes, shows them live and announces the result when it closes, so Pixel stores nothing about votes. User-set text can include tokens (`{{date}}`, `{{date+2}}`, and the rest of the date family); they are stored as written and filled from the scheduled occurrence when the post goes out. `/schedule preview` and the private confirmation after create or edit show the same fill as of the next occurrence (message text; poll question and answers). See Interpolator above. - **Forms:** the text, and the poll's question and answers (one per line), are typed in a **modal**, because slash options are one line. Commands declare these as `form` fields on string options. Discord can answer a slash command with a modal **or** a message, not both, so Pixel parses `when` (and the other slash options) **before** opening the modal (`Dispatcher.prepareForm` / `beforeForm`). A bad time is a private error and the modal never opens, so the body isn't typed against a timestamp that would be thrown away. The modal title is the interpreted time. `beforeForm` can also prefill fields (`values`) so an edit opens with the text already saved, and name which declared fields to show (`fields`); an empty list skips the form so `/schedule edit` can list the caller's items. The adapter keeps the typed options under a random token for 15 minutes for that one person, and runs the command when the form is submitted. The dispatcher checks everything again. -- **Edit:** `/schedule edit` is the same access as `/schedule` (member + `schedule-posts`). Only the Discord user who created a schedule (by immutable user ID, `createdBy.ref`) may edit it, and only while it still has an upcoming occurrence (a one-off is removed after it posts; a repeat can still be edited for later occurrences). Channel, mentions, poll duration and multiple-choice are not edited. The list, suggestions and refusals never reveal anyone else's schedules. +- **Edit:** `/schedule edit` is the same access as `/schedule` (member + `schedule-posts`). Only the Discord user who created a schedule (by immutable user ID, `createdBy.ref`) may edit it, and only while it still has an upcoming occurrence (a one-off is removed after it posts; a repeat can still be edited for later occurrences). Channel, mentions, poll duration and multiple-choice are not edited. A change to `when`, `repeat` or `days` counts from the edit (`lastRunAt` moves to now, as on resume), so an occurrence of the new timing that fell before the edit never posts late. An edit that would leave nothing to post (a repeat turned into a one-off whose start has passed) is refused, and so is resuming a one-off whose time passed while paused, so neither keeps a dead schedule. The list, suggestions and refusals never reveal anyone else's schedules. - **When:** Discord has no date picker for bots (no slash option type, no modal component, no Components v2 picker). Autocomplete would force picking from a short list of dates, so `when` is **free-form text**. Pixel parses it (`core/when.ts`: `wed 19:00`, `wed 1900`, `19u30`, `14 oct 19:00`, `tomorrow 9am`, and similar) into one future wall-clock moment in `PIXEL_TIMEZONE` and **confirms that time** in the private reply. Compact 24h (`1900`) and Dutch `19u` are times, not guessed by a date library. A day without a time is 19:00; a time without a day is the next clock hit. `repeat` is once, weekly, fortnightly (on one or more `days`, such as `wed sat` — autocomplete there is only a weekday vocabulary, not dates), monthly or every 2 months (on the start's day of the month, or the month's last day). Times stay on the wall clock through the summer/winter clock change (`core/recurrence.ts`). - **Running:** the feature checks every 30 seconds (`features/schedules/runner.ts`). A due post goes out once; if Pixel was down, it still posts up to an hour late, skips anything later, and only the latest of several missed occurrences can go out. Every occurrence is recorded as handled whether it posted, was skipped or failed, and remembered in memory too, so nothing ever posts twice. Failures are logged and reported, never retried. A one-off is removed once it's handled. - **Storage:** `data/schedules.yaml` (`services/schedules.ts`), written atomically, at most 50 schedules. Unlike the rest of `data/`, it's **not safe to delete**. If it's invalid, nothing is posted or changed (and it's never overwritten) until it's fixed; Pixel logs and reports why. diff --git a/docs/commands.md b/docs/commands.md index 2ed5390..77e940b 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -119,7 +119,7 @@ Every scheduled post, with where and when it goes out next. -Changes one of your own scheduled posts before it goes out: its content, and optionally when and how often. +Changes one of your own scheduled posts before it goes out: its content, and optionally when and how often. A new time or repeat counts from now, so nothing it would have posted before the edit goes out late. Turning a repeat into a one-off needs a new `when`. @@ -137,7 +137,7 @@ Stops a schedule from posting until it's resumed. -Starts a paused schedule again. Nothing missed while paused is posted. +Starts a paused schedule again. Nothing missed while paused is posted. A one-off whose time passed while it was paused can't be resumed: delete it. diff --git a/src/features/schedules/index.ts b/src/features/schedules/index.ts index dcfd7f5..7d7c584 100644 --- a/src/features/schedules/index.ts +++ b/src/features/schedules/index.ts @@ -92,7 +92,7 @@ export function createSchedulesFeature(deps: SchedulesDeps): Feature { }; const whenOption: CommandOption = { name: "when", - description: "When to post (Europe/Amsterdam), e.g. wed 1900, 14 oct 19:00, tomorrow 9am", + description: `When to post (${zone}), e.g. wed 1900, 14 oct 19:00, tomorrow 9am`, type: "string", required: true, }; @@ -267,13 +267,33 @@ export function createSchedulesFeature(deps: SchedulesDeps): Feature { throw new UserFacingError(CANT_EDIT); } - /** Apply optional `when` / `repeat` / `days` on top of what's already stored. */ + /** + * Apply optional `when` / `repeat` / `days` on top of what's already stored. Refuses a + * change that would leave nothing to post (a one-off whose start has passed). + */ function editTiming( schedule: Schedule, args: CommandContext["args"], ): { start: LocalDateTime; recurrence: Recurrence; + } { + const change = parseEditTiming(schedule, args); + const edited = { ...schedule, start: formatLocal(change.start), recurrence: change.recurrence }; + if (!nextRun(edited, zone, now())) { + throw new UserFacingError( + "That would leave it with nothing to post: a one-off needs a time that's still to come. Add `when` with the new time.", + ); + } + return change; + } + + function parseEditTiming( + schedule: Schedule, + args: CommandContext["args"], + ): { + start: LocalDateTime; + recurrence: Recurrence; } { const hasWhen = typeof args.when === "string" && args.when !== ""; const hasRepeat = typeof args.repeat === "string"; @@ -361,11 +381,17 @@ export function createSchedulesFeature(deps: SchedulesDeps): Feature { : { start: parseLocal(schedule.start) as LocalDateTime, recurrence: schedule.recurrence }; const post = editedPost(schedule.post, ctx.args); const name = typeof ctx.args.name === "string" ? cleanName(ctx.args.name) : schedule.name; + const start = formatLocal(timingChange.start); + const retimed = + start !== schedule.start || + JSON.stringify(timingChange.recurrence) !== JSON.stringify(schedule.recurrence); const updated = deps.store.update(schedule.id, { post, - start: formatLocal(timingChange.start), + start, recurrence: timingChange.recurrence, name, + // New timing counts from now, like resume: nothing it "missed" before the edit goes out. + ...(retimed ? { lastRunAt: fromNow(schedule, now()) } : {}), }); if (!updated) refuseEdit(ctx); ctx.logger.info( @@ -596,18 +622,18 @@ export function createSchedulesFeature(deps: SchedulesDeps): Feature { }; }), manage("resume", "Start a paused schedule again", (schedule, ctx) => { + // Counts from the same instant as the lastRunAt saved below, so it's the post it will make. + const next = nextRun(schedule, zone, now()); + if (!next) { + throw new UserFacingError( + `${label(schedule)} has nothing left to post: its time passed. \`/schedule delete\` removes it.`, + ); + } // From now on: nothing missed while it was paused is posted. - const from = - schedule.lastRunAt && new Date(schedule.lastRunAt) > now() - ? schedule.lastRunAt - : now().toISOString(); - const updated = deps.store.update(schedule.id, { paused: false, lastRunAt: from }); + deps.store.update(schedule.id, { paused: false, lastRunAt: fromNow(schedule, now()) }); ctx.logger.info({ event: "schedule.resumed", id: schedule.id }, "resumed a schedule"); - const next = updated ? nextRun(updated, zone, now()) : undefined; return { - text: next - ? `Resumed ${label(schedule)}. Next post: ${describeWhen(toLocal(next, zone))}.` - : `Resumed ${label(schedule)}, but it has no more posts to make.`, + text: `Resumed ${label(schedule)}. Next post: ${describeWhen(toLocal(next, zone))}.`, private: true, }; }), @@ -712,6 +738,13 @@ function suggestSchedules(schedules: readonly Schedule[], typed: string): Sugges .map((s) => ({ name: `${s.name} (${s.id})${s.paused ? " · paused" : ""}`, value: s.id })); } +/** `lastRunAt` for a schedule that should only post from `now` on (the stored one if later). */ +function fromNow(schedule: Schedule, now: Date): string { + return schedule.lastRunAt && new Date(schedule.lastRunAt) > now + ? schedule.lastRunAt + : now.toISOString(); +} + /** When a schedule posts next from `now` on, if ever. */ export function nextRun(schedule: Schedule, zone: string, now: Date): Date | undefined { const start = parseLocal(schedule.start); diff --git a/src/features/schedules/schedules.test.ts b/src/features/schedules/schedules.test.ts index 34e796e..b359b62 100644 --- a/src/features/schedules/schedules.test.ts +++ b/src/features/schedules/schedules.test.ts @@ -41,6 +41,8 @@ function setup(options: { check?: ChannelCheck; ready?: boolean } = {}) { const store = new ScheduleStore({ logger: silentLogger }); const posts = new ChannelPosts(); const posted: ChannelPost[] = []; + // The feature's clock; tests move it with `clock.now = ...`. + const clock = { now: NOW }; const poster = { check: vi.fn(async (): Promise => options.check ?? { ok: true, name: "general" }), post: vi.fn(async (_id: string, post: ChannelPost) => { @@ -56,10 +58,10 @@ function setup(options: { check?: ChannelCheck; ready?: boolean } = {}) { posts, timezone: AMS, stillAllowed: async () => true, - interpolator: createInterpolator({ timezone: AMS, now: () => NOW }), + interpolator: createInterpolator({ timezone: AMS, now: () => clock.now }), logger, reporter: nullErrorReporter, - now: () => NOW, + now: () => clock.now, }); const registry = new CommandRegistry({ capabilities: new CapabilityRegistry([SCHEDULE_CAPABILITY]), @@ -131,11 +133,19 @@ function setup(options: { check?: ChannelCheck; ready?: boolean } = {}) { subcommand, args, }); + /** One check by the background runner at the clock's time, as `start()` does. */ + const tick = async () => { + const stop = feature.start?.(); + await new Promise((resolve) => setTimeout(resolve, 0)); + stop?.(); + }; return { store, posts, poster, posted, + clock, + tick, feature, run, message, @@ -509,12 +519,29 @@ describe("managing schedules", () => { }); }); - it("says a one-off that's already gone has no more posts when resumed", async () => { + it("won't resume a one-off whose time passed while paused, and says how to delete it", async () => { const ctx = setup(); - await ctx.message({}); + await ctx.message({ name: "Pizza" }); const id = ctx.store.all()[0]?.id as string; - ctx.store.update(id, { lastRunAt: "2026-10-14T17:00:00.000Z", paused: true }); - expect((await ctx.run("resume", { schedule: id })).reply.text).toContain("no more posts"); + await ctx.run("pause", { schedule: id }); + ctx.clock.now = new Date("2026-10-15T08:00:00Z"); + const result = await ctx.run("resume", { schedule: id }); + expect(result.reply.text).toBe( + `**Pizza** (\`${id}\`) has nothing left to post: its time passed. \`/schedule delete\` removes it.`, + ); + expect(ctx.store.get(id)).toMatchObject({ paused: true, lastRunAt: null }); + }); + + it("resumes a repeat from now on, so nothing missed while paused goes out late", async () => { + const ctx = setup(); + await ctx.message({ when: "mon 19:00", repeat: "weekly" }); + const id = ctx.store.all()[0]?.id as string; + await ctx.run("pause", { schedule: id }); + ctx.clock.now = new Date("2026-10-12T17:20:00Z"); // Mon 19:20, 20 minutes after it was due + const result = await ctx.run("resume", { schedule: id }); + expect(result.reply.text).toContain("Next post: Mon 19 Oct 2026, 19:00"); + await ctx.tick(); + expect(ctx.posted).toEqual([]); }); it("shows names people chose as plain text, not formatting", async () => { @@ -540,6 +567,28 @@ describe("when is free-form text", () => { expect(when && "choices" in when ? when.choices : undefined).toBeUndefined(); }); + it("names the configured zone in its description, not a fixed one", () => { + const feature = createSchedulesFeature({ + store: new ScheduleStore({ logger: silentLogger }), + posts: new ChannelPosts(), + timezone: "America/New_York", + stillAllowed: async () => true, + interpolator: createInterpolator({ timezone: "America/New_York", now: () => NOW }), + logger: silentLogger, + reporter: nullErrorReporter, + }); + const cmd = feature.commands?.[0]; + if (!cmd || !isGroup(cmd)) throw new Error("expected a command group"); + const descriptions = cmd.subcommands.flatMap((s) => + isSubgroup(s) ? [] : (s.options ?? []).filter((o) => o.name === "when"), + ); + expect(descriptions.map((o) => o.description)).toEqual([ + "When to post (America/New_York), e.g. wed 1900, 14 oct 19:00, tomorrow 9am", + "When to post (America/New_York), e.g. wed 1900, 14 oct 19:00, tomorrow 9am", + "When to post (America/New_York), e.g. wed 1900, 14 oct 19:00, tomorrow 9am", + ]); + }); + it("confirms the interpreted time in the space's zone", async () => { const shown = text((await setup().message({ when: "tomorrow 9am" })).reply.embeds?.[0]); expect(shown).toContain("Understood as **Tue 13 Oct 2026, 09:00** (Europe/Amsterdam)."); @@ -647,6 +696,74 @@ describe("/schedule edit", () => { ); }); + // A weekly Monday 19:00 post that last went out on Monday 12 October. + async function ranMonday() { + const ctx = setup(); + await ctx.message({ name: "Weekly", when: "mon 19:00", repeat: "weekly" }); + const id = ctx.store.all()[0]?.id as string; + ctx.store.update(id, { lastRunAt: "2026-10-12T17:00:00.000Z" }); + return { ...ctx, id }; + } + + it.each<{ change: string; args: Record; at: string; next: string }>([ + { + change: "days", + args: { days: "thu" }, + at: "2026-10-15T17:20:00Z", // Thu 15 Oct 19:20, 20 minutes after a Thursday post would have been + next: "Thu 22 Oct 2026, 19:00", + }, + { + change: "repeat", + args: { repeat: "monthly" }, + at: "2026-11-12T18:20:00Z", // Thu 12 Nov 19:20, 20 minutes after a monthly post on the 12th + next: "Sat 12 Dec 2026, 19:00", + }, + ])( + "counts new $change from the edit, so it doesn't post straight away", + async ({ args, at, next }) => { + const ctx = await ranMonday(); + ctx.clock.now = new Date(at); + const result = await ctx.run("edit", { schedule: ctx.id, ...args }); + expect(text(result.reply.embeds?.[0])).toContain(`• ${next}`); + expect(ctx.store.get(ctx.id)?.lastRunAt).toBe(new Date(at).toISOString()); + await ctx.tick(); + expect(ctx.posted).toEqual([]); + }, + ); + + it("leaves when it last ran alone when the timing doesn't change", async () => { + const ctx = await ranMonday(); + ctx.clock.now = new Date("2026-10-15T17:20:00Z"); + await ctx.run("edit", { schedule: ctx.id, text: "New text" }); + await ctx.run("edit", { schedule: ctx.id, repeat: "weekly", days: "mon" }); + expect(ctx.store.get(ctx.id)).toMatchObject({ + lastRunAt: "2026-10-12T17:00:00.000Z", + post: { text: "New text" }, + }); + }); + + it("refuses to turn a repeat into a one-off whose time has passed, before the form opens", async () => { + const ctx = await ranMonday(); + ctx.clock.now = new Date("2026-10-15T17:20:00Z"); + const refusal = + "That would leave it with nothing to post: a one-off needs a time that's still to come. Add `when` with the new time."; + expect(await ctx.form("edit", { schedule: ctx.id, repeat: "once" })).toMatchObject({ + ready: false, + refuse: { reply: { text: refusal } }, + }); + const result = await ctx.run("edit", { schedule: ctx.id, repeat: "once", text: "Last one" }); + expect(result.reply.text).toBe(refusal); + expect(ctx.store.get(ctx.id)).toMatchObject({ + recurrence: { kind: "weekly" }, + post: { text: "Pizza night tonight!" }, + }); + await ctx.run("edit", { schedule: ctx.id, repeat: "once", when: "fri 19:00" }); + expect(ctx.store.get(ctx.id)).toMatchObject({ + start: "2026-10-16T19:00", + recurrence: { kind: "once" }, + }); + }); + it("refuses someone else's schedule, an unknown id, and a one-off that already posted, without leaking", async () => { const { run, store, mine, theirs } = await withTwoOwners(); store.update(mine.id, { lastRunAt: "2026-10-14T17:00:00.000Z" });