feat: item reminders — the fire-at-an-instant primitive, and one overdue rule for all four surfaces (IDEA-2641, closes #1010) (#1244)

* feat(store): item reminders — the fire-at-an-instant primitive (IDEA-2641)

Adds the storage, the scheduler tick, and the canonical event for one-shot
item reminders (GitHub #1010). Nothing in Pad acted at a target time before
this: a due_date makes an item show up as overdue once somebody asks the
dashboard, so "revisit TASK-X on the 1st" had to live in an external cron.

A TABLE, NOT A SCHEMA-FIELD ANNOTATION. The design sketch proposed marking
schema date fields with a `reminds: true` key on models.FieldDef; recon
overturned it. Such a key does not survive an ordinary collection edit, two
independent ways: the web editor destructures each field into an EditableField
and rebuilds a fresh definition key-by-key on save, so unknown keys are
dropped (`pattern` and `unique_scope` survive only because two lines were
hand-added for them), and models.CollectionSchema has fixed fields with no
catch-all, so any Go unmarshal+marshal round-trip strips unknown properties —
the hazard retargetRelationFieldsTx mutates raw JSON to avoid. Both failures
are silent and both disarm a whole collection's reminders at once. It is the
same defect class that moved traits out of the schema column in TASK-2657.

The table also gives the lifecycle a home. A reminder is armed, then fired,
then acknowledged, and a re-arm returns it to armed — per-reminder state a
field definition has nowhere to keep.

remind_at is an RFC3339 UTC instant, deliberately not a `date` schema value:
those admit both YYYY-MM-DD and full RFC3339 and are compared against the
SERVER'S LOCAL calendar day. A fire-at time cannot carry that ambiguity. The
remaining timezone question for due_date is filed separately.

Firing is one transaction per reminder carrying BOTH the fired_at write and
the outbox insert. That pairing is the point: a fired_at committed without its
event is a reminder that silently notifies nobody and can never be retried,
because the row has left the armed set; an event without fired_at fires every
tick forever. The UPDATE's own `fired_at IS NULL` predicate is the arbiter, so
two instances ticking at once produce exactly one winner.

item.reminder_due is admitted to the closed events/1 set as v1.2, with a new
PayloadReminder family and no SSE name. The subject is the REMINDER, not the
item: two reminders can be armed on one item, so an item-subject event could
not say which fired, and the reminder id is what an acknowledgement addresses.
A new payload family rather than reusing the item snapshot for the same
reason — a snapshot would validate and still not answer the only question the
event exists to answer. No SSE name in v1 because the poll surface is the
contract; adding one later is additive, removing one is not.

Ack is explicit and nothing else acks. An item reaching a terminal status
deliberately does NOT ack: that would make every status write a reminder
mutation, and it would silently consume a reminder set to fire after the work
was done.

* feat(server): reminder surfaces, and one shared overdue rule for all four

Second half of IDEA-2641: the HTTP surface, the scheduler tick's wiring, and
the fix for the finding that justified the unit — `ready` / `next` did no date
handling at all.

OVERDUE NOW HAS ONE IMPLEMENTATION. It used to live inline in the dashboard's
attention loop, which meant `pad project stale` inherited it (it filters that
very list) and the recommendation surface never saw it. So a deadline reached
the two surfaces that REPORT on work and never the one an agent PULLS from.
overdue.go is now the only place that decides, and all four call it.

Two behaviour changes fall out, both deliberate:

  - An overdue item bypasses the orphan branch's high/critical priority gate.
    That gate was where a deadline quietly stopped: a low-priority item three
    weeks late was reported by `stale` and never suggested by `next`.

  - Overdue sorts above in-progress. The list is capped at three, so a rank
    below in-progress would not merely order the deadline lower — on any
    workspace with three things in flight it would keep an overdue item off
    the surface entirely, which is indistinguishable from not shipping this.

The server-local-today comparison is UNCHANGED and known to be wrong for
multi-timezone deployments; it is filed as its own item with the cloud case
stated. Changing what "overdue" means on every existing instance inside a
change about where the rule LIVES is the kind of behaviour change nobody
reviews.

Fired reminders reach `next` / `ready` two ways, from one filtered list:
PendingReminders is the addressable form (it carries the id an ack needs), and
a prepended suggestion is the rendered form. They are prepended AFTER the cap
rather than entered as ranking candidates — a reminder is not a task competing
on priority, and whether it appeared should not depend on how busy the
workspace is.

Terminal-item reminders are FILTERED from the surface, never acked. Acking on
terminal status would couple every status write to reminder state and would
consume a reminder armed to fire after the work was done. The row stays
exactly as the user left it; the distinction is observable, and asserted.

Three guard tests caught this change and each was answered rather than
silenced:

  - The request-body reader guard was right: the handlers now go through
    decodeJSON, inheriting the NUL refusal and the size cap.

  - The canonical-events guard was right: item.reminder_due is admitted to the
    duplicated contract table as SPEC-3 v1.7, with the reminder subject kind
    and the new payload family. SPEC-3's own text owes the same amendment.

  - The NUL census asked for a decision on eight new columns. None carries
    caller text: ids and FKs are server-generated, four are the server clock,
    and remind_at is now re-parsed and re-formatted in the STORE as well as at
    the edge — so the stored value is always machine-produced from a parsed
    time and no caller bytes reach the column. The doc comment that used to
    say "the caller normalizes" protected nothing.

    Regenerating the baseline also found that GEN_NUL_BASELINE=1, which the
    test's own instructions name, was never implemented — the flag did
    nothing, so the documented path was hand-editing the file. Implemented, so
    the next reader gets the mechanism the instructions promise.

* test(reminders): the lifecycle, the four surfaces, and 22 killed mutants

Every test here was designed against a specific mutation and the mutation was
RUN. A green suite proves nothing about a suite nobody tried to break, and
three of the mutants I first wrote were not experiments at all.

Store (10 mutants, all killed): candidate predicate <= flipped to >=; the
event emission lifted out of the fire transaction; the fire UPDATE's
`fired_at IS NULL` arbiter removed; the RowsAffected check ignored; re-arm
clearing fired_at but not acked_at; ack losing `fired_at IS NOT NULL`; the
poll surface losing `acked_at IS NULL`; normalizeRemindAt no longer refusing;
it dropping .UTC(); GetReminder losing its workspace scope.

Surfaces (12, all killed): the priority gate no longer bypassing on overdue;
the sort no longer ranking overdue first; attention leaving the shared helper;
the reason losing its OVERDUE prefix; the comparison flipped to >; terminal
items no longer skipped; terminal reminders no longer filtered; the filter
ACKING instead of hiding; reminders appended instead of prepended; the tick
running on a far-future clock; ack answering 200 for an unfired reminder;
parseRemindAt accepting a bare date.

THREE MUTANTS DID NOT COUNT ON THE FIRST PASS and were rewritten. Two failed
to compile (`if false` orphaned a variable; deleting a parse orphaned an
import) and one had an anchor matching two call sites. A non-compiling mutant
emits zero FAIL lines and reads exactly like a surviving one — it invents a
hole that is not there — so the harness reports BUILD-FAIL and ANCHOR-BAD as
outcomes distinct from SURVIVED. It also restores files from an in-memory copy
rather than `git checkout`, which would delete uncommitted work in the tree.

ONE MUTANT GENUINELY SURVIVED and the test was at fault, not the mutant:
appending rather than prepending reminder suggestions was undetectable because
the fixture had a single item, so the reminder sat at index 0 either way. The
fixture now fills the three-item cap with in-progress work, where an appended
reminder lands fourth and vanishes. Faithful mutant, weak test — checked in
that order.

The same lesson shapes the four-surface fixture: it is a LOW-priority open
orphan, because that is the case the old code handled worst. A high-priority
task would have made the ready/next leg pass against the unfixed tree, which
is a green that measures nothing.

Negative controls throughout: a future deadline is not overdue and does not
reach the gate bypass; a tick with nothing due fires nothing; a completed item
is neither overdue nor suggested. Without them a helper that reported every
date, or a tick that fired everything, would satisfy every positive leg.

The lead's pin is asserted in both directions: a fired reminder on a done item
is ABSENT from the surface and PRESENT and still unacknowledged in the table.
Asserting only the absence would pass against an implementation that consumed
the row, which is the behaviour the pin exists to forbid.

* feat(mcp): pad_item.remind + ack-reminder, ToolSurfaceVersion 0.28

An agent that can RECEIVE a reminder but not set one has half the primitive.
The poll surface is pad_project.next / ready, both long exposed, so reminders
already reached agents — what was missing is the other half: deferring a piece
of work is exactly the moment an agent knows when it wants to be asked again,
and it had no way to say so.

Two additive actions, two optional params. Nothing existing moved, so a v0.27
consumer enumerating neither is unaffected — the v0.13 / v0.11 / v0.8
disposition, which likewise wired existing CLI verbs onto the catalog.

remind_at REFUSES a bare date rather than reading it as midnight. Worth
stating because the `date` schema type accepts YYYY-MM-DD and a caller will
reasonably try it here: a bare date names a 24-hour span, and choosing an hour
inside it would fire at a time nobody picked.

Re-arm and disarm stay CLI-only. Both address a reminder by an id the agent
would have to list first, and no listing action exists on this surface — a
door with no handle. Adding them later is additive.

Five guards had to be taught, and each was answered on its merits rather than
excluded: the HTTP parity test (route mappers added, so the actions work on
the remote transport rather than being advertised and unrouted), the
read-only catalog's cmdhelp fixture and expected cmdPath map, the field-
conflict classifier (remind_at / reminder_id are NOT field writers — a
reminder is a row in its own table addressed by its own id, so listing them
as classified sources would have pointed detectFieldConflicts at something
that is not a field source), and the instructions.md / README action tables.

That machinery is why the version bump is safe to make now, and it earned its
keep on this change: every one of the five failed on the first build after the
catalog entry landed.

CONVE-23 sweep for prose this falsifies:

  - SPEC-3 (DOC-2653) amended to v1.7 in the room, recording item.reminder_due
    with its new subject kind and payload family — the first canonical event
    with no user mutation behind it, since a scheduler tick produces it.
  - CLAUDE.md gains the reminder routes, the CLI verbs, and the v0.28 entry.
    It was also stale at 0.26 with NO v0.27 entry at all: the 0.27 unit swept
    instructions.md and README.md and missed this file. Both added.
  - skills/pad/SKILL.md gains the verbs and a routing entry, including the two
    things an agent will get wrong — the time is an instant, so ask for a time
    of day rather than picking one, and finishing the item does not
    acknowledge the reminder.

* fix(reminders): codex round 1 — four findings, all real, all with a pin

Round 1 found four defects and refuted none of them. Each fix carries a test
that fails against the code as it was, and each of those was mutation-checked.

**P1 — pending reminders bypassed item-level visibility.** Every other
dashboard section reads `allItems`, which the store already scoped to the
caller's collections AND their granted item ids. The pending-reminder list is
a direct workspace-wide query and inherited none of that, so a guest holding a
grant on ONE item could read the refs and titles of every other item in the
collection through its reminders — an item-level leak wearing a
notification's clothes. Now filtered with the same `isItemVisibleToGuest` call
the sibling sections use. The test's two items share a COLLECTION on purpose:
a collection-level filter was already applied, so separate collections would
have made it pass against the unfixed code.

**P1 — soft-deleted items could starve the queue permanently.** Candidate
selection ignored `deleted_at`, and `fireOneReminder` rolls back when it finds
the item gone — which leaves the reminder ARMED and therefore a candidate
again on the next pass. Candidates are ordered oldest-first and bounded by a
limit, so enough archived reminders fill every batch and no live reminder ever
fires. Silent, too: the tick reports zero fired and looks idle. Excluded in
the candidate query rather than skipped downstream, so those rows never occupy
a slot; the reminders themselves are kept, so restoring an item restores its
reminder with it — asserted, because a fix that reaped them would pass the
starvation test alone.

**P2 — the pass stopped at the first failing reminder.** The per-reminder
transaction exists precisely so one unfireable row cannot hold back the rest,
and `return fired, err` made that comment false — with candidates
oldest-first, one persistently broken old reminder blocks every newer one
forever. Now continues and joins the errors, so a pass that fired seven and
failed three reports both halves rather than reading as clean. The loop is
split behind an injected seam because a real mid-transaction failure is not
reachable from outside: the database refuses the corrupt rows that would cause
one (verified — invalid JSON in items.fields is rejected by the schema).

**P2 — suggestions dropped the reminder id.** The docs tell an agent to
acknowledge what it sees in next/ready, and the payload carried no handle: a
stateless poller could read the reminder and had no way to retire it, so it
would be shown the same item forever. `DashboardSuggestion` now carries
`reminder_id` (omitempty), `pad project next` prints the exact ack command,
and the test acks with the id the surface handed out rather than merely
checking the field is populated — a wrong-but-present id satisfies equality
with itself.

Four mutants, four killed; one was rewritten first because its anchor matched
two call sites and was therefore not an experiment.

* fix(reminders): codex round 2 — four findings, all real

**`--rearm` was unusable.** `ExactArgs(1)` forced an item ref that the rearm
branch then ignored, so the flag could not be reached without supplying a ref
that was silently discarded. Now `MaximumNArgs(1)`, with each mode checked
explicitly: a ref is required to arm, and a ref supplied ALONGSIDE `--rearm`
is refused rather than ignored — it names an item the reminder may not even
belong to, and quietly dropping it is how a user learns nothing about the
reminder they just moved.

**`unremind --format json` emitted plain text**, breaking the parseable-output
contract every sibling command honours.

**The MCP `ref` param did not list `remind`.** Agents read that flat
description to decide what to send, so an action missing from it is an
invalid call waiting to happen. It now also says what `ack-reminder` takes
instead, and why: a reminder is addressed by its own id because an item can
carry several.

**Fractional seconds fired early.** `time.Parse` accepts `09:00:00.900Z` and
`Format(RFC3339)` drops the fraction, so it was stored as `09:00:00Z` and
fired 900ms BEFORE the moment the caller named — silently, having rewritten
their value on the way in. Seconds are genuinely the stored resolution (the
column is compared as a string against a whole-second clock, and the tick runs
every 30s), so the only question was which way to resolve it, and truncation
resolved it the wrong way. `NormalizeInstant` now rounds UP: at most a second
of lateness, in exchange for a guarantee that can be stated — a reminder never
fires before the instant it was set for. Late is a reminder; early is a wrong
answer. Whole seconds round-trip exactly, which is asserted, because an
implementation that added a second unconditionally would otherwise pass.

Three mutants for this round, three killed (round-up→truncate,
round-up→unconditional-add, MaximumNArgs→ExactArgs). Thirty across the unit.

Two fixes carry no dedicated test and it is worth being explicit rather than
implying coverage: the `--format json` branch on `unremind` is a one-line
output change with no server-free way to drive it, and the MCP `ref`
description is prose the drift tests do not read — they assert an action is
DOCUMENTED, not that a param's sentence lists it.

* docs(reminders): the ack id is on the surface an agent polls, not only on the arm response

CONVE-23 follow-through on the round-1 fix. Both agent-facing docs told a
caller to acknowledge a reminder with the id "returned when you armed it" —
true, and useless to the caller that matters: a poller reading next/ready
never armed anything. The suggestion now carries reminder_id and `pad project
next` prints the exact ack command, so the docs say that instead.

The prose was written before the fix existed, which is exactly the case
CONVE-23 is about: a change that makes an instruction stale without touching
the file the instruction lives in.

* test(reminders): bind the tick LOOP to the work, not just the pass (CONVE-19)

Every other test in this file calls runReminderTick directly. That vouches for
the component and says nothing about whether anything ever calls it — a tick
that is never started is indistinguishable, from those tests, from one that
is. It is the convention's exact case, and the failure I recorded on my own
identity doc three times in one unit: I test the component and not the
binding.

Driven through the injectable tick channel so the assertion pins a SPECIFIC
pass instead of racing a 30-second ticker, and polled to a bounded deadline so
a loop that never runs FAILS rather than hanging the suite.

Mutant: drop `s.runReminderTick()` from the select and this goes red while
every direct-call test stays green. Killed.

The idempotence leg exists because a second Start spawning a second loop would
leave one running after Stop, making the BUG-842 drain invariant false for
this sweeper specifically — the one property a copied lifecycle is most likely
to get right by accident and least likely to be checked.

The cmd/pad call site (cmd_server.go, alongside StartTokenReaper) stays
verified by inspection: a source-scanning guard for it would be an instrument
asserting facts about source, which is code with an adversary and not worth it
for one line that sits in the middle of five identical neighbours.

* fix(reminders): codex round 3 — a deferred reminder fired anyway, and the poll surface was unbounded

**A re-arm mid-pass did not stop the fire.** The candidate scan selects an id;
before the UPDATE runs, a `--rearm` can move that reminder into the future.
Re-arm clears `fired_at`, so a predicate checking only `fired_at IS NULL`
still matched — the pass fired a reminder the user had just deferred and
emitted its event. The re-arm cannot undo that: it can clear the mark, but the
event is already on the outbox and at-least-once means a consumer has seen it.

The fire UPDATE now revalidates `remind_at <= nowTS` against the SAME nowTS the
candidate scan used. Same-value deliberately: the arbiter and the scan must
agree about when this pass is, or a reminder could pass one and fail the other
for no reason but clock drift inside a single pass.

**The poll surface was unbounded.** Every fired-and-unacknowledged reminder was
loaded and turned into a suggestion prepended to a list that is otherwise
capped at three, so a workspace with five hundred unacknowledged reminders
returned five hundred suggestions — in the dashboard response, the hottest read
in the product, growing until somebody acknowledged them.

Two bounds, because they are two different guarantees: the query takes a window
(default 50, oldest-fired first, so it holds what has waited longest), and the
prepended suggestions are capped at 5 so `suggested_next` stays a
recommendation rather than a second inbox. The full set stays addressable in
`pending_reminders`.

Truncation is REPORTED as a boolean, not a count. A count would have to be
post-visibility-filter to be true for the caller reading it, and the store
cannot compute that — the filter runs per item, above. "There are more than you
can see here" is the strongest claim the data supports, so it is the one made.

Four mutants; two killed outright, two survived and were run down under
CONVE-28:

- **Uncapped suggestions survived because the fixture had ONE reminder** —
  capped and uncapped are the same list at n=1. That is the SECOND time a
  single-item fixture hid a count-or-order property in this file. Fixture now
  arms eight; it also asserts all eight remain in `pending_reminders`, so the
  cap is pinned to the recommendation and not to the data.

- **Removing the SQL LIMIT survived, correctly, and the test comment now says
  so.** The Go slice cap bounds the PAYLOAD; the SQL LIMIT bounds the
  DATABASE'S work. Only the first is observable at this level — with the LIMIT
  gone the response is still bounded, while the query silently goes back to
  materialising every pending row before discarding most of them. That is a
  memory and I/O property with no assertion available here, so it is stated as
  a coverage boundary rather than papered over with a green that would not have
  measured it.

* docs(reminders): the fire predicate arbitrates against two actors, not one

CONVE-23 inside the file the round-3 fix touched. The comment described the
UPDATE as an arbiter for concurrent TICKS, which is what it was written for
and is why I did not re-read it when asked whether a user edit could race the
pass. It now says what it actually defends against, and names the general
shape: an arbiter is only an arbiter with respect to the writers it can see.

* fix(reminders): codex round 4 — the round-3 bound recreated the round-1 starvation

Round 3 bounded the poll surface. Round 4 caught what that bound did: the
query took the first N rows and the dashboard then discarded the ones it could
not show — hidden items, unauthorised items, completed items — so N such rows
hide a visible reminder behind them indefinitely, with no continuation to
reach it.

That is the SAME defect I had removed from the fire path one round earlier,
reintroduced in the read path within the hour. The general form is worth
stating because I clearly did not hold it: **a bounded window is only safe
when the discarding happens BEFORE the bound.** Filtering above a limit is a
starvation every time, and it does not matter what the filter is for.

Two halves, because the two filters are not the same kind of thing:

**Visibility is now scoped IN SQL**, using the same collection-id / item-id
sets every other dashboard section gets through `allItems` — the same
three-way shape as ItemListParams, where holding both collection grants and
item grants is an OR. Invisible rows no longer occupy the window at all, which
is strictly better than filtering them out afterwards and is what the sibling
sections have always done.

**Terminality is paged**, because SQL cannot evaluate it — a collection's
schema defines which statuses are terminal. The collector refills from the
next page when a page comes back short, bounded by a max scan so a workspace
full of completed items cannot turn a dashboard read into a table scan. The
bound is 10x the window: the common shape fills on the first page, and the
pathological shape terminates in a fixed number of indexed reads. Stopping at
the scan bound reports truncation, which is honest — there may be more, and we
did not look.

The empty-scope case is a THIRD state that reads like the second: nil
CollectionIDs means unrestricted, a non-nil EMPTY slice means this caller sees
no collections. Without an explicit guard they collapse, because the switch
matches none of its cases at length zero and adds no clause at all — so
"nothing visible" would return the whole workspace.

Three mutants, one survived: the empty-scope guard, because no dashboard-level
test produces that state (callers that would are refused earlier by workspace
access). Faithful mutant, missing test — it now has a direct one, with a
sanity leg so a build returning nothing cannot pass it by accident. A guard for
a state nothing exercises is exactly the one that rots.

* fix(reminders): codex round 5 — the MCP action I shipped did not work over stdio

**P1: local stdio MCP `remind` was unusable.** cmdhelp derives positionals by
regex from a command's `Use` string, and `<instant>` inside
`remind <ref> --remind-at <instant>` matched — it became a second REQUIRED
positional, so dispatch failed with `missing required argument "instant"`. The
action was advertised on a transport where it could not run.

**The MCP catalog's own tests did not catch it, and the reason is the finding.**
That suite builds its cmdhelp document BY HAND: I wrote `Args: mkArgs("ref")`
in it, so the fixture agreed with what I meant rather than with what the CLI
says. Five parity and drift tests passed against a document I authored to
match my own intention — the "a test that agrees with whatever the table says
is not a test of the table" shape, which the canonical-events test warns about
in its own comment two packages away. The new test reads the REAL command tree
via cmdhelp.Build, which is the only thing in this repo that can disagree with
me about what the CLI declares.

**P2: `pad project ready` withheld the ack handle** that `next` prints.
Showing a fired reminder on the surface an agent polls while withholding the
id it needs to retire it means the same entry comes back on every poll,
forever.

**P2: suggestions asserted a collection they did not have.** The orphan branch
admits ANY collection — its own comment claimed it gated on tasks "mirroring
the active-plan branch", and that comment was simply false — while the output
hardcoded `Collection: "tasks"` and the reason said "Open task". Pre-existing
for high-priority items since BUG-1082; my overdue bypass widened it to any
overdue item, which is how it surfaced.

Fixed by carrying the item's REAL collection rather than by narrowing the
branch: narrowing would silently drop the non-task items this has surfaced for
a year, and the defect is the mislabelling, not the inclusion. The false
comment is replaced with what the code actually does.

The first version of that test used an overdue IDEA and SKIPPED — ideas use
`new`, and the branch requires `open` or an active status, so it never became
a candidate. A test that cannot fire is a failed reconstruction, not a pass;
the fixture is now a bug-like collection whose vocabulary contains `open`,
which is the population the defect can actually reach.

Three mutants, three killed. Forty-one across the unit.

* fix(reminders): codex round 6 — reminders fired from soft-deleted workspaces

**P1, and the only defect in this unit whose consequence leaves the process.**
Workspace soft-delete deliberately keeps items for the 30-day restore window,
so the candidate query's filter on the ITEM's deleted_at found nothing wrong —
and the tick kept firing, emitting outbound webhook events for a workspace
whose owner had deleted it, possibly while deleting their account.

Both queries now join workspaces and require `w.deleted_at IS NULL`. Nothing
is destroyed: a restored workspace resumes firing, which the test asserts,
because "stops firing" and "is destroyed" are very different answers to
someone who restores a workspace and only one of them is right.

That test first failed for the WRONG REASON and the fixture was at fault: it
counted every outbox row in the workspace, and item creation writes its own,
so the assertion was satisfiable by the fixture itself and discriminated
nothing. Scoped to the reminder event type.

**`Use: "remind <ref>"` declared a requirement the command contradicts.**
cmdhelp derives the machine-readable arg spec from that string, and `--rearm`
takes no ref — so the published contract said "required" for something
optional. The requirement is CONDITIONAL, which cmdhelp cannot express, so the
honest declaration is `[ref]` plus the explicit check that names both call
shapes. The round-5 test grew a `required` column, which is what makes this
observable at all: asserting only the arg NAMES would have passed.

**The pad_item tool description omitted both new actions.** The params were
declared and the actions dispatched, but the prose an agent reads to decide
what a tool can do did not mention them — discoverable only by someone who
already knew to look. It now describes both, including the two things an agent
gets wrong: remind_at is an instant, and nothing but an explicit ack retires a
fired reminder.

Three mutants, three killed. Forty-four across the unit.

* fix(reminders): codex round 7 — one predicate for the scan and the arbiter

Third instance of one class, so this fixes the SHAPE rather than the instance.

The class: the candidate scan filters on something the fire transaction does
not revalidate, so a change committed between them fires a reminder that no
longer qualifies. Round 3 was a re-armed instant. Round 1's soft-deleted item
was the same thing caught from the other side. Round 7 is a workspace deleted
between the scan and the fire — the round-6 fix added the condition to the
SCAN only, and the arbiter went on not knowing about it.

Fixing those one at a time is what let the third happen. `reminderFireable` is
now a single string that both sites reference: the scan asks it and the fire
UPDATE re-asks it, so they cannot disagree, and a fourth condition is one edit
in one place rather than two edits someone has to remember are paired.

Written as a correlated EXISTS on item_reminders.item_id rather than a JOIN
precisely so the identical text is valid in both a SELECT and an UPDATE, and
the scan drops its table alias so the two uses are the same characters.

What deliberately stays outside it: `fired_at IS NULL` and `remind_at <= ?`
live on the reminder row itself, are already spelled identically at both
sites, and folding them in would need a parameter order the shared form cannot
express. Said in the comment so the omission reads as a decision.

Both directions are now tested at the arbiter — a workspace deleted mid-pass
and an item deleted mid-pass — because the item case previously relied on the
item load coming back nil, and someone simplifying the EXISTS down to the
workspace check alone would otherwise still see green.

Three mutants, three killed: the arbiter dropping the shared predicate, and
the predicate dropping each of its two halves. Forty-seven across the unit.

* fix(reminders): codex round 8 — workspace export silently dropped every reminder

WorkspaceExport is a hand-maintained field list, so a new table joins it only
if someone remembers. Reminders did not: a backup/restore, or a
SQLite→Postgres migration via `pad db migrate-to-pg`, dropped every pending
reminder with nothing in the destination to show anything had gone.

The line that list has always drawn is item-scoped workspace CONTENT
(comments, links, versions — exported) versus per-user state (stars, watches —
not). A reminder has no user column and hangs off an item, which puts it on
the exported side. Stating the rule rather than just adding the field, because
the next person adding a table needs to know which side they are on.

LIFECYCLE MARKS ARE CARRIED, not reset. A fired-and-unacknowledged reminder is
still owed to whoever armed it, so it arrives pending; an armed one whose
instant has passed fires once on the destination's first tick, which is what
would have happened had the workspace never moved. Re-arming everything on
import would invent a schedule the user did not set. NULL rather than empty
string for the unset marks — the lifecycle is defined by NULL-ness, and ""
would make a never-fired reminder read as fired at "".

TestMigratedTablesCoversTheExport caught the second half, which I would have
missed: `pad db migrate-to-pg`'s NUL preflight decides what to REFUSE on from
MigratedTables, so a table the migration copies and the preflight does not
know about is a gap in exactly the guard that exists to prevent one. Added
there too, with the reason it can never actually fire — every column is
machine-produced, so it is listed for coverage rather than expectation — and
the "six tables" prose it falsified is now seven.

Two mutants, two killed: export dropping the block, and import discarding the
marks. Forty-nine across the unit.

* test(reminders): state the fire-path invariant and pin it from the invariant

The lead's read on why rounds 4 and 7 were the same class: the fire path had
no stated invariant, so each fix defended an instance. This states it, and
derives the pin from the paragraph rather than from the bug history.

THE INVARIANT: the candidate scan is a hint and may be assumed to prove
nothing. Every condition that made a row a candidate is re-asserted inside the
transaction that marks it fired, in the same statement that does the marking,
so checking and writing are one atomic act.

Worded as "the scan proves nothing" rather than as a list on purpose — a list
invites the next person to add a condition to the scan and stop, which is
exactly what happened four times here.

TestFirePathInvariant is the pin: one table, one row per scan-side condition,
each invalidating that condition in the window between the scan and the fire
and asserting the same three things — nothing fires, no event leaves, the
reminder is not consumed. The earlier per-defect tests are folded in as rows;
they said the same thing one instance at a time, which is how four of these
shipped. Adding a fifth condition to the scan without a row here should feel
like an omission. It carries a positive control, because four cases that all
assert nothing happens would pass against a build that never fires at all.

The matrix immediately falsified a claim in the paragraph I had just written.
I wrote that the item load inside the transaction is "for the payload, not for
the check"; removing the item half of reminderFireable alone changes no
observable behaviour, because the load then returns nil and the deferred
rollback undoes the write. Item liveness is defended TWICE and a single-mutant
experiment cannot say which guard is carrying it — removing both is what kills
the test. Both are kept, the predicate is named as primary (the row never
matches, so no write happens at all), and the asymmetry is stated: workspace
liveness has no second line, which is why dropping ITS half does fail the pin.

Six mutants: five singles plus the pair. Five killed alone; the item single
survives by design and is documented as such rather than left as an unexplained
green. Fifty-five across the unit.

* fix(reminders): codex round 9 — one legacy row could hide every reminder

**P1: items.item_number is NULLABLE and I scanned it into an int.** Migration
006 added the column to existing rows, so a pre-numbering item still carries
NULL — and scanning NULL into an int fails the Scan, which fails the QUERY,
which degrades the whole pending-reminder section. One old row, and the
feature is dark for everyone in that workspace.

ListWatchesForUser, which this query was modelled on, uses sql.NullInt64 for
exactly this column. I copied its shape and dropped the part that handles the
column's actual nullability — the same way of being wrong as the round-5
cmdhelp fixture: borrowing a form without borrowing what it knows. The legacy
row now carries no ref rather than a fabricated "PREFIX-0", which would name a
different item.

**P1: export shipped reminders that import could only discard.** The items
section filters on deleted_at IS NULL, so a soft-deleted item is not in the
bundle and its reminder can never be reunited with it. My comment claimed the
item_links rationale — round-trip the raw graph so a restore reunites them —
which is true for links and false here, because links keep soft-deleted
endpoints in the bundle and items do not. A link is a row ABOUT two items; a
reminder whose item is absent is a dangling schedule.

**P2: import wrote remind_at raw.** Import is a writer, and a bundle is not
necessarily one this server produced — hand-edited, or from another instance.
A local offset or a bare date would land in the one column every comparison
downstream treats as a UTC instant, firing early, late, or never. It now
normalizes like every other door. An unparseable value is SKIPPED with a
warning rather than failing the restore, matching the lenient import-side
precedent already in this file, and the raw value's LENGTH is logged rather
than its content.

Three mutants, three killed; two needed rewriting because the single-line form
did not compile — reverting the nullable scan also requires reverting the
render, and dropping the normalization orphans a variable.

PROCESS FAULT, recorded because it makes this round's findings weaker than
they look: I edited the tree while this review was reading it — committed the
invariant work and ran five mutation experiments, which write and restore
source, over the same files. A review binds to the tree it read and I moved it
underneath. Every finding above was re-verified against the current tree
before being acted on, and the next round runs with no concurrent edits.

* fix(reminders): codex round 10 — one orphaned item aborted a whole restore

An ORPHANED item — one whose collection is missing from the bundle — still
gets an itemMap entry. It has to: the entry is written before the skip because
parent resolution inside the same loop reads the map for items it has not
reached yet. So `itemMap[x] != ""` is satisfied by an id that names no row,
and inserting a foreign key to it fails (SQLite enforces FKs here via the
DSN's `_pragma=foreign_keys(on)`; Postgres always does).

The pre-existing mapping is the sharp edge. The aggravating half was mine:
this loop treated a failed reminder insert as FATAL, where item_links and
item_versions both skip, so one orphaned item carrying a reminder rolled back
an entire 900-item workspace restore. A reminder is the least critical thing
in a bundle and it had the strictest failure handling in the file.

Both halves fixed: the loop gates on items that actually landed, and a failed
insert warns and skips like its siblings.

TWO GUARDS THAT ONLY DIE TOGETHER, and this is measured rather than assumed.
Reverting either alone leaves the test green — with the map gate restored the
skip survives the FK failure, and with the fatal return restored the gate
means the insert never fails. Removing both is what fails it. They are kept as
a pair because they defend the same failure at different depths (prevent the
bad write / survive a bad write arriving some other way), and the pair is
recorded in the code so a future reader does not delete one as dead after
watching its mutant survive. Second time this shape appeared today; the first
was item liveness on the fire path.

The bundle in the test is hand-built, because ExportWorkspace cannot produce
an orphan — which is the reason it needed a test. That shape only arrives from
a hand-edited or foreign bundle, and surviving those is what import is for.

Three mutants: two singles that survive by design, plus the pair that kills.
Sixty-one across the unit.

* fix(reminders): codex round 11 — four contract slips, one of them another unit's

**suggested_next returned up to eight entries against a cap of three.** Round 3
prepended reminders PAST the list's own cap, reasoning they should not compete
for slots. Every consumer — the web dashboard, `pad project next`, `pad project
ready` — is written for three.

Worse, it silently falsified a decision recorded elsewhere: BootstrapDashboard
deliberately has no suggested_next_overflow_count BECAUSE this list is capped
at three upstream, and its comment names raising that cap as the moment to add
one. My change made another unit's reasoning wrong in a file I never opened.
The combined list is now trimmed back to three, reminders still leading — a
reminder can push a task suggestion out, which is the right way round, and the
full set stays addressable in pending_reminders.

My first version of that trim used `limit`, which is REASSIGNED above to
len(candidates) — so on a workspace whose only entries are reminders it would
have truncated to zero, killing precisely the case the surface exists for.
Caught by reading the surrounding lines before running anything; it has its own
test now.

**pending_reminders was uncapped in the bootstrap projection.**
BootstrapDashboard embeds *DashboardResponse, so every new field joins the boot
payload automatically — here, a window of up to 50, which is the budget
PLAN-1410 spent a unit trimming. Capped at 5 with an overflow count, under its
own constant rather than borrowing bootstrapAttentionCap: they answer different
questions and a future change to one must not silently move the other.

**Truncation was reported from the wrong question.** The collector used the
store's `more` flag, which answers "is there another PAGE", not "did I read all
of THIS one" — so a window filling part way through the final page reported
that the caller had seen everything while unread rows sat behind the fill
point. The paging bounds are now injectable so the case is testable at all:
building it with a window of 50 needs ~75 rows in a specific pattern, with a
window of 3 it is four.

**Import accepted acked-without-fired**, which is not one of the lifecycle's
three states. Such a row fires, is excluded from the pending surface because it
is already acked, and can never be acknowledged because AckReminder requires
acked_at IS NULL — an event emitted into permanent invisibility. The
acknowledgement is dropped and the schedule kept, since an ack of something
that never fired means nothing.

Five mutants, five killed (one rewritten — removing the flag orphans a
variable). Sixty-six across the unit.

* fix(reminders): codex round 12 — a read is not a hold; scope the arm; ack from the ack

Four P2s from round 12 (two independent runs, both landing on the same
line of the fire path), each closed at the layer where it lives:

- fireOneReminder pins the item and workspace rows FOR NO KEY UPDATE on
  Postgres before the arbiter UPDATE. reminderFireable re-asserted
  liveness at the predicate's instant and nothing held it to the commit
  instant; under READ COMMITTED an archival could commit in between and
  the event left the process about a deleted resource. Same idiom and
  same lock strength as CreateAttachmentForLiveItem; SQLite is excluded
  by its BEGIN IMMEDIATE, not skipped for convenience. Two PG-only pins
  verify "blocked" in pg_stat_activity, not by elapsed time; the
  pin-removed mutant fails both.
- CreateReminder asserts "live item of THIS workspace" in the INSERT's
  own SELECT and returns ErrReminderItemGone otherwise. The table had an
  FK and no same-workspace constraint; a mismatched pair fed another
  workspace's title to this one's dashboard and webhooks. Handler maps
  it to 404.
- AckReminder matches every fired row (COALESCE keeps the first ack,
  updated_at moves only when acked_at does), so a no-match means exactly
  "not fired at the instant of the ack". The handler no longer decides
  409-vs-200 from the row it read before the UPDATE.
- The invariant paragraph gains its missing sentence: "at that instant"
  means the commit instant, and the pin is what makes the predicate's
  instant and the commit instant the same one.

Round-12 caveat carried: both runs were static reads (sandbox blocked
Go's build cache), so "four" is a floor, not a measurement.

Refs IDEA-2641

* fix(reminders): codex round 13 — a reminder's workspace must agree with its item's, at every read

Every reader scoped by r.workspace_id and then joined the item without
asserting the two agree. No door writes a disagreeing row today
(CreateReminder derives the pair from the item; import maps within the
workspace), and the table has nothing that forbids one — so a hand-edited
bundle, a future move door, or a direct write would carry one
workspace's item into another's dashboard, export, and webhooks.

The identity goes into reminderFireable (scan + arbiter), the Postgres
row pin, ListPendingReminders and the export query. One test writes the
row raw — the only way one can exist — and asserts it is inert at each
site; the predicate-removed mutant scans and fires it.

Refs IDEA-2641

* fix(reminders): codex round 14 — the by-id and by-item reads assert the same identity as every other read

GetReminder scoped by the row's own workspace_id and ListRemindersForItem
by item_id alone, so a row whose two columns disagree — the class rounds
12 and 13 closed at the scan, the arbiter, the pin, the pending surface
and the export — was still readable through the two reads that reach a
single row. reminderOwned is that identity on its own, without the
liveness half those two reads must not have (a fired reminder on an
archived item is history worth showing). The write paths reach a row
only through GetReminder, so scoping it scopes them; a row no door can
write needs no door to delete it. ListRemindersForItem now takes the
workspace its caller already resolved the item in.

The raw-row test asserts both reads refuse the row from both sides; the
reminderOwned-removed mutant surfaces it through GetReminder.

Refs IDEA-2641

* fix(reminders): codex round 16 — an archived item's reminders are readable, and its verbs say "archived"

The doors resolved the item live. Listing an archived item's reminders
answered 409 from a GET, and ack/re-arm/delete answered a bare 404 for a
reminder that exists on an item that exists — while the store, since
round 14, deliberately keeps that history readable. The API already has a
posture for archived items: GET reads them, mutations answer 409
"archived … restore it before editing" (writeItemResolveError). The list
now follows handleGetItem; the lifecycle verbs load the item
include-deleted, run the visibility check first, and then answer the same
409 every other item mutation does. One test walks archive → list 200 /
ack 409 / arm 409 → restore → ack 200 on the same rows.

Refs IDEA-2641

* fix(reminders): codex round 17 — one suggestion per item, the archived 409 by slug, and the door courtesy named

Three findings on the server pass. (1) An item that was both a fired
reminder and an ordinary candidate appeared in suggested_next twice; the
ordinary entry is dropped, the reminder entry (which carries the ack id)
stays, and two reminders on one item remain two entries. (2) Round 16's
409 for an archived item's reminder was written by re-resolving item.Ref,
which is derived and empty for a legacy item with no item_number — so the
class most likely to be legacy fell through to a bare 404. The slug is
handed over instead. (3) The archived check in resolveReminderForWrite is
check-then-write, and an archive landing in between lets the verb through:
accepted and documented — it is the posture of every item mutation here
(UpdateItem's UPDATE has no liveness clause), the outcome is benign, and
putting liveness in AckReminder's WHERE would re-create the no-match
ambiguity round 12 removed.

Refs IDEA-2641
This commit is contained in:
xarmian
2026-09-04 11:36:11 -04:00
committed by GitHub
parent a1716d8170
commit b437cc582d
40 changed files with 5593 additions and 65 deletions
+14 -3
View File
File diff suppressed because one or more lines are too long
+3 -3
View File
File diff suppressed because one or more lines are too long
+8
View File
@@ -228,6 +228,11 @@ func nextCmd() *cobra.Command {
// branch's framing.
var dash struct {
SuggestedNext []struct {
// ReminderID is present only on a fired-reminder
// suggestion, and it is the handle an ack needs — a
// surface that shows a reminder without it can be read
// and not acted on (IDEA-2641, codex round 1).
ReminderID string `json:"reminder_id,omitempty"`
ItemSlug string `json:"item_slug"`
ItemRef string `json:"item_ref,omitempty"`
ItemTitle string `json:"item_title"`
@@ -259,6 +264,9 @@ func nextCmd() *cobra.Command {
bold.Sprint(s.ItemTitle),
dim.Sprint(s.Reason),
)
if s.ReminderID != "" {
fmt.Printf(" %s\n", dim.Sprintf("acknowledge with: pad item ack %s", s.ReminderID))
}
}
return nil
},
+198
View File
@@ -0,0 +1,198 @@
package main
import (
"fmt"
"text/tabwriter"
"github.com/fatih/color"
"github.com/spf13/cobra"
"github.com/PerpetualSoftware/pad/internal/cli"
)
// `pad item remind` and friends — the CLI half of IDEA-2641 / GitHub #1010.
//
// The verbs mirror the lifecycle rather than inventing a vocabulary: arm
// (`remind`), see (`reminders`), move (`remind --rearm`), acknowledge (`ack`),
// disarm (`unremind`).
var (
remindAtFlag string
remindRearmID string
)
func remindCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "remind [ref]",
Short: "Arm a reminder on an item",
Long: `Arm a one-shot reminder that fires at a specific instant.
The instant is RFC3339 and must carry a time of day 2026-08-01T09:00:00Z, or
2026-08-01T09:00:00-04:00, which is stored as the same moment in UTC. A bare
date is refused rather than assumed to mean midnight: "2026-08-01" names a
24-hour span, and picking an hour inside it would be Pad choosing a time you
did not and then firing at it.
When the reminder fires it appears in 'pad project next' and 'pad project
ready' until you acknowledge it with 'pad item ack', and it emits an
item.reminder_due webhook event. The poll surface is not optional: an instance
with no webhook configured delivers reminders that way and only that way.`,
// `[ref]` rather than `<ref>` in Use, because cmdhelp derives the
// machine-readable arg spec from this string and `<ref>` would declare
// a REQUIRED positional that --rearm does not take (codex round 6).
// The requirement is conditional, which cmdhelp has no way to express,
// so the honest declaration is "optional" plus the explicit check
// below that names the two ways to call it.
//
// MaximumNArgs, not ExactArgs: --rearm addresses a REMINDER by id and
// needs no item ref, so requiring one made the flag unusable (codex
// round 2). The two modes are checked below rather than merged,
// because a ref supplied alongside --rearm is ambiguous — it names an
// item the reminder may not even belong to — and silently ignoring it
// is how a user learns nothing about the reminder they just moved.
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
client, _ := getClient()
ws := getWorkspace()
if remindAtFlag == "" {
return fmt.Errorf("--remind-at is required (an RFC3339 instant, e.g. 2026-08-01T09:00:00Z)")
}
if remindRearmID != "" {
if len(args) > 0 {
return fmt.Errorf("--rearm addresses a reminder by id, so it takes no item ref (got %q)", args[0])
}
r, err := client.RearmReminder(ws, remindRearmID, remindAtFlag)
if err != nil {
return err
}
if formatFlag == "json" {
return cli.PrintJSON(r)
}
fmt.Printf("Re-armed reminder %s for %s\n", r.ID, r.RemindAt)
return nil
}
if len(args) == 0 {
return fmt.Errorf("an item ref is required (e.g. pad item remind TASK-5 --remind-at 2026-08-01T09:00:00Z), or use --rearm <id> to move an existing reminder")
}
item, err := client.GetItem(ws, args[0])
if err != nil {
return err
}
r, err := client.CreateItemReminder(ws, item.Slug, remindAtFlag)
if err != nil {
return err
}
if formatFlag == "json" {
return cli.PrintJSON(r)
}
fmt.Printf("Reminder armed on %s for %s (id %s)\n", item.Ref, r.RemindAt, r.ID)
return nil
},
}
cmd.Flags().StringVar(&remindAtFlag, "remind-at", "", "when to fire (RFC3339 instant, e.g. 2026-08-01T09:00:00Z)")
cmd.Flags().StringVar(&remindRearmID, "rearm", "", "move an existing reminder by id instead of arming a new one")
return cmd
}
func remindersCmd() *cobra.Command {
return &cobra.Command{
Use: "reminders <ref>",
Short: "Show an item's reminders",
Long: `List every reminder on an item armed, fired, and acknowledged.
Fired reminders are kept rather than deleted: the row is the record that a
reminder existed and went out.`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
client, _ := getClient()
ws := getWorkspace()
item, err := client.GetItem(ws, args[0])
if err != nil {
return err
}
reminders, err := client.ListItemReminders(ws, item.Slug)
if err != nil {
return err
}
if formatFlag == "json" {
return cli.PrintJSON(reminders)
}
if len(reminders) == 0 {
fmt.Printf("No reminders on %s.\n", item.Ref)
return nil
}
w := tabwriter.NewWriter(cmd.OutOrStdout(), 0, 4, 2, ' ', 0)
fmt.Fprintf(w, "ID\tWHEN\tSTATE\n")
for _, r := range reminders {
state := "armed"
switch {
case r.FiredAt != nil && r.AckedAt != nil:
state = "acknowledged"
case r.FiredAt != nil:
state = "FIRED — needs ack"
}
fmt.Fprintf(w, "%s\t%s\t%s\n", r.ID, r.RemindAt, state)
}
return w.Flush()
},
}
}
func ackCmd() *cobra.Command {
return &cobra.Command{
Use: "ack <reminder-id>",
Short: "Acknowledge a fired reminder",
Long: `Acknowledge a reminder that has fired, removing it from 'pad project next'.
Nothing else acknowledges a reminder. In particular, completing the item does
NOT: a reminder may have been armed precisely to fire after the work was done,
and consuming it on a status change would throw that away. A reminder on a
completed item is hidden from the recommendation surface but stays in the
table, still unacknowledged, exactly as you left it.`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
client, _ := getClient()
ws := getWorkspace()
r, err := client.AckReminder(ws, args[0])
if err != nil {
return err
}
if formatFlag == "json" {
return cli.PrintJSON(r)
}
color.New(color.Faint).Printf("Acknowledged reminder %s\n", r.ID)
return nil
},
}
}
func unremindCmd() *cobra.Command {
return &cobra.Command{
Use: "unremind <reminder-id>",
Short: "Disarm a reminder",
Long: `Remove a reminder.
Deletion is the only disarm there is no cancelled state, because a cancelled
reminder and an absent one are indistinguishable to everything that reads them.`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
client, _ := getClient()
ws := getWorkspace()
if err := client.DeleteReminder(ws, args[0]); err != nil {
return err
}
if formatFlag == "json" {
return cli.PrintJSON(map[string]any{"id": args[0], "deleted": true})
}
fmt.Printf("Removed reminder %s\n", args[0])
return nil
},
}
}
+9
View File
@@ -868,6 +868,15 @@ func serveCmd() *cobra.Command {
}
srv.StartTokenReaper()
// Item reminder scheduler (IDEA-2641 / GitHub #1010). The only
// thing in Pad that ACTS at a target time rather than reporting
// on one when asked. Default: 30s, override-able via env
// (PAD_REMINDER_TICK_INTERVAL) the same way the reaper is.
if reminderInterval := parseDurationEnv("PAD_REMINDER_TICK_INTERVAL", 0); reminderInterval != 0 {
srv.SetReminderTickConfig(reminderInterval, 0)
}
srv.StartReminderTick()
// Workspace hard-purge sweeper (TASK-1966). Periodic sweep
// that hard-deletes workspaces soft-deleted more than 30 days
// ago — cascading every child row and reclaiming attachment
+4
View File
@@ -124,6 +124,10 @@ func itemCmd() *cobra.Command {
bulkUpdateCmd(),
commentCmd(),
commentsCmd(),
remindCmd(),
remindersCmd(),
ackCmd(),
unremindCmd(),
noteCmd(),
decideCmd(),
blocksCmd(),
+7
View File
@@ -66,6 +66,13 @@ for active plans.`,
label := strings.TrimSpace(strings.Join([]string{s.ItemRef, s.ItemTitle}, " "))
fmt.Printf(" %s %s\n", dim.Sprintf("%d.", i+1), bold.Sprint(label))
fmt.Printf(" %s\n", dim.Sprint(s.Reason))
// The ack handle, same as `next` (codex round 5). Showing a
// fired reminder on the surface an agent polls and withholding
// the id it needs to retire it means the same entry comes back
// on every poll forever.
if s.ReminderID != "" {
fmt.Printf(" %s\n", dim.Sprintf("acknowledge with: pad item ack %s", s.ReminderID))
}
}
return nil
},
+127
View File
@@ -0,0 +1,127 @@
package main
import (
"testing"
"github.com/PerpetualSoftware/pad/internal/cmdhelp"
"github.com/PerpetualSoftware/pad/internal/server"
)
// The CLI half of IDEA-2641's stale leg.
//
// `pad project stale` does no date work of its own — it filters the
// dashboard's attention list and keeps four types. The server-side leg pins
// that an overdue item lands in that list carrying type "overdue"; this pins
// the other half, that stale still keeps it. Split across the two packages
// because that is where the two halves actually live: a single test could not
// fail for the CLI's reason.
//
// MUTANT: removing "overdue" from filterAgentAttention's interesting map makes
// deadlines vanish from `pad project stale` while every server-side assertion
// stays green.
func TestStaleKeepsOverdueAttention(t *testing.T) {
attention := []server.DashboardAttention{
{Type: "overdue", ItemRef: "TASK-1", ItemTitle: "Late", Reason: "due date was 2020-01-01"},
{Type: "plan_completion", ItemRef: "PLAN-1", ItemTitle: "Done plan"},
}
got := filterAgentAttention(attention)
var sawOverdue bool
for _, a := range got {
if a.Type == "overdue" && a.ItemRef == "TASK-1" {
sawOverdue = true
}
if a.Type == "plan_completion" {
t.Error("plan_completion is not an agent-actionable attention type and must be filtered out")
}
}
if !sawOverdue {
t.Error("`pad project stale` dropped the overdue entry; deadlines never reach the CLI surface")
}
}
// TestRemindArgsAcceptRearmWithoutARef — codex round 2.
//
// `--rearm` addresses a reminder by id and needs no item ref, but ExactArgs(1)
// forced one and the rearm branch then ignored it — so the flag could not be
// used at all, and the ref a user supplied to satisfy cobra was silently
// discarded.
//
// MUTANT: restore ExactArgs(1) and the zero-arg case fails; drop the
// ref-with-rearm refusal and the ambiguous case stops failing.
func TestRemindArgsAcceptRearmWithoutARef(t *testing.T) {
cmd := remindCmd()
if err := cmd.Args(cmd, []string{}); err != nil {
t.Errorf("remind must accept zero args so --rearm is usable: %v", err)
}
if err := cmd.Args(cmd, []string{"TASK-1"}); err != nil {
t.Errorf("remind must still accept an item ref: %v", err)
}
if err := cmd.Args(cmd, []string{"TASK-1", "TASK-2"}); err == nil {
t.Error("remind accepted two positional args")
}
}
// TestReminderCommandsExposeTheArgsMCPExpects — codex round 5, P1.
//
// cmdhelp derives positionals by regex from a command's `Use` string, and
// `<instant>` inside `remind <ref> --remind-at <instant>` matched: it became a
// second REQUIRED positional, so local stdio MCP dispatch failed with
// `missing required argument "instant"` — the action was advertised and
// unusable on that transport.
//
// The MCP catalog's own test did not catch it because its cmdhelp document is
// HAND-BUILT: I wrote `Args: mkArgs("ref")` there, so the fixture agreed with
// what I meant rather than with what the CLI says. This test reads the REAL
// tree, which is the only thing that can disagree with me.
//
// MUTANT: put a `<...>` placeholder back in any of these Use strings and the
// matching case fails.
func TestReminderCommandsExposeTheArgsMCPExpects(t *testing.T) {
doc := cmdhelp.Build(newRootCmd(), newRootCmd(), cmdhelp.Options{MaxDepth: -1})
for _, tc := range []struct {
path string
want []string
required []bool
}{
// `remind`'s ref is OPTIONAL: --rearm addresses a reminder by id and
// takes none. cmdhelp cannot express a conditional requirement, so
// declaring it required would be a machine-readable claim the command
// contradicts (codex round 6).
{"item remind", []string{"ref"}, []bool{false}},
{"item ack", []string{"reminder-id"}, []bool{true}},
{"item reminders", []string{"ref"}, []bool{true}},
{"item unremind", []string{"reminder-id"}, []bool{true}},
} {
cmd, ok := doc.Commands[tc.path]
if !ok {
t.Errorf("%q is missing from cmdhelp entirely", tc.path)
continue
}
var got []string
for _, a := range cmd.Args {
got = append(got, a.Name)
}
if len(got) != len(tc.want) {
t.Errorf("%q positionals = %v, want %v", tc.path, got, tc.want)
continue
}
for i := range got {
if got[i] != tc.want[i] {
t.Errorf("%q positionals = %v, want %v", tc.path, got, tc.want)
break
}
if cmd.Args[i].Required != tc.required[i] {
t.Errorf("%q arg %q required = %v, want %v", tc.path, got[i], cmd.Args[i].Required, tc.required[i])
}
}
}
// The flag MCP actually sends must exist under the name it sends.
if _, ok := doc.Commands["item remind"].Flags["remind-at"]; !ok {
t.Error("`item remind` has no --remind-at flag; the MCP remind_at param maps to nothing")
}
}
+41
View File
@@ -2033,3 +2033,44 @@ func parseErrorBody(status int, body []byte) error {
}
return fmt.Errorf("API error: %d %s", status, string(body))
}
// --- Item reminders (IDEA-2641) ---
// ListItemReminders returns every reminder on an item, armed or fired.
func (c *Client) ListItemReminders(wsSlug, itemSlug string) ([]models.Reminder, error) {
var result struct {
Reminders []models.Reminder `json:"reminders"`
}
if err := c.get("/workspaces/"+wsSlug+"/items/"+itemSlug+"/reminders", &result); err != nil {
return nil, err
}
return result.Reminders, nil
}
// CreateItemReminder arms a reminder. remindAt must be an RFC3339 instant —
// the server refuses a bare date rather than assuming a time of day, and the
// CLI passes the user's string through so that refusal reaches them with the
// server's wording rather than a second, differently-worded local one.
func (c *Client) CreateItemReminder(wsSlug, itemSlug, remindAt string) (*models.Reminder, error) {
var result models.Reminder
return &result, c.post("/workspaces/"+wsSlug+"/items/"+itemSlug+"/reminders",
map[string]string{"remind_at": remindAt}, &result)
}
// RearmReminder moves a reminder's instant, clearing its fire marks.
func (c *Client) RearmReminder(wsSlug, reminderID, remindAt string) (*models.Reminder, error) {
var result models.Reminder
return &result, c.patch("/workspaces/"+wsSlug+"/reminders/"+reminderID,
map[string]string{"remind_at": remindAt}, &result)
}
// AckReminder acknowledges a fired reminder.
func (c *Client) AckReminder(wsSlug, reminderID string) (*models.Reminder, error) {
var result models.Reminder
return &result, c.post("/workspaces/"+wsSlug+"/reminders/"+reminderID+"/ack", nil, &result)
}
// DeleteReminder disarms a reminder by removing it.
func (c *Client) DeleteReminder(wsSlug, reminderID string) error {
return c.delete("/workspaces/" + wsSlug + "/reminders/" + reminderID)
}
+38 -1
View File
@@ -32,12 +32,17 @@
// contract's shape, not running code.
package kernelevents
// Canonical event names — the events/1 set (SPEC-3 §Taxonomy, v1.1).
// Canonical event names — the events/1 set (SPEC-3 §Taxonomy, v1.7).
//
// item.restored and item.bulk_updated were admitted in v1.1 during TASK-2658
// recon: restore is a live first-class mutation whose silence would let an
// item reappear unobserved, and the batch event preserves TASK-1668's
// anti-flood decision for lane-wide mutations.
//
// item.reminder_due was admitted in v1.7 with the reminder primitive
// (IDEA-2641): it is the first canonical event with no user mutation behind
// it — a scheduler tick produces it — which is why it needed a version to be
// admitted in rather than arriving as a side effect of the feature.
const (
// ItemCreated fires on item creation. Payload carries the post-create
// snapshot.
@@ -132,6 +137,22 @@ const (
PackInstalled = "pack.installed"
PackUpgraded = "pack.upgraded"
PackDisabled = "pack.disabled"
// ItemReminderDue fires when a reminder's instant arrives and the
// scheduler tick claims it (IDEA-2641, GitHub #1010). Admitted in v1.7.
//
// The SUBJECT IS THE REMINDER, not the item it is about, and that is the
// one surprising thing here given the name. Two reminders can be armed on
// one item, so an item-subject event could not tell a consumer WHICH one
// fired, and the reminder id is what an acknowledgement addresses — a
// subject a consumer cannot act on is a subject in name only. The item is
// carried in the payload, where it is what the reminder is ABOUT rather
// than what the event is OF. Same reasoning that makes comment.created a
// comment-subject event rather than an item-subject one.
//
// The name keeps the `item.` prefix because the reminder has no meaning
// apart from its item and consumers filter this family by prefix.
ItemReminderDue = "item.reminder_due"
)
// Subject kinds — what an event is about. Stored alongside the event so a
@@ -143,6 +164,10 @@ const (
SubjectAttachment = "attachment"
SubjectMember = "member"
SubjectPack = "pack"
// SubjectReminder: the reminder row itself. See ItemReminderDue for why a
// reminder-due event is not item-subject.
SubjectReminder = "reminder"
)
// eventSpec is everything the kernel knows about one canonical event.
@@ -240,6 +265,17 @@ const (
// PayloadPack: reserved with the pack events; no producer yet.
PayloadPack = "pack"
// PayloadReminder: the reminder row plus the item it is about.
//
// Deliberately NOT a reuse of PayloadItemSnapshot, which would have
// validated and still failed the consumer: a snapshot cannot say which
// reminder fired or what instant it was armed for, and once an item
// carries two reminders that is the only question the event exists to
// answer. The family check is what stops an event and a payload that were
// not meant for each other from being written together, so a family whose
// shape omits the event's own subject would defeat it from the inside.
PayloadReminder = "reminder"
)
var canonical = map[string]eventSpec{
@@ -259,6 +295,7 @@ var canonical = map[string]eventSpec{
PackInstalled: {SubjectPack, []string{PayloadPack}, ""},
PackUpgraded: {SubjectPack, []string{PayloadPack}, ""},
PackDisabled: {SubjectPack, []string{PayloadPack}, ""},
ItemReminderDue: {SubjectReminder, []string{PayloadReminder}, ""},
}
// PayloadFamilies returns every payload shape a canonical event may carry, and
+32 -1
View File
@@ -71,6 +71,20 @@ var padItemTool = ToolDef{
"unlink": actionItemUnlink,
"deps": passThrough([]string{"item", "deps"}),
// Reminders (IDEA-2641 / GitHub #1010). An agent that can RECEIVE a
// reminder but not set one has half the primitive: deferring a piece
// of work is exactly the moment an agent knows when it wants to be
// asked again. `remind` arms; `ack-reminder` acknowledges a fired one
// so it leaves pad_project's next/ready surface.
//
// Re-arm and disarm are deliberately CLI-only for now: both address a
// reminder the agent would have had to list first, and the listing
// action does not exist on this surface yet. Adding them later is
// additive; shipping them without a way to discover an id would be
// advertising a door with no handle.
"remind": passThrough([]string{"item", "remind"}),
"ack-reminder": passThrough([]string{"item", "ack"}),
// Stars
"star": passThrough([]string{"item", "star"}),
"unstar": passThrough([]string{"item", "unstar"}),
@@ -130,7 +144,7 @@ var padItemTool = ToolDef{
// keeping the schema simple to maintain.
var padItemSchemaParams = []ParamDef{
// ── Targeting ──
{Name: "ref", Type: "string", Description: "Item reference (e.g. TASK-5, IDEA-12, PLAYB-3, CONVE-7). Required for: update, delete, restore, get, move, link, unlink, deps, star, unstar, comment, list-comments, note, decide, export. NOT used for bulk-update — pass `refs` (array) instead."},
{Name: "ref", Type: "string", Description: "Item reference (e.g. TASK-5, IDEA-12, PLAYB-3, CONVE-7). Required for: update, delete, restore, get, move, link, unlink, deps, star, unstar, comment, list-comments, note, decide, export, remind. NOT used for ack-reminder (which addresses a REMINDER by `reminder_id`, since an item can carry several) and NOT used for bulk-update — pass `refs` (array) instead."},
{Name: "refs", Type: "array<string>", Description: "Item references for batch operations. Required for: bulk-update (one or more refs)."},
{Name: "target", Type: "string", Description: "The OTHER end of a relationship. Required for: link, unlink (paired with `ref` and `link_type`). For link_type=blocks, target is the item being blocked; for blocked-by it's the blocker; for supersedes it's the superseded item; etc."},
{Name: "link_type", Type: "string", Description: "Type of relationship for action=link/unlink.", Enum: []string{"blocks", "blocked-by", "supersedes", "implements", "split-from"}},
@@ -147,6 +161,10 @@ var padItemSchemaParams = []ParamDef{
// artifact carries the frontmatter the server needs to reconstruct
// the item's collection + typed fields. `export` returns this same
// text as its tool result.
// ── Reminders ── (IDEA-2641)
{Name: "remind_at", Type: "string", Description: "When a reminder should fire, as an RFC3339 INSTANT (e.g. 2026-08-01T09:00:00Z, or 2026-08-01T09:00:00-04:00 which is stored as the same moment in UTC). Required for: remind. A bare date (2026-08-01) is REFUSED, not assumed to mean midnight — it names a 24-hour span, and choosing an hour inside it would fire at a time nobody picked."},
{Name: "reminder_id", Type: "string", Description: "A reminder's id, as returned when it was armed. Required for: ack-reminder. Acknowledging removes a fired reminder from pad_project's next/ready surface; nothing else acknowledges one, and in particular completing the item does not."},
{Name: "artifact", Type: "string", Description: "Full portable artifact text (YAML frontmatter + Markdown body). Required for: import — this is the artifact a prior `export` produced. NOT the same as `content` (which is just the item's Markdown body)."},
// ── Status / priority / scheduling ──
@@ -311,6 +329,19 @@ Actions:
Required: ref, target, link_type.
deps Show all dependencies (incoming + outgoing) for an item.
Required: ref.
remind Arm a one-shot reminder that fires at a specific instant.
Required: ref, remind_at (RFC3339 INSTANT a bare date is
refused, since it names a 24-hour span rather than a moment).
When it fires, the item appears in pad_project next/ready
carrying the reminder_id, until you acknowledge it. Use this
when you defer work: it is how you ask to be reminded.
ack-reminder Acknowledge a fired reminder so it leaves next/ready.
Required: reminder_id (from the fired suggestion, or from
the response when you armed it NOT the item ref, since an
item can carry several reminders).
Nothing else acknowledges one: completing the item does not,
because a reminder may have been armed to fire after the
work was done.
star Star an item for quick access.
Required: ref.
unstar Remove star.
+9
View File
@@ -1621,6 +1621,15 @@ func TestFieldConflictProperty_SourcesDerivedFromTheDeclaredSchema(t *testing.T)
"message": true, "reply_to": true, // action=comment
"comment": true, // the audit note on update
// action=remind / ack-reminder (IDEA-2641). Neither writes an item
// FIELD: a reminder is a row in its own table addressed by its own
// id, so these cannot collide with `fields` or `field` the way a
// promoted key can. They are listed here rather than added to a
// classified key set for exactly that reason — detectFieldConflicts
// visiting them would be visiting something that is not a field
// source.
"remind_at": true, "reminder_id": true,
// The two SOURCES themselves, not keys within them.
"fields": true,
"field": true,
+18
View File
@@ -123,6 +123,8 @@ func TestReadOnlyCatalog_ActionsMatchCmdhelp(t *testing.T) {
{"pad_item", "move"}: {"item", "move"},
{"pad_item", "restore"}: {"item", "restore"},
{"pad_item", "deps"}: {"item", "deps"},
{"pad_item", "remind"}: {"item", "remind"},
{"pad_item", "ack-reminder"}: {"item", "ack"},
{"pad_item", "star"}: {"item", "star"},
{"pad_item", "unstar"}: {"item", "unstar"},
{"pad_item", "starred"}: {"item", "starred"},
@@ -278,6 +280,8 @@ func TestReadOnlyCatalog_ActionsDispatchExpectedCmdPath(t *testing.T) {
{"pad_item", "move"}: {"item", "move"},
{"pad_item", "restore"}: {"item", "restore"},
{"pad_item", "deps"}: {"item", "deps"},
{"pad_item", "remind"}: {"item", "remind"},
{"pad_item", "ack-reminder"}: {"item", "ack"},
{"pad_item", "star"}: {"item", "star"},
{"pad_item", "unstar"}: {"item", "unstar"},
{"pad_item", "starred"}: {"item", "starred"},
@@ -319,6 +323,10 @@ func TestReadOnlyCatalog_ActionsDispatchExpectedCmdPath(t *testing.T) {
"code": "123456",
// pad_attachment.show needs an attachment_id positional.
"attachment_id": "att-1",
// pad_item.ack-reminder addresses a REMINDER, not an item — an item
// can carry several, so `ref` cannot name one (IDEA-2641).
"reminder_id": "rem-1",
"remind_at": "2026-08-01T09:00:00Z",
}
for _, def := range Catalog {
@@ -645,6 +653,16 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document {
Args: mkArgs("ref"),
Flags: mkFlags("workspace"),
},
"item remind": {
Summary: "arm a reminder",
Args: mkArgs("ref"),
Flags: mkFlags("workspace", "remind-at", "rearm"),
},
"item ack": {
Summary: "acknowledge a fired reminder",
Args: mkArgs("reminder-id"),
Flags: mkFlags("workspace"),
},
"item star": {
Summary: "star item",
Args: mkArgs("ref"),
@@ -56,6 +56,8 @@ func parityFixtureInput() map[string]any {
"artifact": "---\ntitle: t\n---\nbody",
"url": "https://example.test/hook",
"status": "open",
"remind_at": "2026-08-01T09:00:00Z",
"reminder_id": "rem-1",
}
}
+16
View File
@@ -371,6 +371,22 @@ func init() {
// terminal status. Without --all, the handler hides them.
"item starred": mapItemStarred,
// --- Reminders (IDEA-2641) ---
// `item remind` takes the item's slug-or-ref in the URL, same as
// star/show/delete — handleCreateItemReminder resolves through
// store.ResolveItem, which accepts UUIDs, slugs and issue refs.
// `item ack` addresses the REMINDER instead: an item can carry
// several, so the id is the only thing that names one.
"item remind": routeSpec{
method: http.MethodPost,
pathTemplate: "/api/v1/workspaces/{workspace}/items/{ref}/reminders",
bodyKeys: []string{"remind_at"},
}.toRouteMapper(),
"item ack": routeSpec{
method: http.MethodPost,
pathTemplate: "/api/v1/workspaces/{workspace}/reminders/{reminder_id}/ack",
}.toRouteMapper(),
// --- Roles (admin) ---
"role create": mapRoleCreate,
"role update": mapRoleUpdate,
+2 -2
View File
@@ -6,13 +6,13 @@ Pad is a project tracker for developers and AI agents — issues (TASK, BUG), pl
If the user is asking general code questions with no project-management thread, you don't need this server.
## Tool surface (v0.27)
## Tool surface (v0.28)
Ten resource × action tools, plus `pad_set_workspace` (which takes a `workspace` slug only — no action enum). Eleven tools total.
Inputs are validated strictly: an undeclared top-level key is rejected with a structured `validation_failed` naming it, never accepted and silently dropped.
- `pad_item` — Items: create / update / delete / get / list / move / restore / link / unlink / deps / star / unstar / starred / comment / list-comments / backlinks / bulk-update / note / decide / export / import / history. On create/update, field values may be passed as a `fields` OBJECT (the same shape reads return, e.g. `{"status":"done","effort":"l"}`) — it merges into the same path as the dedicated params and `field: ["key=value"]`; the same key in two places with CONFLICTING values is refused, not silently resolved. `list` accepts `unparented: true` to keep items with no parent or implements relationship (mutually exclusive with `parent`). `list` results are SUMMARY-shaped by default on both transports — no content bodies; pass `full: true` for complete bodies (token-expensive), or prefer `get` for a single item's body. `update` field writes are a server-side field-level merge (only the keys you set change); pass `expected_updated_at` for optimistic concurrency (a stale value fails with a structured 409 `update_conflict`). `move` changes an item's COLLECTION within its workspace: system metadata (implementation notes, decision log, linked PR) survives it, and any field value the target schema has no home for is dropped AND reported in the move's activity entry — check there rather than assuming a move is lossless. Three system keys — `implementation_notes`, `decision_log`, `convention` — cannot be set through `field` on update or move; the call is refused with `validation_failed`, naming the key and the write path that does maintain it (`note`, `decide`, and library activation respectively). `github_pr` is the exception on UPDATE only — a move or copy still refuses it — because `pad github link` needs a local git checkout you don't have and an update would be your only way in. Be aware that it does not currently work either: a `field` value arrives as a string, so the PR data is stored double-encoded and no link appears (BUG-2696). Treat linking a PR as something to hand to a human for now, rather than a call to retry. On `create` none of them are blocked, since that door is shared with Pad's own writers — but don't hand-write `implementation_notes` / `decision_log` there either. Doing so does not merely fail to help: it stores something Pad cannot read back, which hides the existing entries on every surface and makes `note` / `decide` refuse on that item until it is repaired. `history` returns read-only item version metadata (newest-first), bounded to the NEWEST 50 versions by default (max 300 — pass `limit` to change the window); pass `full: true` to include each version's resolved content body (token-expensive). There is no `offset`: versions are stored as reverse patches, so only a newest-end window is cheap to reconstruct. To UNASSIGN an item, pass `clear_assigned_user: true` (or `clear_agent_role: true`) — the canonical form, works on both transports. To DETACH an item from its parent, pass `clear_parent: true` — same canonical shape, works on both transports. Setting and clearing the same field in one call is refused, not silently resolved, so don't pair `clear_assigned_user`/`clear_agent_role`/`clear_parent` with `assign`/`role`/`parent` respectively. An empty `assign` / `role` / `parent` does NOT clear: those name a person, a slug, or a ref, so an empty value reads as "not provided", exactly like every other optional string here. (Two older forms still work and are not deprecated: `field: ["assigned_user_id="]` on either transport, and a direct `assigned_user_id: ""` param over remote `/mcp` only — prefer the boolean, which is the only one this schema advertises.)
- `pad_item` — Items: create / update / delete / get / list / move / restore / link / unlink / deps / star / unstar / starred / comment / list-comments / backlinks / bulk-update / note / decide / export / import / history / remind / ack-reminder. On create/update, field values may be passed as a `fields` OBJECT (the same shape reads return, e.g. `{"status":"done","effort":"l"}`) — it merges into the same path as the dedicated params and `field: ["key=value"]`; the same key in two places with CONFLICTING values is refused, not silently resolved. `list` accepts `unparented: true` to keep items with no parent or implements relationship (mutually exclusive with `parent`). `list` results are SUMMARY-shaped by default on both transports — no content bodies; pass `full: true` for complete bodies (token-expensive), or prefer `get` for a single item's body. `update` field writes are a server-side field-level merge (only the keys you set change); pass `expected_updated_at` for optimistic concurrency (a stale value fails with a structured 409 `update_conflict`). `move` changes an item's COLLECTION within its workspace: system metadata (implementation notes, decision log, linked PR) survives it, and any field value the target schema has no home for is dropped AND reported in the move's activity entry — check there rather than assuming a move is lossless. Three system keys — `implementation_notes`, `decision_log`, `convention` — cannot be set through `field` on update or move; the call is refused with `validation_failed`, naming the key and the write path that does maintain it (`note`, `decide`, and library activation respectively). `github_pr` is the exception on UPDATE only — a move or copy still refuses it — because `pad github link` needs a local git checkout you don't have and an update would be your only way in. Be aware that it does not currently work either: a `field` value arrives as a string, so the PR data is stored double-encoded and no link appears (BUG-2696). Treat linking a PR as something to hand to a human for now, rather than a call to retry. On `create` none of them are blocked, since that door is shared with Pad's own writers — but don't hand-write `implementation_notes` / `decision_log` there either. Doing so does not merely fail to help: it stores something Pad cannot read back, which hides the existing entries on every surface and makes `note` / `decide` refuse on that item until it is repaired. `history` returns read-only item version metadata (newest-first), bounded to the NEWEST 50 versions by default (max 300 — pass `limit` to change the window); pass `full: true` to include each version's resolved content body (token-expensive). There is no `offset`: versions are stored as reverse patches, so only a newest-end window is cheap to reconstruct. To UNASSIGN an item, pass `clear_assigned_user: true` (or `clear_agent_role: true`) — the canonical form, works on both transports. To DETACH an item from its parent, pass `clear_parent: true` — same canonical shape, works on both transports. Setting and clearing the same field in one call is refused, not silently resolved, so don't pair `clear_assigned_user`/`clear_agent_role`/`clear_parent` with `assign`/`role`/`parent` respectively. An empty `assign` / `role` / `parent` does NOT clear: those name a person, a slug, or a ref, so an empty value reads as "not provided", exactly like every other optional string here. `remind` arms a one-shot reminder on an item: pass `remind_at` as an RFC3339 INSTANT (`2026-08-01T09:00:00Z`; an offset is stored as the same moment in UTC). A bare date is REFUSED rather than read as midnight, because it names a 24-hour span and picking an hour inside it would fire at a time nobody chose. When it fires the item appears in `pad_project.action: next` / `ready` until you acknowledge it with `ack-reminder` (which takes a `reminder_id` — carried on the fired suggestion itself as `reminder_id`, so a poller that never armed it can still retire it, and also returned when you arm one) — that poll surface is the delivery path on any instance without a webhook configured, so it is where you will actually see it. Nothing else acknowledges a reminder: completing the item does NOT, because a reminder may have been armed precisely to fire after the work was done. A reminder on a completed item is hidden from next/ready and left untouched in place. (Two older forms still work and are not deprecated: `field: ["assigned_user_id="]` on either transport, and a direct `assigned_user_id: ""` param over remote `/mcp` only — prefer the boolean, which is the only one this schema advertises.)
- `pad_workspace` — Workspaces: list / members / invite / storage / audit-log / create / claim / deleted / restore.
- `pad_collection` — Collections: list / create / update / delete.
- `pad_project` — Project intelligence: dashboard / next / ready / stale / standup / changelog / report / activity. Use `ready` for the actionable backlog and `stale` for items needing attention; `activity` to catch up on what other agents/users changed since you last worked (non-streaming feed with item refs + change details).
+31 -2
View File
@@ -620,7 +620,7 @@ const CmdhelpVersion = "0.1"
// condition would stop matching. That client was retrying a
// permanent failure.
// - "0.26" — current. IDEA-2756: `pad_workspace.action=create` is now
// - "0.26" — IDEA-2756: `pad_workspace.action=create` is now
// REFUSED with a 403 when the calling OAuth connection's grant has
// `may_create_workspaces=false`. Previously that flag gated only the
// post-creation auto-add, so the create succeeded — and on a
@@ -789,7 +789,36 @@ const CmdhelpVersion = "0.1"
// shape changed, and the behaviour did. Every refusal added here
// replaced a call that SUCCEEDED while doing something other than
// what it said, so the break is the fix in each case.
const ToolSurfaceVersion = "0.27"
//
// 0.28 — IDEA-2641 / GitHub #1010. Two ADDITIVE actions on
// `pad_item`: `remind` arms a one-shot reminder at an RFC3339
// instant (`remind_at`), and `ack-reminder` acknowledges a fired
// one by id (`reminder_id`). Two new params, both optional, both
// ignored by every other action. Purely additive — no existing
// name, enum or shape moved, and a 0.27 consumer that enumerates
// neither action is unaffected. Same disposition as v0.13, v0.11
// and v0.8, which likewise wired existing CLI verbs onto the
// catalog.
//
// WHY AN AGENT NEEDS THIS AT ALL, since agents already RECEIVE
// reminders without it: the poll surface is `pad_project.action:
// next` / `ready`, which were already exposed, so a reminder was
// already reaching agents. What was missing is the other half —
// deferring a piece of work is exactly the moment an agent knows
// when it wants to be asked again, and it had no way to say so.
//
// `remind_at` REFUSES a bare date rather than reading it as
// midnight. That is a refusal at the edge of a brand-new param, so
// it breaks nothing, but it is stated here because the `date`
// schema type accepts `YYYY-MM-DD` and a caller will reasonably
// try it: a bare date names a 24-hour span, and choosing an hour
// inside it would be the server firing at a time nobody picked.
//
// Re-arm and disarm are deliberately CLI-ONLY for now. Both address
// a reminder by an id the agent would have to list first, and no
// listing action exists on this surface — a door with no handle.
// Adding them later is additive.
const ToolSurfaceVersion = "0.28"
// MetaVersionURI is the canonical URI of the queryable version document.
// Lives outside the pad://workspace/{ws}/... namespace because it's a
+25
View File
@@ -10,6 +10,31 @@ type WorkspaceExport struct {
Comments []CommentExport `json:"comments,omitempty"`
ItemLinks []ItemLinkExport `json:"item_links,omitempty"`
ItemVersions []ItemVersionExport `json:"item_versions,omitempty"`
// Reminders round-trip with the workspace (IDEA-2641). They are
// item-scoped workspace CONTENT, like links and versions, not per-user
// state like stars and watches — which is the line this list has always
// drawn, and it puts reminders on the exported side of it. Without them a
// backup/restore or a SQLite→Postgres migration silently loses every
// pending reminder, and "silently" is the part that matters: nothing in
// the destination would show that anything was dropped.
Reminders []ReminderExport `json:"reminders,omitempty"`
}
// ReminderExport is one item reminder in a workspace bundle.
//
// The LIFECYCLE MARKS ARE CARRIED, not reset. A fired-and-unacknowledged
// reminder is still owed to whoever armed it, so it arrives pending on the
// destination; an armed one whose instant has passed fires once on the first
// tick there, which is the same thing that would have happened had the
// workspace never moved. Re-arming everything on import would be inventing a
// new schedule the user did not set.
type ReminderExport struct {
ItemID string `json:"item_id"`
RemindAt string `json:"remind_at"`
FiredAt string `json:"fired_at,omitempty"`
AckedAt string `json:"acked_at,omitempty"`
CreatedAt string `json:"created_at"`
UpdatedAt string `json:"updated_at"`
}
// AttachmentManifestEntry describes one attachment blob in the
+93
View File
@@ -0,0 +1,93 @@
package models
// Reminder is a one-shot, fire-at-an-instant signal attached to an item
// (IDEA-2641, GitHub #1010).
//
// It is deliberately NOT a schema field. See migration 085 for why the
// annotation-on-a-FieldDef shape was overturned; the short form is that any
// key added to FieldDef is silently dropped both by the web collection editor
// (which rebuilds each field key-by-key from an allowlist) and by any Go
// unmarshal+marshal round-trip through CollectionSchema, which has fixed
// fields. A reminder also has a LIFECYCLE that a field definition has nowhere
// to keep.
//
// The lifecycle is three states, and they are three states rather than two
// because acking must not re-arm:
//
// ARMED fired_at IS NULL — a tick may fire it
// FIRED-UNACKED fired_at set, acked_at NULL — on the poll surface
// FIRED-ACKED both set — history
//
// Re-arming (moving RemindAt on a fired row) returns it to ARMED by clearing
// both marks.
type Reminder struct {
ID string `json:"id"`
WorkspaceID string `json:"workspace_id"`
ItemID string `json:"item_id"`
// RemindAt is an RFC3339 instant in UTC, always. It is not a `date`
// schema value: those admit both YYYY-MM-DD and RFC3339 and are compared
// against the server's LOCAL calendar day, an ambiguity a fire-at time
// cannot carry.
RemindAt string `json:"remind_at"`
// FiredAt is nil while armed. Non-nil means the scheduler emitted this
// reminder's event; it is never cleared except by an explicit re-arm.
FiredAt *string `json:"fired_at,omitempty"`
// AckedAt is nil until a caller explicitly acknowledges. NOTHING else
// acks — in particular an item reaching a terminal status does not, which
// would both couple every status write to reminder state and silently
// consume a reminder set to fire after the work was done. The poll
// surface filters terminal-item reminders out of its listing instead,
// leaving the row untouched.
AckedAt *string `json:"acked_at,omitempty"`
CreatedAt string `json:"created_at"`
UpdatedAt string `json:"updated_at"`
}
// Armed reports whether a tick would still consider this reminder.
func (r *Reminder) Armed() bool { return r.FiredAt == nil }
// PendingAck reports whether this reminder has fired and not been
// acknowledged — the set the agent poll surface reads.
func (r *Reminder) PendingAck() bool { return r.FiredAt != nil && r.AckedAt == nil }
// ReminderCreate is the input shape for arming a reminder. RemindAt is
// required and must parse as RFC3339; the handler normalizes it to UTC before
// it reaches the store, so the store never has to reason about zones.
type ReminderCreate struct {
ItemID string `json:"item_id"`
RemindAt string `json:"remind_at"`
}
// ReminderUpdate carries a re-arm. RemindAt is the only mutable field: a
// reminder has no other content to change, and making the ONE mutation that
// exists also the one that clears the fire marks keeps the re-arm rule
// ("changing remind_at on a fired row re-arms it") impossible to apply
// half-way.
type ReminderUpdate struct {
RemindAt string `json:"remind_at"`
}
// PendingReminder is a fired-and-unacked reminder joined to the item it is
// about — what the poll surface renders. The item fields are carried here
// rather than fetched per-row because the surface's whole job is to be one
// cheap query an agent runs often.
type PendingReminder struct {
Reminder
ItemRef string `json:"item_ref"`
ItemTitle string `json:"item_title"`
ItemSlug string `json:"item_slug"`
CollectionSlug string `json:"collection_slug"`
// ItemFields and CollectionID exist for the caller's terminal-status
// filter and are not part of the wire shape. Terminality is defined by a
// collection's schema, so the filter cannot run in SQL; it runs where the
// dashboard already builds that context. json:"-" because a pending
// reminder is a notification, not a second way to read an item's fields.
ItemFields string `json:"-"`
CollectionID string `json:"-"`
}
+24 -5
View File
@@ -390,6 +390,14 @@ type BootstrapDashboard struct {
ActiveItemsOverflowCount int `json:"active_items_overflow_count,omitempty"`
ActivePlansOverflowCount int `json:"active_plans_overflow_count,omitempty"`
ByRoleOverflowCount int `json:"by_role_overflow_count,omitempty"`
// PendingRemindersOverflowCount caps the reminder list (IDEA-2641). It
// needs one where suggested_next does not, and the difference is the whole
// reason the note above is worth reading: suggested_next is capped at 3
// upstream, so a bootstrap cap of 5 could never fire, while
// pending_reminders arrives with a window of up to 50 and would otherwise
// embed all of them in the boot payload — which is the budget PLAN-1410
// spent a whole unit trimming.
PendingRemindersOverflowCount int `json:"pending_reminders_overflow_count,omitempty"`
}
// Bootstrap caps clamp the per-array sizes in the bootstrap dashboard
@@ -401,11 +409,18 @@ type BootstrapDashboard struct {
// remaining three (active_items / active_plans / by_role) are TASK-1422
// (IDEA-1421 absorbed). suggested_next is excluded — upstream cap of 3.
const (
bootstrapAttentionCap = 5
bootstrapRecentActivityCap = 5
bootstrapActiveItemsCap = 5
bootstrapActivePlansCap = 5
bootstrapByRoleCap = 5
bootstrapAttentionCap = 5
// Its own constant rather than borrowing bootstrapAttentionCap, which it
// happens to equal: the two answer different questions, and a future
// change to how much ATTENTION an agent should see must not silently
// change how many REMINDERS it sees. Five for the same reason as its
// neighbours — the practical depth for a greeting or status pass, with the
// overflow count telling the agent to pull the full dashboard.
bootstrapPendingRemindersCap = 5
bootstrapRecentActivityCap = 5
bootstrapActiveItemsCap = 5
bootstrapActivePlansCap = 5
bootstrapByRoleCap = 5
)
// BuildAgentBootstrap assembles the bootstrap blob from store queries.
@@ -1012,6 +1027,10 @@ func capBootstrapDashboard(d *DashboardResponse) *BootstrapDashboard {
copied.RecentActivity = copied.RecentActivity[:bootstrapRecentActivityCap]
out.RecentActivityOverflowCount = n
}
if n := len(copied.PendingReminders) - bootstrapPendingRemindersCap; n > 0 {
copied.PendingReminders = copied.PendingReminders[:bootstrapPendingRemindersCap]
out.PendingRemindersOverflowCount = n
}
if n := len(copied.ActiveItems) - bootstrapActiveItemsCap; n > 0 {
copied.ActiveItems = copied.ActiveItems[:bootstrapActiveItemsCap]
out.ActiveItemsOverflowCount = n
+242 -44
View File
@@ -24,6 +24,20 @@ type DashboardResponse struct {
Attention []DashboardAttention `json:"attention"`
RecentActivity []DashboardActivity `json:"recent_activity"`
SuggestedNext []DashboardSuggestion `json:"suggested_next"`
// PendingReminders are fired-but-unacknowledged reminders (IDEA-2641).
//
// THIS IS THE MANDATORY SURFACE, not a convenience: the outbox drain acks
// an event immediately when no webhook dispatcher is configured, which is
// the common self-hosted shape, so a reminder delivered only by webhook
// would be a no-op on most installs. On those instances this list is the
// entire delivery mechanism.
PendingReminders []DashboardReminder `json:"pending_reminders,omitempty"`
// PendingRemindersTruncated says the window above was not the whole set.
// A BOOLEAN rather than a count, deliberately: a count would have to be
// post-visibility-filter to be true for the caller reading it, and the
// store cannot compute that — the filter runs per item, up here. "There
// are more than you can see" is the strongest honest claim.
PendingRemindersTruncated bool `json:"pending_reminders_truncated,omitempty"`
// HasAgentActivity is true when any non-deleted item in the workspace
// was created via an agent surface — direct CLI or Remote MCP (both
// paths persist source='cli'; future MCP-distinct attribution would
@@ -164,6 +178,26 @@ type DashboardSuggestion struct {
ItemTitle string `json:"item_title"`
Collection string `json:"collection"`
Reason string `json:"reason"`
// ReminderID is set only on suggestions produced by a fired reminder
// (IDEA-2641). It is the id an acknowledgement addresses — without it a
// poller reading this surface can SEE the reminder and has no way to
// retire it, which was the shape codex round 1 caught: the docs told
// agents to ack what they saw here, and the payload did not carry the
// handle.
ReminderID string `json:"reminder_id,omitempty"`
}
// DashboardReminder is one fired-and-unacknowledged reminder, rendered with
// the item it is about.
type DashboardReminder struct {
ID string `json:"id"`
ItemSlug string `json:"item_slug"`
ItemRef string `json:"item_ref"`
ItemTitle string `json:"item_title"`
Collection string `json:"collection"`
RemindAt string `json:"remind_at"`
FiredAt string `json:"fired_at"`
}
// Blocked-item resolution (both the attention-blocked section and the
@@ -597,30 +631,68 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
}
}
// (b) Overdue: items with a due_date or end_date in the past whose
// done field isn't in a terminal state.
todayStr := now.Format("2006-01-02")
// (b) Overdue: items past a deadline whose done field isn't terminal.
//
// The rule itself lives in overdue.go now, and this is one of four
// surfaces that call it — the others being `pad project stale` (which
// filters this very list) and `ready` / `next` (which rank on it below).
// Before IDEA-2641 the rule WAS this loop, so the recommendation surface
// had no deadline awareness at all.
todayStr := overdueToday(now)
for _, item := range allItems {
if isItemDone(item.Fields, item.CollectionID, ctxMap) {
continue
}
for _, dateField := range []string{"due_date", "end_date"} {
dateVal := extractFieldValue(item.Fields, dateField)
if dateVal == "" {
continue
}
// Compare date strings lexicographically (YYYY-MM-DD format)
if dateVal < todayStr {
resp.Attention = append(resp.Attention, DashboardAttention{
Type: "overdue",
ItemSlug: item.Slug,
ItemRef: item.Ref,
ItemTitle: item.Title,
Collection: item.CollectionSlug,
Reason: strings.ReplaceAll(dateField, "_", " ") + " was " + dateVal,
})
break // only report once per item even if both fields are overdue
if field, value, ok := itemOverdue(item.Fields, todayStr); ok {
resp.Attention = append(resp.Attention, DashboardAttention{
Type: "overdue",
ItemSlug: item.Slug,
ItemRef: item.Ref,
ItemTitle: item.Title,
Collection: item.CollectionSlug,
Reason: overdueReason(field, value),
})
}
}
// (b2) Fired-but-unacknowledged reminders (IDEA-2641).
//
// TERMINAL-ITEM REMINDERS ARE FILTERED, NOT ACKED. Acking on terminal
// status would make every status write a reminder mutation, and it would
// consume a reminder a user may have armed precisely to fire after the
// work was done. Filtering leaves the row exactly as the user left it —
// armed, fired, unacked, still theirs — while keeping a finished item off
// the surface an agent polls. The distinction is observable: the reminder
// is absent from here and present in the table.
// Visibility is scoped IN THE QUERY, using the same collection/item id sets
// every other section here reads through `allItems` (codex rounds 1 and 4).
// Filtering after a bounded window is what let fifty invisible rows hide a
// visible one forever, so the only filter left above is terminality, which
// SQL cannot evaluate — a collection's schema defines it. That one is
// handled by PAGING: a page that comes back short of the window is refilled
// from the next page, bounded so a workspace full of completed items cannot
// turn a dashboard read into a table scan.
if pending, truncated, err := s.collectPendingReminders(workspaceID, store.PendingReminderScope{
CollectionIDs: dashCollIDs,
ItemIDs: dashItemIDs,
}, ctxMap); err != nil {
markDegraded("pending_reminders", err)
} else {
resp.PendingRemindersTruncated = truncated
for _, pr := range pending {
firedAt := ""
if pr.FiredAt != nil {
firedAt = *pr.FiredAt
}
resp.PendingReminders = append(resp.PendingReminders, DashboardReminder{
ID: pr.ID,
ItemSlug: pr.ItemSlug,
ItemRef: pr.ItemRef,
ItemTitle: pr.ItemTitle,
Collection: pr.CollectionSlug,
RemindAt: pr.RemindAt,
FiredAt: firedAt,
})
}
}
@@ -843,6 +915,12 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
status string
priority int
inProgress bool
// overdue and overdueReason carry the deadline verdict from the
// shared helper so the sort and the reason text read the same
// judgement — recomputing it at render time is how the attention
// entry and the suggestion would drift.
overdue bool
overdueReason string
}
var candidates []suggestion
@@ -875,12 +953,15 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
continue
}
pri := extractFieldValue(task.Fields, "priority")
odField, odValue, isOverdue := itemOverdue(task.Fields, todayStr)
candidates = append(candidates, suggestion{
item: task,
plan: dp.Title,
status: taskStatus,
priority: priorityRank(pri),
inProgress: isInProgress,
item: task,
plan: dp.Title,
status: taskStatus,
priority: priorityRank(pri),
inProgress: isInProgress,
overdue: isOverdue,
overdueReason: overdueReasonOrEmpty(odField, odValue, isOverdue),
})
}
}
@@ -912,10 +993,17 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
if _, dup := seen[item.ID]; dup {
continue
}
// Skip non-tasks (the active-plan loop walks plan children;
// the orphan branch is similarly task-shaped). isCollectionVisible
// + collection-task gating mirrors the active-plan branch's
// shape so behaviour stays consistent.
// The comment that stood here claimed this branch gates on task
// collections "mirroring the active-plan branch". It does not, and
// never did — the only gate below is collection VISIBILITY, so an
// idea or a doc could always reach suggested_next. The overdue bypass
// (IDEA-2641) widened that from high-priority items to any overdue
// one, which is how codex round 5 found it.
//
// Rather than narrow the branch — which would silently drop the
// high-priority non-task items it has surfaced since BUG-1082 — the
// output now carries each item's REAL collection instead of asserting
// "tasks", so the surface stops mislabelling what it recommends.
if !isCollectionVisible(item.CollectionID, visibleIDs) {
continue
}
@@ -929,21 +1017,31 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
continue
}
pri := extractFieldValue(item.Fields, "priority")
odField, odValue, isOverdue := itemOverdue(item.Fields, todayStr)
// Open orphans must be high or critical to surface — open
// in-progress items always do (continuing-work signal beats
// priority gating).
if !isInProgress && pri != "high" && pri != "critical" {
//
// AN OVERDUE ITEM BYPASSES THAT GATE (IDEA-2641). A deadline that has
// already passed is a stronger actionability signal than the priority
// someone typed when they filed it, and without this the gate is
// where the deadline would quietly stop: a low-priority orphan three
// weeks late would be reported by `stale` and never suggested by
// `next`, which is the exact split GitHub #1010 is about.
if !isInProgress && !isOverdue && pri != "high" && pri != "critical" {
continue
}
if _, blocked := firstActiveBlocker[item.ID]; blocked {
continue
}
candidates = append(candidates, suggestion{
item: item,
plan: "", // empty plan name signals orphan in the reason text below
status: taskStatus,
priority: priorityRank(pri),
inProgress: isInProgress,
item: item,
plan: "", // empty plan name signals orphan in the reason text below
status: taskStatus,
priority: priorityRank(pri),
inProgress: isInProgress,
overdue: isOverdue,
overdueReason: overdueReasonOrEmpty(odField, odValue, isOverdue),
})
}
@@ -952,6 +1050,14 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
// "active-plan continuation" suggestion stays at the top when
// both are present. Lower rank = higher priority.
sort.Slice(candidates, func(i, j int) bool {
// OVERDUE FIRST, above in-progress (IDEA-2641). The list is capped at
// three, so a rank below in-progress would not merely order the
// deadline lower — on any workspace with three things in flight it
// would keep an overdue item off the surface entirely, which is
// indistinguishable from not implementing this at all.
if candidates[i].overdue != candidates[j].overdue {
return candidates[i].overdue
}
if candidates[i].inProgress != candidates[j].inProgress {
return candidates[i].inProgress
}
@@ -966,36 +1072,128 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
return iPlan && !jPlan
})
// Take top 3
limit := 3
// Take top 3. maxSuggestions is a CONSTANT and the trim below uses it
// rather than `limit`, which is reassigned to len(candidates) when there
// are fewer — reusing it would truncate the combined list to zero on a
// workspace whose only entries are reminders, which is exactly the case
// the reminder surface exists for.
const maxSuggestions = 3
limit := maxSuggestions
if len(candidates) < limit {
limit = len(candidates)
}
for _, c := range candidates[:limit] {
pri := extractFieldValue(c.item.Fields, "priority")
// "task" only when it IS one. The orphan branch admits any collection
// (see above), so hardcoding the noun mislabels an idea or a doc as a
// task in the one place an agent reads to decide what to do next.
noun := "item"
if c.item.CollectionSlug == "tasks" {
noun = "task"
}
var reason string
switch {
case c.inProgress && c.plan != "":
reason = "In-progress task in active plan \"" + c.plan + "\""
reason = "In-progress " + noun + " in active plan \"" + c.plan + "\""
case c.inProgress:
reason = "In-progress task"
reason = "In-progress " + noun
case c.plan != "":
reason = "Open task in active plan \"" + c.plan + "\""
reason = "Open " + noun + " in active plan \"" + c.plan + "\""
default:
reason = "Open task"
reason = "Open " + noun
}
if pri != "" {
reason += " (" + pri + " priority)"
}
if c.overdue {
// Prefixed rather than appended: the deadline is why this is at
// the top of the list, and a reason that leads with "Open task"
// buries the part that changed the ranking.
reason = "OVERDUE — " + c.overdueReason + "; " + reason
}
resp.SuggestedNext = append(resp.SuggestedNext, DashboardSuggestion{
ItemSlug: c.item.Slug,
ItemRef: c.item.Ref,
ItemTitle: c.item.Title,
Collection: "tasks",
ItemSlug: c.item.Slug,
ItemRef: c.item.Ref,
ItemTitle: c.item.Title,
// The item's REAL collection, not the literal "tasks" that stood
// here: this branch admits any collection, so the constant was a
// claim the data did not support.
Collection: c.item.CollectionSlug,
Reason: reason,
})
}
// Fired reminders lead the recommendation list (IDEA-2641).
//
// They are prepended rather than entered as ranking candidates: a reminder
// is not a task competing on priority, it is an instruction the user left
// for this moment, and whether it appeared should not depend on how busy
// the workspace is. But the COMBINED list is then trimmed back to the same
// cap this surface has always had.
//
// Trimming was the round-11 correction. Prepending after the cap made
// suggested_next return up to eight entries where every consumer — the web
// dashboard, `pad project next`, `pad project ready` — was written against
// three. Worse, it silently falsified a decision recorded in
// BootstrapDashboard: that projection deliberately has no
// suggested_next_overflow_count BECAUSE this list is capped at three
// upstream, and its comment names raising that cap as the moment to add
// one. Raising it here would have made another unit's reasoning wrong
// somewhere else in the tree.
//
// A reminder can now push a task suggestion out, which is the right way
// round: the user asked to be told about this now, and the full set stays
// addressable in pending_reminders regardless.
//
// The same filtered set feeds resp.PendingReminders, which is the
// ADDRESSABLE form — it carries the reminder id an acknowledgement needs.
// This is the rendered form, for the surfaces that show a human or an
// agent what to do next. Both derive from the one list built above rather
// than each re-querying, so they cannot disagree about what is pending.
if len(resp.PendingReminders) > 0 {
// CAPPED SEPARATELY from the pending list. suggested_next is a
// recommendation — three entries by construction — and prepending an
// unbounded number of reminders turns it into a second inbox, burying
// the suggestions it exists to make. The full set stays addressable in
// pending_reminders; this is the "what should I do next" view of it.
const maxReminderSuggestions = 5
reminderSuggestions := make([]DashboardSuggestion, 0, maxReminderSuggestions)
for _, pr := range resp.PendingReminders {
if len(reminderSuggestions) == maxReminderSuggestions {
break
}
reminderSuggestions = append(reminderSuggestions, DashboardSuggestion{
ReminderID: pr.ID,
ItemSlug: pr.ItemSlug,
ItemRef: pr.ItemRef,
ItemTitle: pr.ItemTitle,
Collection: pr.Collection,
Reason: "REMINDER due — armed for " + pr.RemindAt,
})
}
// ONE ENTRY PER ITEM ACROSS THE TWO SOURCES (codex round 17). An item
// can be both a fired reminder and an ordinary candidate (in progress,
// high priority, overdue); the reminder entry carries the ack handle
// and the ordinary one carries nothing the reminder does not, so the
// ordinary one is dropped. Two REMINDERS on one item stay two entries:
// each is a separate thing to acknowledge.
remindedItems := make(map[string]struct{}, len(reminderSuggestions))
for _, rs := range reminderSuggestions {
remindedItems[rs.ItemSlug] = struct{}{}
}
kept := make([]DashboardSuggestion, 0, len(resp.SuggestedNext))
for _, sg := range resp.SuggestedNext {
if _, dup := remindedItems[sg.ItemSlug]; dup {
continue
}
kept = append(kept, sg)
}
resp.SuggestedNext = append(reminderSuggestions, kept...)
if len(resp.SuggestedNext) > maxSuggestions {
resp.SuggestedNext = resp.SuggestedNext[:maxSuggestions]
}
}
// Role breakdown: items per role with assigned users.
// When visibility is restricted, recompute from visible items only.
if visibleIDs != nil {
+407
View File
@@ -0,0 +1,407 @@
package server
import (
"errors"
"net/http"
"time"
"github.com/PerpetualSoftware/pad/internal/models"
"github.com/PerpetualSoftware/pad/internal/store"
"github.com/go-chi/chi/v5"
)
// Item reminder handlers (IDEA-2641, GitHub #1010).
//
// The write surface is deliberately small: arm, re-arm, acknowledge, disarm.
// A reminder has no content of its own — it is an instant and a lifecycle —
// so there is nothing else to edit.
// reminderRequest is the arm/re-arm body.
type reminderRequest struct {
RemindAt string `json:"remind_at"`
}
// parseRemindAt normalizes a caller-supplied instant to RFC3339 UTC.
//
// THE PARSE IS STRICT AND THE NORMALIZATION HAPPENS HERE, once, at the edge.
// Everything downstream compares remind_at as a string against a UTC clock, so
// a value that reached the store still carrying a local offset would compare
// wrong by that offset — and it would compare wrong SILENTLY, firing early or
// late with nothing in the row to show why. Doing it at the boundary means the
// store never has to reason about zones and there is exactly one place that
// decides what an instant means.
//
// A BARE DATE IS REFUSED, and this is the one refusal worth explaining: the
// `date` schema type accepts `YYYY-MM-DD`, so a caller reasonably expects it
// here too. But a bare date does not name an instant — "2026-08-01" is a
// 24-hour span, and picking midnight for them would be this code inventing a
// time the user did not choose and then firing at it. Refusing with a message
// that names the accepted form costs one round trip; guessing costs a reminder
// that arrives at 00:00 for someone who meant "that morning".
func parseRemindAt(raw string) (string, error) {
t, err := time.Parse(time.RFC3339, raw)
if err != nil {
return "", err
}
return store.NormalizeInstant(t), nil
}
func writeRemindAtError(w http.ResponseWriter) {
writeError(w, http.StatusBadRequest, "invalid_remind_at",
"remind_at must be an RFC3339 instant (e.g. 2026-08-01T09:00:00Z). A bare date has no time of day, so it is refused rather than assumed to mean midnight.")
}
// handleListItemReminders returns every reminder on an item, armed or fired.
// GET /api/v1/workspaces/{slug}/items/{itemSlug}/reminders
//
// ARCHIVED ITEMS ARE READABLE HERE, AND ONLY HERE (codex round 16). This
// route follows handleGetItem: an archived item resolves read-only, so its
// reminder history stays visible after the item is archived — which is why
// the store's reminderOwned enforces identity and not liveness. The lifecycle
// verbs (arm, re-arm, ack, delete) follow every other item MUTATION instead
// and answer 409 "archived … restore it before editing" through the same
// writeItemResolveError the rest of the API uses. Nothing waits on an ack the
// door refuses: the candidate scan and the pending surface already exclude an
// archived item's reminders, RestoreItem brings them back exactly as they
// were, and a hard delete cascades the rows away.
func (s *Server) handleListItemReminders(w http.ResponseWriter, r *http.Request) {
workspaceID, ok := s.getWorkspaceID(w, r)
if !ok {
return
}
itemSlug := chi.URLParam(r, "itemSlug")
item, err := s.store.ResolveItemIncludeDeleted(workspaceID, itemSlug)
if err != nil {
writeInternalError(w, err)
return
}
if item == nil {
writeError(w, http.StatusNotFound, "not_found", "Item not found")
return
}
if !s.requireItemVisible(w, r, workspaceID, item) {
return
}
reminders, err := s.store.ListRemindersForItem(workspaceID, item.ID)
if err != nil {
writeInternalError(w, err)
return
}
writeJSON(w, http.StatusOK, map[string]any{
"ref": item.Ref,
"reminders": reminders,
})
}
// handleCreateItemReminder arms a reminder on an item.
// POST /api/v1/workspaces/{slug}/items/{itemSlug}/reminders
func (s *Server) handleCreateItemReminder(w http.ResponseWriter, r *http.Request) {
workspaceID, ok := s.getWorkspaceID(w, r)
if !ok {
return
}
item := s.resolveVisibleItem(w, r, workspaceID)
if item == nil {
return
}
if !s.requireEditPermission(w, r, workspaceID, item.ID, item.CollectionID) {
return
}
var req reminderRequest
if err := decodeJSON(r, &req); err != nil {
writeError(w, http.StatusBadRequest, "invalid_body", "Request body must be JSON")
return
}
remindAt, err := parseRemindAt(req.RemindAt)
if err != nil {
writeRemindAtError(w)
return
}
reminder, err := s.store.CreateReminder(workspaceID, item.ID, remindAt)
if errors.Is(err, store.ErrReminderItemGone) {
// resolveVisibleItem saw a live item in this workspace a moment ago;
// the store's own predicate did not. The item was archived in the
// window, and the answer is the one the resolver would have given.
writeError(w, http.StatusNotFound, "not_found", "Item not found")
return
}
if err != nil {
writeInternalError(w, err)
return
}
writeJSON(w, http.StatusCreated, reminder)
}
// handleRearmReminder moves a reminder's instant, clearing its fire marks.
// PATCH /api/v1/workspaces/{slug}/reminders/{reminderID}
func (s *Server) handleRearmReminder(w http.ResponseWriter, r *http.Request) {
workspaceID, ok := s.getWorkspaceID(w, r)
if !ok {
return
}
reminder, _ := s.resolveReminderForWrite(w, r, workspaceID)
if reminder == nil {
return
}
var req reminderRequest
if err := decodeJSON(r, &req); err != nil {
writeError(w, http.StatusBadRequest, "invalid_body", "Request body must be JSON")
return
}
remindAt, err := parseRemindAt(req.RemindAt)
if err != nil {
writeRemindAtError(w)
return
}
updated, err := s.store.RearmReminder(workspaceID, reminder.ID, remindAt)
if err != nil {
writeInternalError(w, err)
return
}
if updated == nil {
writeError(w, http.StatusNotFound, "not_found", "Reminder not found")
return
}
writeJSON(w, http.StatusOK, updated)
}
// handleAckReminder acknowledges a fired reminder.
// POST /api/v1/workspaces/{slug}/reminders/{reminderID}/ack
func (s *Server) handleAckReminder(w http.ResponseWriter, r *http.Request) {
workspaceID, ok := s.getWorkspaceID(w, r)
if !ok {
return
}
reminder, _ := s.resolveReminderForWrite(w, r, workspaceID)
if reminder == nil {
return
}
acked, err := s.store.AckReminder(workspaceID, reminder.ID)
if err != nil {
writeInternalError(w, err)
return
}
if acked == nil {
// THE PRE-READ IS NOT CONSULTED HERE (codex round 12). AckReminder
// matches every fired row, acknowledged or not, so a nil answer means
// exactly "not fired at the instant of the ack" — or "gone", which a
// fresh read tells apart. The earlier form decided 409-vs-200 from the
// row resolveReminderForWrite read before the UPDATE, and a fire or
// re-arm landing in between made it answer for a state that had
// already stopped holding. The variable is still called `reminder`
// above only because the resolver's permission checks need it.
current, err := s.store.GetReminder(workspaceID, reminder.ID)
if err != nil {
writeInternalError(w, err)
return
}
if current == nil {
writeError(w, http.StatusNotFound, "not_found", "Reminder not found")
return
}
writeError(w, http.StatusConflict, "reminder_not_fired",
"This reminder has not fired yet, so there is nothing to acknowledge.")
return
}
writeJSON(w, http.StatusOK, acked)
}
// handleDeleteReminder disarms a reminder by removing it.
// DELETE /api/v1/workspaces/{slug}/reminders/{reminderID}
func (s *Server) handleDeleteReminder(w http.ResponseWriter, r *http.Request) {
workspaceID, ok := s.getWorkspaceID(w, r)
if !ok {
return
}
reminder, _ := s.resolveReminderForWrite(w, r, workspaceID)
if reminder == nil {
return
}
removed, err := s.store.DeleteReminder(workspaceID, reminder.ID)
if err != nil {
writeInternalError(w, err)
return
}
if !removed {
writeError(w, http.StatusNotFound, "not_found", "Reminder not found")
return
}
writeJSON(w, http.StatusOK, map[string]any{"id": reminder.ID, "deleted": true})
}
// resolveVisibleItem resolves {itemSlug} and enforces read visibility,
// writing the error response itself. Returns nil when the caller should stop.
func (s *Server) resolveVisibleItem(w http.ResponseWriter, r *http.Request, workspaceID string) *models.Item {
itemSlug := chi.URLParam(r, "itemSlug")
item, err := s.store.ResolveItem(workspaceID, itemSlug)
if err != nil {
writeInternalError(w, err)
return nil
}
if item == nil {
s.writeItemResolveError(w, r, workspaceID, itemSlug)
return nil
}
if !s.requireItemVisible(w, r, workspaceID, item) {
return nil
}
return item
}
// resolveReminderForWrite resolves {reminderID} and enforces edit permission
// on the ITEM the reminder hangs off.
//
// PERMISSION IS THE ITEM'S, not the reminder's, and the reminder has no
// separate owner on purpose: a reminder is a property of an item's schedule,
// so anyone who may edit the item may schedule work on it, and anyone who may
// not must not be able to arm one and make the workspace notify about it.
//
// The visibility check runs BEFORE the edit check for the usual reason: an
// edit-permission failure on an item the caller cannot see would confirm the
// reminder exists, which is the existence-oracle shape a sibling handler
// family already had to be fixed for.
// An ARCHIVED item's reminder answers 409 "archived" here, exactly as an edit
// to the item itself would (writeItemResolveError), rather than a 404 that
// says nothing about why — the reminder exists, its item exists, and the
// remedy is the item's restore.
//
// THE ARCHIVED CHECK IS A DOOR COURTESY, NOT A STORE INVARIANT, and an archive
// that lands between this check and the store's UPDATE lets the verb through
// (codex round 17, accepted). That is the posture of every item mutation in
// this API — UpdateItem's own UPDATE is `WHERE id = ?` with no liveness
// clause — and the outcome is benign: an ack, re-arm or delete on a reminder
// whose item was archived a moment ago leaves rows the scan and the pending
// surface already exclude, and RestoreItem brings back whatever state they
// hold. Asserting liveness inside the store's WHERE would make AckReminder's
// no-match ambiguous again ("not fired" vs "archived"), which round 12 removed
// on purpose; the courtesy stays at the door. See handleListItemReminders for the read
// side of the same posture. The include-deleted load is what lets the
// visibility check run first, so a guest who could not see the item learns
// nothing from the difference between 404 and 409.
func (s *Server) resolveReminderForWrite(w http.ResponseWriter, r *http.Request, workspaceID string) (*models.Reminder, *models.Item) {
id := chi.URLParam(r, "reminderID")
reminder, err := s.store.GetReminder(workspaceID, id)
if err != nil {
writeInternalError(w, err)
return nil, nil
}
if reminder == nil {
writeError(w, http.StatusNotFound, "not_found", "Reminder not found")
return nil, nil
}
item, err := s.store.GetItemIncludeDeleted(reminder.ItemID)
if err != nil {
writeInternalError(w, err)
return nil, nil
}
if item == nil {
writeError(w, http.StatusNotFound, "not_found", "Reminder not found")
return nil, nil
}
if !s.requireItemVisible(w, r, workspaceID, item) {
return nil, nil
}
if item.DeletedAt != nil {
// BY SLUG, not Ref (codex round 17): Ref is derived and EMPTY for a
// legacy item with no item_number or collection prefix, and
// writeItemResolveError re-resolves by what it is handed — an empty
// ref matches nothing and falls through to a 404 that says nothing
// about why. The slug is the stable identity every item has.
s.writeItemResolveError(w, r, workspaceID, item.Slug)
return nil, nil
}
if !s.requireEditPermission(w, r, workspaceID, item.ID, item.CollectionID) {
return nil, nil
}
return reminder, item
}
// Bounds for the pending-reminder read (IDEA-2641, codex round 4).
const (
// pendingReminderWindow is how many pending reminders the dashboard shows.
pendingReminderWindow = 50
// pendingReminderMaxScan bounds how many rows may be READ to fill that
// window. The two differ because one filter cannot run in SQL: terminality
// is defined by a collection's schema, so a workspace where most reminders
// sit on completed items would otherwise need an unbounded scan to fill a
// bounded window.
//
// THE RECEIPT: 10x the window, so the common shape — a handful of finished
// items among live ones — fills the window on the first page, and the
// pathological shape (hundreds of completed items with reminders, which the
// documented "arm it to fire after the work is done" pattern actually
// produces) still terminates in a fixed number of indexed reads. When the
// scan bound stops us, the result is reported as truncated, which is
// honest: there may be more, and we did not look further.
pendingReminderMaxScan = 500
)
// collectPendingReminders fills the pending-reminder window, paging past
// reminders whose items are in a terminal state.
//
// Terminal-item reminders are FILTERED, never acked — see the ack handler for
// why. That filtering happens here rather than in SQL because terminality is
// schema-defined, and it is the reason this function exists at all: without
// paging, a bounded query plus an above-the-query filter is a starvation, and
// that is precisely the defect this replaced.
func (s *Server) collectPendingReminders(workspaceID string, scope store.PendingReminderScope, ctxMap map[string]doneContext) ([]*models.PendingReminder, bool, error) {
return s.collectPendingRemindersBounded(workspaceID, scope, ctxMap, pendingReminderWindow, pendingReminderMaxScan)
}
// collectPendingRemindersBounded is the paging loop with its bounds injected,
// so a test can drive the case the production constants make impractical to
// build: a window that fills PART WAY through a page. Reaching that with a
// window of 50 needs ~75 rows in a specific terminal pattern; with a window of
// 3 it is four rows. Same split, and the same reason, as the store's arbiter
// and isolation seams.
func (s *Server) collectPendingRemindersBounded(workspaceID string, scope store.PendingReminderScope, ctxMap map[string]doneContext, window, maxScan int) ([]*models.PendingReminder, bool, error) {
var out []*models.PendingReminder
scanned := 0
for len(out) < window && scanned < maxScan {
page, more, err := s.store.ListPendingReminders(workspaceID, scope, window, scanned)
if err != nil {
return nil, false, err
}
if len(page) == 0 {
// Source exhausted with room to spare: nothing was truncated.
return out, false, nil
}
scanned += len(page)
filledMidPage := false
for i, pr := range page {
if isItemDone(pr.ItemFields, pr.CollectionID, ctxMap) {
continue
}
out = append(out, pr)
if len(out) == window {
// Rows AFTER this one in the page are pending reminders the
// caller is not being shown, so the set is truncated even if
// this was the last page (codex round 11). Reporting `more`
// alone said "you have seen everything" while unread rows sat
// in the very page we stopped reading.
filledMidPage = i < len(page)-1
break
}
}
if filledMidPage {
return out, true, nil
}
if !more {
// We read to the end of the set. Whatever we have is all there is,
// even if it is short of the window.
return out, false, nil
}
}
// Either the window filled or the scan bound stopped us; in both cases
// rows remain unread, so say so.
return out, true, nil
}
+903
View File
@@ -0,0 +1,903 @@
package server
import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"github.com/PerpetualSoftware/pad/internal/models"
"github.com/PerpetualSoftware/pad/internal/store"
)
// Reminder HTTP + poll-surface tests (IDEA-2641).
const (
pastInstant = "2020-01-01T00:00:00Z"
futureInstant = "2099-01-01T00:00:00Z"
)
func armViaAPI(t *testing.T, srv *Server, wsSlug string, item models.Item, at string) models.Reminder {
t.Helper()
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+wsSlug+"/items/"+item.Slug+"/reminders",
map[string]string{"remind_at": at})
if rr.Code != http.StatusCreated {
t.Fatalf("arm reminder: expected 201, got %d: %s", rr.Code, rr.Body.String())
}
var r models.Reminder
parseJSON(t, rr, &r)
return r
}
// TestArmRejectsABareDate. The `date` schema type accepts YYYY-MM-DD, so a
// caller will try it here. It is refused rather than assumed to mean midnight:
// a bare date names a 24-hour span, and picking an hour inside it would be the
// server inventing a time the user did not choose and then firing at it.
func TestArmRejectsABareDate(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Ship it", "fields": `{"status":"open"}`,
})
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/reminders",
map[string]string{"remind_at": "2026-08-01"})
if rr.Code != http.StatusBadRequest {
t.Fatalf("expected 400 for a bare date, got %d: %s", rr.Code, rr.Body.String())
}
// The message has to name the accepted form, or the refusal costs the
// caller a guess instead of a round trip.
if !strings.Contains(rr.Body.String(), "RFC3339") {
t.Errorf("refusal does not name the accepted form: %s", rr.Body.String())
}
}
// TestAckBeforeFireIsAConflict. "Nothing happened" is the same response for an
// armed reminder (too early) and an already-acked one (already done), and
// those need opposite reactions from the caller — so they get different codes.
func TestAckBeforeFireIsAConflict(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Ship it", "fields": `{"status":"open"}`,
})
r := armViaAPI(t, srv, slug, item, futureInstant)
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/reminders/"+r.ID+"/ack", nil)
if rr.Code != http.StatusConflict {
t.Fatalf("expected 409 acking an armed reminder, got %d: %s", rr.Code, rr.Body.String())
}
}
// TestFiredReminderReachesTheSuggestionSurface — the mandatory poll path. On
// an instance with no webhook dispatcher (the common self-hosted shape) the
// outbox acks the event instantly, so this list is the entire delivery.
func TestFiredReminderReachesTheSuggestionSurface(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
// COMPETING CANDIDATES ARE THE POINT of this fixture. With only the
// reminder's own item in the workspace, the reminder lands at index 0
// whether it is prepended or appended — the assertion below would hold
// against an implementation that appends, and the ordering claim would be
// untested. Three in-progress high-priority tasks fill the cap, so a
// reminder that is merely appended ends up fourth and invisible.
for _, title := range []string{"Busy one", "Busy two", "Busy three"} {
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": title, "fields": `{"status":"in-progress","priority":"high"}`,
})
}
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Revisit the schema", "fields": `{"status":"open","priority":"low"}`,
})
armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
resp := getDashboard(t, srv, slug)
if len(resp.PendingReminders) != 1 {
t.Fatalf("expected 1 pending reminder, got %d", len(resp.PendingReminders))
}
if resp.PendingReminders[0].ItemRef != item.Ref {
t.Errorf("pending reminder names %q, want %q", resp.PendingReminders[0].ItemRef, item.Ref)
}
if len(resp.SuggestedNext) == 0 || resp.SuggestedNext[0].ItemTitle != "Revisit the schema" {
t.Fatalf("a fired reminder must lead suggested_next; got %+v", resp.SuggestedNext)
}
if !strings.HasPrefix(resp.SuggestedNext[0].Reason, "REMINDER due") {
t.Errorf("suggestion reason = %q, want it to say a reminder fired", resp.SuggestedNext[0].Reason)
}
}
// TestFiredReminderOnADoneItemIsFilteredNotAcked is the lead's pin, and the
// two halves are the whole point: ABSENT from the surface, PRESENT in the
// table. Acking on terminal status would couple every status write to reminder
// state and would consume a reminder armed to fire after the work was done.
//
// Asserting only the absence would pass against an implementation that acked
// the row, which is the behaviour this exists to forbid.
func TestFiredReminderOnADoneItemIsFilteredNotAcked(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Finished work", "fields": `{"status":"open"}`,
})
r := armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
// Sanity: it IS on the surface while the item is open. Without this leg
// the test would pass on a build where reminders never surface at all.
if len(getDashboard(t, srv, slug).PendingReminders) != 1 {
t.Fatal("the reminder is not on the surface even before the item is done")
}
rr := doRequest(srv, "PATCH", "/api/v1/workspaces/"+slug+"/items/"+item.Slug,
map[string]interface{}{"fields": `{"status":"done"}`})
if rr.Code != http.StatusOK {
t.Fatalf("mark done: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
resp := getDashboard(t, srv, slug)
if len(resp.PendingReminders) != 0 {
t.Errorf("a reminder on a completed item must not be shown, got %d", len(resp.PendingReminders))
}
for _, sug := range resp.SuggestedNext {
if sug.ItemTitle == "Finished work" {
t.Error("a completed item is suggested via its reminder")
}
}
// PRESENT IN THE TABLE, and still unacknowledged — the user's intent is
// preserved and no status write touched the row.
stored, err := srv.store.GetReminder(item.WorkspaceID, r.ID)
if err != nil {
t.Fatalf("GetReminder: %v", err)
}
if stored == nil {
t.Fatal("the reminder row was removed; filtering must not delete")
}
if stored.AckedAt != nil {
t.Error("the reminder was ACKED by the status change; only an explicit ack may do that")
}
if !stored.PendingAck() {
t.Error("the reminder should still be fired-and-unacknowledged in the table")
}
}
// TestRearmReturnsAReminderToTheArmedSet drives the whole loop through HTTP:
// fired, acknowledged, re-armed, fires again.
func TestRearmReturnsAReminderToTheArmedSet(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Ship it", "fields": `{"status":"open"}`,
})
r := armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/reminders/"+r.ID+"/ack", nil)
if rr.Code != http.StatusOK {
t.Fatalf("ack: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
if len(getDashboard(t, srv, slug).PendingReminders) != 0 {
t.Fatal("an acknowledged reminder must leave the surface")
}
// Re-arm into the past and tick again: it must come back.
rr = doRequest(srv, "PATCH", "/api/v1/workspaces/"+slug+"/reminders/"+r.ID,
map[string]string{"remind_at": pastInstant})
if rr.Code != http.StatusOK {
t.Fatalf("re-arm: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
srv.runReminderTick()
if len(getDashboard(t, srv, slug).PendingReminders) != 1 {
t.Error("a re-armed reminder must fire and reappear on the surface")
}
}
// TestTickIsQuietWhenNothingIsDue is the negative control for the tick itself.
// Without it, a tick that fired EVERYTHING would satisfy every test above.
func TestTickIsQuietWhenNothingIsDue(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Ship it", "fields": `{"status":"open"}`,
})
armViaAPI(t, srv, slug, item, futureInstant)
srv.runReminderTick()
if got := len(getDashboard(t, srv, slug).PendingReminders); got != 0 {
t.Errorf("a tick fired %d reminder(s) whose instant has not arrived", got)
}
}
// TestPendingRemindersRespectItemGrants — codex round 1, P1.
//
// Every other dashboard section reads `allItems`, which the store already
// scoped to the caller's collections AND their granted item ids. The pending-
// reminder list is a direct workspace-wide query, so it inherited none of that:
// a guest holding a grant on ONE item could read the refs and titles of every
// other item in the collection through its reminders — an item-level leak
// wearing a notification's clothes.
//
// The two items live in the SAME collection deliberately. A collection-level
// filter is already applied, so putting them in different collections would
// make the test pass against the unfixed code and prove nothing.
//
// MUTANT: remove the isItemVisibleToGuest call and the guest sees both.
func TestPendingRemindersRespectItemGrants(t *testing.T) {
t.Parallel()
srv := testServer(t)
owner := mustUser(t, srv, "reminder-owner@example.com", "reminderowner", "")
ws := mustWorkspace(t, srv, "Reminders", owner.ID)
coll := mustCollection(t, srv, ws.ID, "Tasks")
granted := mustItem(t, srv, ws.ID, coll.ID, "Granted item")
secret := mustItem(t, srv, ws.ID, coll.ID, "Not for the guest")
guest := mustUser(t, srv, "reminder-guest@example.com", "reminderguest", "")
if _, err := srv.store.CreateItemGrant(ws.ID, granted.ID, guest.ID, "edit", owner.ID); err != nil {
t.Fatalf("CreateItemGrant: %v", err)
}
for _, it := range []*models.Item{granted, secret} {
if _, err := srv.store.CreateReminder(ws.ID, it.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
}
srv.runReminderTick()
req := httptest.NewRequest("GET", "/api/v1/workspaces/"+ws.Slug+"/dashboard", nil)
ctx := WithCurrentUser(req.Context(), guest)
ctx = contextWithWorkspaceRoleForTest(ctx, "guest")
ctx = contextWithResolvedWorkspaceIDForTest(ctx, ws.ID)
req = req.WithContext(ctx)
resp, err := srv.buildDashboardResponse(ws.ID, req)
if err != nil {
t.Fatalf("buildDashboardResponse: %v", err)
}
// The guest must see their own item's reminder — without this leg a build
// that filtered EVERYTHING would pass the leak assertion below.
var sawGranted bool
for _, pr := range resp.PendingReminders {
if pr.ItemTitle == "Not for the guest" {
t.Error("a guest read another item's reminder; the pending list is not item-filtered")
}
if pr.ItemTitle == "Granted item" {
sawGranted = true
}
}
if !sawGranted {
t.Error("the guest cannot see the reminder on the item they were granted")
}
for _, sug := range resp.SuggestedNext {
if sug.ItemTitle == "Not for the guest" {
t.Error("the leak reaches suggested_next as well")
}
}
}
// TestFiredReminderSuggestionCarriesItsID — codex round 1, P2.
//
// The docs tell an agent to acknowledge what it sees in next/ready, and the
// payload did not carry the handle: a stateless poller could read the reminder
// and had no way to retire it, so it would be shown the same item forever.
//
// MUTANT: drop the ReminderID assignment and this fails while every other
// reminder test stays green — the id is invisible to all of them.
func TestFiredReminderSuggestionCarriesItsID(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Revisit the schema", "fields": `{"status":"open"}`,
})
armed := armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
resp := getDashboard(t, srv, slug)
if len(resp.SuggestedNext) == 0 {
t.Fatal("no suggestions")
}
if resp.SuggestedNext[0].ReminderID != armed.ID {
t.Fatalf("suggestion carries reminder_id %q, want %q — an agent reading this surface cannot ack",
resp.SuggestedNext[0].ReminderID, armed.ID)
}
// And the id it carries actually works, rather than merely being present:
// a wrong-but-populated id would satisfy an equality check against itself.
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/reminders/"+resp.SuggestedNext[0].ReminderID+"/ack", nil)
if rr.Code != http.StatusOK {
t.Fatalf("acking the id from the suggestion failed: %d %s", rr.Code, rr.Body.String())
}
if len(getDashboard(t, srv, slug).PendingReminders) != 0 {
t.Error("the reminder survived an ack using the id the surface handed out")
}
}
// TestReminderSuggestionsAreCapped — codex round 3, tightened in round 11.
//
// suggested_next is a recommendation list of THREE and every consumer is
// written against that. Round 3 capped the reminders at five and prepended
// them past the list's own cap, which made the surface return up to eight —
// caught in round 11, along with the fact that it falsified a decision
// recorded in BootstrapDashboard (no suggested_next_overflow_count, BECAUSE
// this list is capped at three upstream).
//
// The fixture needs MORE reminders than the cap, which is the leg the first
// version of the prepend test lacked: with one reminder, capped and uncapped
// are the same list. That is the second time a single-item fixture hid an
// ordering-or-count property in this file.
//
// MUTANT: remove the cap and eight suggestions come back.
func TestReminderSuggestionsAreCapped(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
for i := 0; i < 8; i++ {
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": fmt.Sprintf("Task %d", i), "fields": `{"status":"open"}`,
})
armViaAPI(t, srv, slug, item, pastInstant)
}
srv.runReminderTick()
resp := getDashboard(t, srv, slug)
var reminderSuggestions int
for _, sug := range resp.SuggestedNext {
if sug.ReminderID != "" {
reminderSuggestions++
}
}
if len(resp.SuggestedNext) != 3 {
t.Errorf("suggested_next carries %d entries, want the established cap of 3", len(resp.SuggestedNext))
}
if reminderSuggestions != 3 {
t.Errorf("suggested_next carries %d reminder entries, want 3 — reminders lead and the list is trimmed", reminderSuggestions)
}
// All eight stay addressable in the list that is not a recommendation.
if len(resp.PendingReminders) != 8 {
t.Errorf("pending_reminders holds %d, want all 8 — the cap is on the recommendation, not the data", len(resp.PendingReminders))
}
}
// TestOrdinarySuggestionsCarryNoReminderID is the negative control: the field
// is omitempty and must stay empty on a plain task suggestion, or a consumer
// switching on its presence would try to ack a task.
func TestOrdinarySuggestionsCarryNoReminderID(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Just a task", "fields": `{"status":"in-progress","priority":"high"}`,
})
resp := getDashboard(t, srv, slug)
if len(resp.SuggestedNext) == 0 {
t.Fatal("no suggestions")
}
for _, sug := range resp.SuggestedNext {
if sug.ReminderID != "" {
t.Errorf("a plain task suggestion carries reminder_id %q", sug.ReminderID)
}
}
}
// TestStartReminderTickFiresOnATick binds the LOOP to the work (CONVE-19).
//
// Every other test here calls runReminderTick directly, which vouches for the
// component and says nothing about whether anything ever calls it. That is the
// exact gap this convention names, and it is the one I keep falling into: a
// tick that is never started is indistinguishable, from those tests, from one
// that is.
//
// Driven through the injectable tick channel so the assertion is pinned to a
// SPECIFIC pass rather than racing a free-running 30-second ticker.
//
// MUTANT: make StartReminderTick's goroutine ignore its channel (or drop the
// runReminderTick call from the select) and this fails while every direct-call
// test stays green.
func TestStartReminderTickFiresOnATick(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Wake me", "fields": `{"status":"open"}`,
})
armViaAPI(t, srv, slug, item, pastInstant)
ticks := make(chan time.Time, 1)
srv.SetReminderTickChannel(ticks)
srv.StartReminderTick()
defer srv.stopReminderTick()
ticks <- time.Now()
// Poll rather than sleep a fixed interval: the pass is asynchronous, and a
// fixed sleep is either flaky or slow. Bounded so a tick that never runs
// fails rather than hanging the suite.
deadline := time.Now().Add(5 * time.Second)
for {
if len(getDashboard(t, srv, slug).PendingReminders) == 1 {
return
}
if time.Now().After(deadline) {
t.Fatal("the started tick never fired an armed reminder — the loop is not bound to the work")
}
time.Sleep(20 * time.Millisecond)
}
}
// TestStartReminderTickIsIdempotent: a second Start must not spawn a second
// loop, or Stop() would leave one running and the BUG-842 drain invariant
// would be false for this sweeper.
func TestStartReminderTickIsIdempotent(t *testing.T) {
t.Parallel()
srv := testServer(t)
ticks := make(chan time.Time, 1)
srv.SetReminderTickChannel(ticks)
srv.StartReminderTick()
srv.StartReminderTick()
srv.stopReminderTick()
// A second stop must be safe too — Stop() runs unconditionally.
srv.stopReminderTick()
}
// TestPendingRemindersAreNotStarvedByCompletedItems — codex round 4, P1.
//
// This is the defect the ROUND-3 fix introduced, and it is the same shape as
// the round-1 one it had just removed from the fire path: a bounded window
// whose rows are discarded ABOVE the bound hides everything behind them
// forever, with no continuation to reach it. Bounding is only safe when the
// discarding happens before the bound.
//
// The fixture puts more terminal-item reminders than the window (50) AHEAD of
// the live one, ordered by fire time. Fewer than the window would fill from the
// first page and prove nothing.
//
// MUTANT: drop the paging loop back to a single ListPendingReminders call and
// the live reminder never appears.
func TestPendingRemindersAreNotStarvedByCompletedItems(t *testing.T) {
t.Parallel()
srv := testServer(t)
owner := mustUser(t, srv, "starve-owner@example.com", "starveowner", "")
ws := mustWorkspace(t, srv, "Starved", owner.ID)
coll := mustCollection(t, srv, ws.ID, "Tasks")
// Built through the store rather than the API: sixty items plus sixty
// status writes trips the write rate limiter, and a 429 mid-fixture is a
// test that fails for a reason unrelated to what it measures.
for i := 0; i < 60; i++ {
done, err := srv.store.CreateItem(ws.ID, coll.ID, models.ItemCreate{
Title: fmt.Sprintf("Finished %d", i),
Fields: `{"status":"done"}`,
})
if err != nil {
t.Fatalf("CreateItem: %v", err)
}
if _, err := srv.store.CreateReminder(ws.ID, done.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
}
live, err := srv.store.CreateItem(ws.ID, coll.ID, models.ItemCreate{
Title: "Still open", Fields: `{"status":"open"}`,
})
if err != nil {
t.Fatalf("CreateItem: %v", err)
}
if _, err := srv.store.CreateReminder(ws.ID, live.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
srv.runReminderTick()
req := httptest.NewRequest("GET", "/api/v1/workspaces/"+ws.Slug+"/dashboard", nil)
req = req.WithContext(contextWithResolvedWorkspaceIDForTest(WithCurrentUser(req.Context(), owner), ws.ID))
resp, err := srv.buildDashboardResponse(ws.ID, req)
if err != nil {
t.Fatalf("buildDashboardResponse: %v", err)
}
var found bool
for _, pr := range resp.PendingReminders {
if pr.ItemTitle == "Still open" {
found = true
}
if strings.HasPrefix(pr.ItemTitle, "Finished ") {
t.Fatalf("a completed item's reminder reached the surface: %s", pr.ItemTitle)
}
}
if !found {
t.Errorf("the live reminder was starved behind 60 completed ones (%d shown)", len(resp.PendingReminders))
}
}
// TestPendingReminderScopeIsAppliedInTheQuery — codex round 4, the other half.
//
// A guest's invisible rows must not consume the window either. The granted
// item is armed LAST, so it sorts after 60 rows the guest may not see: if
// scoping ran above the bound, those 60 would fill the window and the guest's
// own reminder would be unreachable.
//
// MUTANT: drop the scope clause from the SQL and the guest sees nothing (or
// sees other people's items, which the round-1 test catches).
func TestPendingReminderScopeIsAppliedInTheQuery(t *testing.T) {
t.Parallel()
srv := testServer(t)
owner := mustUser(t, srv, "scope-owner@example.com", "scopeowner", "")
ws := mustWorkspace(t, srv, "Scoped", owner.ID)
coll := mustCollection(t, srv, ws.ID, "Tasks")
for i := 0; i < 60; i++ {
other := mustItem(t, srv, ws.ID, coll.ID, fmt.Sprintf("Not yours %d", i))
if _, err := srv.store.CreateReminder(ws.ID, other.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
}
mine := mustItem(t, srv, ws.ID, coll.ID, "Yours")
if _, err := srv.store.CreateReminder(ws.ID, mine.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
srv.runReminderTick()
guest := mustUser(t, srv, "scope-guest@example.com", "scopeguest", "")
if _, err := srv.store.CreateItemGrant(ws.ID, mine.ID, guest.ID, "edit", owner.ID); err != nil {
t.Fatalf("CreateItemGrant: %v", err)
}
req := httptest.NewRequest("GET", "/api/v1/workspaces/"+ws.Slug+"/dashboard", nil)
ctx := WithCurrentUser(req.Context(), guest)
ctx = contextWithWorkspaceRoleForTest(ctx, "guest")
ctx = contextWithResolvedWorkspaceIDForTest(ctx, ws.ID)
resp, err := srv.buildDashboardResponse(ws.ID, req.WithContext(ctx))
if err != nil {
t.Fatalf("buildDashboardResponse: %v", err)
}
if len(resp.PendingReminders) != 1 || resp.PendingReminders[0].ItemTitle != "Yours" {
var titles []string
for _, pr := range resp.PendingReminders {
titles = append(titles, pr.ItemTitle)
}
t.Fatalf("guest saw %v, want exactly the granted item's reminder", titles)
}
// And the truncation flag must be FALSE: the guest's own set fits, and
// telling them to page through rows they can never see would be a lie in
// the shape of a hint.
if resp.PendingRemindersTruncated {
t.Error("a guest whose whole visible set fits was told there is more")
}
}
// TestBootstrapCapsPendingReminders — codex round 11.
//
// BootstrapDashboard embeds *DashboardResponse, so every new field flows into
// the boot payload automatically — including a reminder window of up to 50,
// which is the budget PLAN-1410 spent an entire unit trimming. It needs a cap
// where suggested_next does not, because suggested_next is capped upstream at
// three and a bootstrap cap could never fire.
//
// MUTANT: remove the cap block and all eight arrive.
func TestBootstrapCapsPendingReminders(t *testing.T) {
t.Parallel()
srv := testServer(t)
owner := mustUser(t, srv, "boot-cap@example.com", "bootcap", "")
ws := mustWorkspace(t, srv, "Boot Cap", owner.ID)
coll := mustCollection(t, srv, ws.ID, "Tasks")
for i := 0; i < 8; i++ {
item := mustItem(t, srv, ws.ID, coll.ID, fmt.Sprintf("Task %d", i))
if _, err := srv.store.CreateReminder(ws.ID, item.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
}
srv.runReminderTick()
req := httptest.NewRequest("GET", "/api/v1/workspaces/"+ws.Slug+"/dashboard", nil)
req = req.WithContext(contextWithResolvedWorkspaceIDForTest(WithCurrentUser(req.Context(), owner), ws.ID))
full, err := srv.buildDashboardResponse(ws.ID, req)
if err != nil {
t.Fatalf("buildDashboardResponse: %v", err)
}
if len(full.PendingReminders) != 8 {
t.Fatalf("setup: dashboard has %d pending reminders, want 8", len(full.PendingReminders))
}
capped := capBootstrapDashboard(full)
if len(capped.PendingReminders) != 5 {
t.Errorf("bootstrap embedded %d reminders, want the cap of 5", len(capped.PendingReminders))
}
if capped.PendingRemindersOverflowCount != 3 {
t.Errorf("overflow count = %d, want 3", capped.PendingRemindersOverflowCount)
}
// The FULL dashboard must be untouched — capBootstrapDashboard copies, and
// a cap that mutated its input would silently shrink `pad project
// dashboard` for everyone.
if len(full.PendingReminders) != 8 {
t.Error("capping the bootstrap projection mutated the dashboard it was built from")
}
}
// TestSuggestedNextKeepsItsCapWithReminders — codex round 11.
//
// Prepending up to five reminders past a list capped at three returned eight
// entries, against consumers written for three — and it falsified the comment
// in BootstrapDashboard that justifies having no suggested_next overflow
// count, which names raising this cap as the moment to add one.
//
// MUTANT: remove the trim and eight come back.
func TestSuggestedNextKeepsItsCapWithReminders(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
for i := 0; i < 3; i++ {
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": fmt.Sprintf("Busy %d", i), "fields": `{"status":"in-progress","priority":"high"}`,
})
}
for i := 0; i < 5; i++ {
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": fmt.Sprintf("Remind %d", i), "fields": `{"status":"open"}`,
})
armViaAPI(t, srv, slug, item, pastInstant)
}
srv.runReminderTick()
resp := getDashboard(t, srv, slug)
if len(resp.SuggestedNext) != 3 {
t.Errorf("suggested_next returned %d entries, want the established cap of 3", len(resp.SuggestedNext))
}
// And the trim keeps the REMINDERS, which lead — trimming the front would
// satisfy the count and defeat the feature.
for i, sug := range resp.SuggestedNext {
if sug.ReminderID == "" {
t.Errorf("entry %d is not a reminder; the trim dropped the leading entries", i)
}
}
// All five stay addressable where they are not a recommendation.
if len(resp.PendingReminders) != 5 {
t.Errorf("pending_reminders holds %d, want all 5", len(resp.PendingReminders))
}
}
// TestSuggestedNextSurvivesWithNoTaskCandidates is the leg that catches the
// bug my own first fix introduced: `limit` is reassigned to len(candidates),
// so trimming with it would truncate to ZERO on a workspace whose only
// entries are reminders — precisely the case the surface exists for.
func TestSuggestedNextSurvivesWithNoTaskCandidates(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Only a reminder", "fields": `{"status":"done"}`,
})
armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
resp := getDashboard(t, srv, slug)
// The item is done, so it filters out of BOTH surfaces — which makes this
// the wrong fixture for the property. Re-open it and re-read.
rr := doRequest(srv, "PATCH", "/api/v1/workspaces/"+slug+"/items/"+item.Slug,
map[string]interface{}{"fields": `{"status":"open"}`})
if rr.Code != http.StatusOK {
t.Fatalf("reopen: %d", rr.Code)
}
resp = getDashboard(t, srv, slug)
if len(resp.SuggestedNext) != 1 {
t.Fatalf("a workspace whose only entry is a reminder returned %d suggestions, want 1", len(resp.SuggestedNext))
}
if resp.SuggestedNext[0].ReminderID == "" {
t.Error("the single suggestion is not the reminder")
}
}
// TestTruncationIsReportedWhenTheWindowFillsMidPage — codex round 11.
//
// The collector reported truncation from the store's `more` flag alone, which
// answers "is there another PAGE" and not "did I read all of THIS one". When
// the window filled part way through the final page, the rows behind the fill
// point were pending reminders the caller was not shown — and it was told it
// had seen everything.
//
// Fixture: window of 3. Page one holds two live reminders and two on completed
// items (so it contributes 2 and exhausts its page); page two holds two live
// ones, of which only the first is needed. The second is unread, in the last
// page, and truncation must say so.
//
// MUTANT: drop the filledMidPage branch and this reports false.
func TestTruncationIsReportedWhenTheWindowFillsMidPage(t *testing.T) {
t.Parallel()
srv := testServer(t)
owner := mustUser(t, srv, "midpage@example.com", "midpage", "")
ws := mustWorkspace(t, srv, "Mid Page", owner.ID)
coll := mustCollection(t, srv, ws.ID, "Tasks")
// Order is by fired_at, and the tick stamps them all in one pass, so the
// tie-break is the reminder id — which means the page composition is not
// something this test can pin by creation order. What it CAN pin is the
// counts: 4 live and 2 done, a window of 3, so the window fills with rows
// still unread whichever way the ids sort.
mk := func(title, status string) {
item, err := srv.store.CreateItem(ws.ID, coll.ID, models.ItemCreate{Title: title, Fields: `{"status":"` + status + `"}`})
if err != nil {
t.Fatalf("CreateItem: %v", err)
}
if _, err := srv.store.CreateReminder(ws.ID, item.ID, pastInstant); err != nil {
t.Fatalf("CreateReminder: %v", err)
}
}
for i := 0; i < 4; i++ {
mk(fmt.Sprintf("Live %d", i), "open")
}
for i := 0; i < 2; i++ {
mk(fmt.Sprintf("Done %d", i), "done")
}
srv.runReminderTick()
req := httptest.NewRequest("GET", "/api/v1/workspaces/"+ws.Slug+"/dashboard", nil)
req = req.WithContext(contextWithResolvedWorkspaceIDForTest(WithCurrentUser(req.Context(), owner), ws.ID))
if _, err := srv.buildDashboardResponse(ws.ID, req); err != nil {
t.Fatalf("buildDashboardResponse: %v", err)
}
colls, err := srv.store.ListCollections(ws.ID)
if err != nil {
t.Fatalf("ListCollections: %v", err)
}
ctxMap := buildDoneContextMap(colls)
out, truncated, err := srv.collectPendingRemindersBounded(ws.ID, store.PendingReminderScope{}, ctxMap, 3, 100)
if err != nil {
t.Fatalf("collectPendingRemindersBounded: %v", err)
}
if len(out) != 3 {
t.Fatalf("collected %d, want the window of 3", len(out))
}
if !truncated {
t.Error("four live reminders through a window of three reported nothing more to see")
}
// Control: a window that fits everything must NOT report truncation, or
// the flag is just always true.
out, truncated, err = srv.collectPendingRemindersBounded(ws.ID, store.PendingReminderScope{}, ctxMap, 10, 100)
if err != nil {
t.Fatalf("collectPendingRemindersBounded (wide): %v", err)
}
if len(out) != 4 {
t.Errorf("wide window collected %d live reminders, want 4", len(out))
}
if truncated {
t.Error("a window that fit every live reminder reported truncation")
}
}
// TestArchivedItemRemindersAreReadableAndNotEditable pins the posture codex
// round 16 read as a contradiction: the store keeps an archived item's
// reminders (reminderOwned enforces identity, not liveness), the LIST follows
// handleGetItem and stays readable, and the lifecycle verbs follow every other
// item mutation and answer 409 "archived" — not a bare 404 — until the item is
// restored, when they work again on the same rows. All three legs are
// asserted, because the read alone would pass against a build that deleted
// the rows, and the 409 alone against one that never restored them.
//
// MUTANT: resolving the list live makes the GET a 409; resolving the write
// live makes the ack a 404; cascading the rows on soft-delete makes the
// post-restore ack 404.
func TestArchivedItemRemindersAreReadableAndNotEditable(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Ship it", "fields": `{"status":"open"}`,
})
r := armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
base := "/api/v1/workspaces/" + slug
if rr := doRequest(srv, "DELETE", base+"/items/"+item.Slug, nil); rr.Code != http.StatusOK && rr.Code != http.StatusNoContent {
t.Fatalf("archive item: %d: %s", rr.Code, rr.Body.String())
}
rr := doRequest(srv, "GET", base+"/items/"+item.Slug+"/reminders", nil)
if rr.Code != http.StatusOK {
t.Fatalf("listing an archived item's reminders: expected 200 (read-only, as GET item), got %d: %s", rr.Code, rr.Body.String())
}
var listed struct {
Reminders []models.Reminder `json:"reminders"`
}
parseJSON(t, rr, &listed)
if len(listed.Reminders) != 1 || listed.Reminders[0].ID != r.ID {
t.Errorf("archived item's reminder history: got %+v, want the one fired reminder", listed.Reminders)
}
rr = doRequest(srv, "POST", base+"/reminders/"+r.ID+"/ack", nil)
if rr.Code != http.StatusConflict {
t.Errorf("acking a reminder on an archived item: expected 409 archived, got %d: %s", rr.Code, rr.Body.String())
}
rr = doRequest(srv, "POST", base+"/items/"+item.Slug+"/reminders", map[string]string{"remind_at": futureInstant})
if rr.Code != http.StatusConflict {
t.Errorf("arming a reminder on an archived item: expected 409 archived, got %d: %s", rr.Code, rr.Body.String())
}
if rr := doRequest(srv, "POST", base+"/items/"+item.Slug+"/restore", nil); rr.Code != http.StatusOK {
t.Fatalf("restore item: %d: %s", rr.Code, rr.Body.String())
}
rr = doRequest(srv, "POST", base+"/reminders/"+r.ID+"/ack", nil)
if rr.Code != http.StatusOK {
t.Fatalf("acking after restore: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
var acked models.Reminder
parseJSON(t, rr, &acked)
if acked.AckedAt == nil || acked.FiredAt == nil {
t.Errorf("the restored reminder should be fired and now acked, got fired=%v acked=%v", acked.FiredAt, acked.AckedAt)
}
}
// TestAnItemIsSuggestedOnceWhenItIsBothRemindedAndACandidate — codex round 17.
// An in-progress, high-priority item with a fired reminder qualified for
// suggested_next twice: once from the reminder (with the ack handle) and once
// as an ordinary candidate. One item, one line, the one that carries the id.
//
// MUTANT: dropping the remindedItems filter puts the item in twice.
func TestAnItemIsSuggestedOnceWhenItIsBothRemindedAndACandidate(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Both", "fields": `{"status":"in-progress","priority":"high"}`,
})
armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
resp := getDashboard(t, srv, slug)
var seen int
for _, sg := range resp.SuggestedNext {
if sg.ItemSlug == item.Slug {
seen++
if sg.ReminderID == "" {
t.Errorf("the surviving entry for %q must be the reminder one (carries the ack id); got %+v", item.Slug, sg)
}
}
}
if seen != 1 {
t.Fatalf("item appears %d times in suggested_next, want exactly 1: %+v", seen, resp.SuggestedNext)
}
}
// TestArchivedLegacyItemStillAnswers409 — codex round 17. A legacy item with no
// item_number has an empty derived Ref; re-resolving by that empty ref inside
// writeItemResolveError matched nothing and turned round 16's 409 into a 404.
// The door now hands over the slug, which every item has.
//
// MUTANT: passing item.Ref again makes this a 404.
func TestArchivedLegacyItemStillAnswers409(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
item := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Old one", "fields": `{"status":"open"}`,
})
r := armViaAPI(t, srv, slug, item, pastInstant)
srv.runReminderTick()
base := "/api/v1/workspaces/" + slug
if rr := doRequest(srv, "DELETE", base+"/items/"+item.Slug, nil); rr.Code != http.StatusOK && rr.Code != http.StatusNoContent {
t.Fatalf("archive item: %d: %s", rr.Code, rr.Body.String())
}
// Make it legacy: migration 006 added item_number to existing rows as NULL.
if _, err := srv.store.DB().Exec(`UPDATE items SET item_number = NULL WHERE id = ?`, item.ID); err != nil {
t.Fatalf("strip item_number: %v", err)
}
rr := doRequest(srv, "POST", base+"/reminders/"+r.ID+"/ack", nil)
if rr.Code != http.StatusConflict {
t.Errorf("acking a reminder on an archived legacy item: expected 409 archived, got %d: %s", rr.Code, rr.Body.String())
}
}
+87
View File
@@ -0,0 +1,87 @@
package server
import (
"strings"
"time"
)
// The one place that decides whether an item is overdue (IDEA-2641).
//
// WHY THIS FILE EXISTS. The rule used to live inline in the dashboard's
// attention loop, and that was the whole implementation — `pad project stale`
// inherited it by filtering the dashboard's attention list, and `pad project
// ready` / `next` did no date handling AT ALL. So a deadline reached the two
// surfaces that report on work and never the surface an agent actually pulls
// from, which is the sharper form of the complaint in GitHub #1010: not "the
// date isn't honored uniformly" but "the date never reaches the recommendation".
//
// Extracting it is what makes "all four surfaces agree" a property of the code
// rather than a thing four call sites happen to do the same way.
//
// WHAT IS DELIBERATELY UNCHANGED: the comparison is still a lexicographic
// string compare against the SERVER'S LOCAL calendar day. That is wrong for a
// multi-timezone deployment and known to be — it is filed as its own item with
// the cloud case stated. Fixing it here would have changed what "overdue"
// means on every existing self-hosted instance inside a change whose subject
// is where the rule LIVES, and a behaviour change smuggled into a refactor is
// the kind nobody reviews.
// overdueDateFields are the field keys that carry a deadline, in report
// priority order.
//
// A LITERAL LIST, not a schema annotation, and that is a decision rather than
// an omission: annotating a FieldDef does not survive an ordinary collection
// edit (the web editor rebuilds each field from an allowlist; CollectionSchema
// has no catch-all), which is exactly why the reminder primitive is a table.
// Convention-by-field-name is the weaker mechanism, but it is the one that
// cannot silently disarm itself.
var overdueDateFields = []string{"due_date", "end_date"}
// overdueToday renders the calendar day deadlines are measured against.
// Server-local, matching the behaviour this preserves.
func overdueToday(now time.Time) string { return now.Format("2006-01-02") }
// itemOverdue reports whether an item has a deadline in the past, and which
// field carried it. Reports at most ONE field per item — the first in
// overdueDateFields order — because an item that is both past its due_date and
// past its end_date is one late item, not two.
//
// Values are compared as strings. ISO-8601 orders lexicographically the same
// way it orders chronologically, so this is correct for `YYYY-MM-DD`, and an
// RFC3339 value (which the `date` field type also admits) sorts after the bare
// day it falls on — so a timestamped value dated TODAY reads as not-yet-late,
// which is the right answer for a due date.
func itemOverdue(fieldsJSON, todayStr string) (field, value string, ok bool) {
if fieldsJSON == "" || fieldsJSON == "{}" {
return "", "", false
}
for _, key := range overdueDateFields {
v := extractFieldValue(fieldsJSON, key)
if v == "" {
continue
}
if v < todayStr {
return key, v, true
}
}
return "", "", false
}
// overdueReason renders the human-facing explanation attached to an overdue
// report ("due date was 2026-08-01"). Shared so the dashboard's attention
// entry and a suggestion's reason cannot drift into two different phrasings of
// the same fact.
func overdueReason(field, value string) string {
return strings.ReplaceAll(field, "_", " ") + " was " + value
}
// overdueReasonOrEmpty renders the reason only when the item is actually
// overdue, so a caller can fill a struct field unconditionally without
// branching. Returning "" for a not-overdue item keeps the empty string
// meaning "no deadline verdict" rather than "a verdict that rendered blank".
func overdueReasonOrEmpty(field, value string, overdue bool) string {
if !overdue {
return ""
}
return overdueReason(field, value)
}
+238
View File
@@ -0,0 +1,238 @@
package server
import (
"net/http"
"strings"
"testing"
)
// The four-surface overdue pins (IDEA-2641).
//
// Before this unit, overdue was computed inline in the dashboard's attention
// loop. `pad project stale` inherited it by filtering that list, and
// `ready` / `next` did no date handling AT ALL — so a deadline reached the two
// surfaces that report on work and never the one an agent pulls from.
//
// Each test below is a LEG: it fails if its own surface drops off the shared
// helper, and it fails for a reason specific to that surface. A single test
// asserting "the helper is called" would pass with three of the four surfaces
// rewired to nothing.
// overdueLowPriorityOrphan is the fixture that discriminates. A LOW-priority,
// open, parentless task is the case the old code handled worst: the orphan
// branch's high/critical gate dropped it, so `next` and `ready` could not have
// surfaced it however they were ranked. Using a high-priority task here would
// have made the ready/next leg pass against the unfixed tree.
const overdueLowPriorityOrphan = `{"status":"open","priority":"low","due_date":"2020-01-01"}`
// TestOverdueReachesDashboardAttention — leg 1.
func TestOverdueReachesDashboardAttention(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Late and unimportant", "fields": overdueLowPriorityOrphan,
})
resp := getDashboard(t, srv, slug)
overdue := filterAttention(resp.Attention, "overdue")
if len(overdue) != 1 {
t.Fatalf("expected 1 overdue attention entry, got %d", len(overdue))
}
if !strings.Contains(overdue[0].Reason, "due date was 2020-01-01") {
t.Errorf("attention reason = %q, want it to name the field and the date", overdue[0].Reason)
}
}
// TestOverdueReachesStale — leg 2. `pad project stale` consumes the dashboard's
// attention list and keeps four types; the CLI-side filter is pinned in
// cmd/pad. What THIS leg pins is the half that lives here: the entry stale
// reads must carry the type it filters on. An entry with the right reason and
// the wrong type would satisfy leg 1 and vanish from stale.
func TestOverdueReachesStale(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Late and unimportant", "fields": overdueLowPriorityOrphan,
})
resp := getDashboard(t, srv, slug)
found := false
for _, a := range resp.Attention {
if a.ItemTitle == "Late and unimportant" {
found = true
if a.Type != "overdue" {
t.Errorf("attention type = %q, want %q — stale filters on this exact string", a.Type, "overdue")
}
}
}
if !found {
t.Fatal("the overdue item is absent from the attention list stale reads")
}
}
// TestOverdueReachesReadyAndNext — leg 3, and the one that would have failed
// before this unit. `ready` and `next` both render dashboard.suggested_next.
func TestOverdueReachesReadyAndNext(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Late and unimportant", "fields": overdueLowPriorityOrphan,
})
resp := getDashboard(t, srv, slug)
if len(resp.SuggestedNext) == 0 {
t.Fatal("suggested_next is empty — an overdue item never reaches ready/next")
}
var found bool
for _, sug := range resp.SuggestedNext {
if sug.ItemTitle == "Late and unimportant" {
found = true
if !strings.HasPrefix(sug.Reason, "OVERDUE — ") {
t.Errorf("suggestion reason = %q, want it to lead with the deadline", sug.Reason)
}
}
}
if !found {
t.Error("a low-priority overdue orphan is missing from suggested_next; the priority gate still stops deadlines")
}
}
// TestOverdueOutranksInProgressWithinTheCap — leg 3's teeth. The list is
// capped at three, so ranking overdue BELOW in-progress does not merely order
// it lower: on a workspace with three things in flight it removes the item
// from the surface entirely. A test that only asserted presence would pass
// against that mutation on an idle workspace and fail in production.
func TestOverdueOutranksInProgressWithinTheCap(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
for _, title := range []string{"Busy one", "Busy two", "Busy three"} {
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": title, "fields": `{"status":"in-progress","priority":"high"}`,
})
}
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Late and unimportant", "fields": overdueLowPriorityOrphan,
})
resp := getDashboard(t, srv, slug)
if len(resp.SuggestedNext) == 0 {
t.Fatal("suggested_next is empty")
}
if resp.SuggestedNext[0].ItemTitle != "Late and unimportant" {
var titles []string
for _, s := range resp.SuggestedNext {
titles = append(titles, s.ItemTitle)
}
t.Errorf("suggested_next leads with %q, want the overdue item; got order %v",
resp.SuggestedNext[0].ItemTitle, titles)
}
}
// TestOverdueIgnoresTerminalItems guards the direction a "surface it
// everywhere" change breaks: a DONE item with a past due date is not late, it
// is finished. This is the assertion that stops the four legs above from being
// satisfied by a helper that simply reports every past date.
func TestOverdueIgnoresTerminalItems(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Finished late", "fields": `{"status":"done","priority":"low","due_date":"2020-01-01"}`,
})
resp := getDashboard(t, srv, slug)
if got := len(filterAttention(resp.Attention, "overdue")); got != 0 {
t.Errorf("a completed item is reported overdue (%d entries)", got)
}
for _, sug := range resp.SuggestedNext {
if sug.ItemTitle == "Finished late" {
t.Error("a completed item is suggested as next work")
}
}
}
// TestFutureDeadlineIsNotOverdue is the negative control for the comparison
// itself. Without it, a helper that reported EVERY item with a date would pass
// every leg above.
func TestFutureDeadlineIsNotOverdue(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Plenty of time", "fields": `{"status":"open","priority":"low","due_date":"2099-12-31"}`,
})
resp := getDashboard(t, srv, slug)
if got := len(filterAttention(resp.Attention, "overdue")); got != 0 {
t.Errorf("a future deadline is reported overdue (%d entries)", got)
}
// And it must not have been let past the priority gate either — the gate
// bypass is keyed on overdue, so a low-priority future item staying out of
// suggested_next is what shows the bypass is conditional rather than
// simply removed.
for _, sug := range resp.SuggestedNext {
if sug.ItemTitle == "Plenty of time" {
t.Error("a low-priority item with a FUTURE deadline was suggested; the gate bypass is unconditional")
}
}
}
// TestSuggestionsCarryTheItemsRealCollection — codex round 5, P2.
//
// The orphan branch admits any collection (its own comment claimed otherwise,
// and the comment was wrong), while the output hardcoded `Collection: "tasks"`
// and the reason said "Open task". So an overdue IDEA was recommended as a
// task in the one surface an agent reads to decide what to work on next.
//
// Pre-existing for high-priority items since BUG-1082; the overdue bypass
// widened it to any overdue item, which is how it surfaced. Fixed by carrying
// the real collection rather than by narrowing the branch — narrowing would
// silently drop the non-task items this has surfaced for a year.
//
// MUTANT: restore the "tasks" literal, or the "Open task" wording, and this
// fails.
func TestSuggestionsCarryTheItemsRealCollection(t *testing.T) {
t.Parallel()
srv := testServer(t)
slug := createWSWithCollections(t, srv)
// A NON-TASK collection whose status vocabulary contains "open", because
// that is the population the defect can actually reach. The first version
// of this test used an idea (status "new") and SKIPPED — the orphan branch
// requires "open" or an active status, so an idea never becomes a
// candidate and the fixture proved nothing. A test that cannot fire is a
// failed reconstruction, not a passing one.
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections", map[string]interface{}{
"name": "Bugs",
"schema": `{"fields":[{"key":"status","type":"select","options":["open","fixing","fixed"],"terminal_options":["fixed"],"default":"open"},{"key":"priority","type":"select","options":["low","high"]},{"key":"due_date","type":"date"}]}`,
})
if rr.Code != http.StatusCreated {
t.Fatalf("create collection: %d %s", rr.Code, rr.Body.String())
}
createItem(t, srv, slug, "bugs", map[string]interface{}{
"title": "Late bug", "fields": `{"status":"open","priority":"low","due_date":"2020-01-01"}`,
})
resp := getDashboard(t, srv, slug)
var found bool
for _, sug := range resp.SuggestedNext {
if sug.ItemTitle != "Late bug" {
continue
}
found = true
if sug.Collection != "bugs" {
t.Errorf("suggestion collection = %q, want %q", sug.Collection, "bugs")
}
if strings.Contains(sug.Reason, "task") {
t.Errorf("a bug is described as a task: %q", sug.Reason)
}
}
if !found {
t.Fatal("the overdue bug never reached suggested_next — the fixture cannot exercise the labelling at all")
}
}
+165
View File
@@ -0,0 +1,165 @@
package server
import (
"log/slog"
"sync"
"time"
)
// The reminder tick: the half of IDEA-2641 that ACTS at a target time.
//
// Everything else in Pad's date handling is reactive — a due_date makes an
// item show up as overdue once somebody asks. Nothing fired on its own, which
// is why GitHub #1010 had to keep "revisit this on the 1st" in an external
// cron. This loop is the engine; the store owns the arbitration and the
// event, and this file owns only the schedule.
//
// It is the sixth instance of a settled shape (outbox drain, token reaper,
// workspace purge, oplog GC, orphan GC, MCP audit sweep): config struct with
// its own mutex and stop channel, tracked by Server.bg so Stop() drains it
// before the DB closes (the BUG-842 invariant), recoverSweeper on the
// goroutine, and an injectable tick channel so a test can pin assertions to
// one specific pass instead of racing a free-running loop.
// defaultReminderTickInterval is how often armed reminders are checked.
//
// THE RECEIPT, because a bare number invites someone to "tune" it: this bounds
// LATENESS, not throughput. A reminder fires at most one interval after its
// instant, so the interval is the promise — 30s means "within half a minute of
// when you asked", which is the resolution a human-set reminder is stated at
// in the first place (nobody arms one for 14:32:07). The cost side is a single
// indexed range scan over a PARTIAL index holding only armed rows, which is
// empty on the overwhelming majority of instances; a tick that finds nothing
// does one query and returns. Going faster buys precision nobody asked for at
// a cost that scales with instance count; going much slower makes "remind me
// at 9" mean something a user would call broken.
const defaultReminderTickInterval = 30 * time.Second
type reminderTickConfig struct {
mu sync.Mutex
interval time.Duration
limit int
stop chan struct{}
running bool
// tick, when non-nil, replaces the interval ticker so a test can drive
// exactly one pass. Same affordance as outboxDrainConfig.tick.
tick <-chan time.Time
}
// SetReminderTickConfig overrides the tick's timings. Zero values keep the
// defaults, so a caller can set one knob without restating the rest. Must be
// called before StartReminderTick; the goroutine captures the interval at
// start.
func (s *Server) SetReminderTickConfig(interval time.Duration, limit int) {
s.reminderTick.mu.Lock()
defer s.reminderTick.mu.Unlock()
if interval > 0 {
s.reminderTick.interval = interval
}
if limit > 0 {
s.reminderTick.limit = limit
}
}
// SetReminderTickChannel replaces the interval ticker with a caller-driven
// channel. Test affordance only.
func (s *Server) SetReminderTickChannel(c <-chan time.Time) {
s.reminderTick.mu.Lock()
defer s.reminderTick.mu.Unlock()
s.reminderTick.tick = c
}
// StartReminderTick starts the periodic reminder sweep. Idempotent.
//
// Started from the real server bootstrap path, not Server.New, so unit tests
// that construct a Server don't spawn a background goroutine unless they opt
// in — the same rule every sweeper here follows.
func (s *Server) StartReminderTick() {
s.reminderTick.mu.Lock()
if s.reminderTick.running {
s.reminderTick.mu.Unlock()
return
}
if s.reminderTick.interval == 0 {
s.reminderTick.interval = defaultReminderTickInterval
}
s.reminderTick.stop = make(chan struct{})
s.reminderTick.running = true
interval := s.reminderTick.interval
stop := s.reminderTick.stop
tick := s.reminderTick.tick
s.reminderTick.mu.Unlock()
slog.Info("reminder tick started", "interval", interval.String())
s.bg.Add(1)
go func() {
defer s.bg.Done()
defer s.recoverSweeper("reminder-tick")
var c <-chan time.Time
if tick != nil {
c = tick
} else {
t := time.NewTicker(interval)
defer t.Stop()
c = t.C
}
for {
select {
case <-stop:
return
case <-c:
s.runReminderTick()
}
}
}()
}
// stopReminderTick signals the loop to exit. Safe when it never started.
func (s *Server) stopReminderTick() {
s.reminderTick.mu.Lock()
defer s.reminderTick.mu.Unlock()
if !s.reminderTick.running {
return
}
close(s.reminderTick.stop)
s.reminderTick.running = false
}
// runReminderTick is one pass: fire every reminder whose instant has arrived.
//
// The store does the arbitration and writes each event in the same transaction
// as the fired_at that retires the reminder, so this function deliberately has
// no delivery logic of its own — the outbox drain picks the events up on its
// own schedule, and on an instance with no webhook dispatcher the poll surface
// serves them instead.
//
// NOW IS TAKEN ONCE per pass and passed down, rather than each row reading the
// clock: a pass that computed "now" per row could fire a reminder whose
// instant fell between two rows of the same scan, making the batch's contents
// depend on how long the batch took.
func (s *Server) runReminderTick() {
if s.store == nil {
return
}
s.reminderTick.mu.Lock()
limit := s.reminderTick.limit
s.reminderTick.mu.Unlock()
nowTS := time.Now().UTC().Format(time.RFC3339)
// A pass can BOTH fire and fail: the store continues past a broken row
// rather than letting it block newer reminders, so it returns the
// reminders it fired alongside the joined errors. Both halves are
// reported — logging only the error would hide work that happened, and
// logging only the count would hide work that did not.
fired, err := s.store.FireDueReminders(nowTS, limit)
if err != nil {
// LOUD ON FAILURE, and never silent: a tick that fails quietly looks
// exactly like a tick that found nothing, which is the shape that let
// a broken watcher sit for thirty minutes reading as "still running".
slog.Error("reminder tick: some reminders could not be fired", "error", err, "fired", len(fired))
}
if len(fired) > 0 {
slog.Info("reminder tick: fired reminders", "count", len(fired))
}
}
+25
View File
@@ -248,6 +248,12 @@ type Server struct {
// loop via stopOutboxDrain.
outboxDrain outboxDrainConfig
// reminderTick holds the periodic config + lifecycle for the item-reminder
// scheduler (IDEA-2641). Mirrors outboxDrain. Configured via
// SetReminderTickConfig + started via StartReminderTick; Stop() signals
// the loop via stopReminderTick.
reminderTick reminderTickConfig
// inFlightUploadHashes tracks content_hash values for uploads
// that have called AttachmentStore.Put but not yet inserted the
// attachments row. Without this, the orphan GC could delete a
@@ -441,6 +447,9 @@ func (s *Server) Stop() {
// SPEC-3 event outbox drain (TASK-2714). Same lifecycle pattern; an
// in-flight delivery is tracked on s.bg and awaited below.
s.stopOutboxDrain()
// Item reminder tick (IDEA-2641). Same lifecycle pattern; an in-flight
// pass is tracked on s.bg and awaited below.
s.stopReminderTick()
// MCP audit writer / sweeper run on s.bg too. Signal first so
// the workers see the close BEFORE Wait() blocks; without the
// signal Wait would hang forever on the writer's blocking
@@ -1818,6 +1827,15 @@ func (s *Server) setupRouter() {
// bus/stream — no durable row, see handlePushToItem's
// doc comment. `pad push <ref> -m "message"`.
r.Post("/push", s.handlePushToItem)
// Reminders (IDEA-2641 / GitHub #1010): the
// fire-at-an-instant primitive. Arming lives under
// the item because a reminder is meaningless without
// one; the lifecycle verbs live at the workspace
// level below, addressed by reminder id, because an
// acknowledgement is about the reminder rather than
// about the item it names.
r.Get("/reminders", s.handleListItemReminders)
r.Post("/reminders", s.handleCreateItemReminder)
})
// Links (v2)
@@ -1909,6 +1927,13 @@ func (s *Server) setupRouter() {
r.Get("/me", s.handleGetMe)
// Dashboard (v2)
// Reminder lifecycle, addressed by reminder id rather
// than by item: an acknowledgement is about the reminder,
// and an item can carry several. Permission is still the
// ITEM's — see resolveReminderForWrite.
r.Patch("/reminders/{reminderID}", s.handleRearmReminder)
r.Post("/reminders/{reminderID}/ack", s.handleAckReminder)
r.Delete("/reminders/{reminderID}", s.handleDeleteReminder)
r.Get("/dashboard", s.handleGetDashboard)
// Workspace graph — {nodes, edges} for the 3D
+2 -2
View File
@@ -139,8 +139,8 @@ func waitForLockWait(t *testing.T, s *Store, needle string, done <-chan error) {
for {
select {
case err := <-done:
t.Fatalf("the insert completed (err = %v) instead of blocking on the "+
"item row held by an uncommitted archival — it read around the lock", err)
t.Fatalf("the statement completed (err = %v) instead of blocking on the "+
"row held by an uncommitted archival — it read around the lock", err)
default:
}
+3 -1
View File
@@ -1373,7 +1373,7 @@ func TestWriteOutboxTx_RejectsMismatchedPayloadFamily(t *testing.T) {
// TASK-2714 edits this table (the handler-path bulk mapping), which is why the
// independent copy lands as this unit's first commit.
func TestCanonicalEventsAreFullyDeclared(t *testing.T) {
// The events/1 set at SPEC-3 v1.4. Adding, removing or re-homing an entry
// The events/1 set at SPEC-3 v1.7. Adding, removing or re-homing an entry
// here is a CONTRACT CHANGE: update the spec version and the taxonomy's
// doc comment in the same commit.
want := map[string]struct {
@@ -1397,6 +1397,7 @@ func TestCanonicalEventsAreFullyDeclared(t *testing.T) {
"pack.installed": {kernelevents.SubjectPack, []string{kernelevents.PayloadPack}, ""},
"pack.upgraded": {kernelevents.SubjectPack, []string{kernelevents.PayloadPack}, ""},
"pack.disabled": {kernelevents.SubjectPack, []string{kernelevents.PayloadPack}, ""},
"item.reminder_due": {kernelevents.SubjectReminder, []string{kernelevents.PayloadReminder}, ""},
}
// The name constants are pinned to their wire strings separately, because
@@ -1419,6 +1420,7 @@ func TestCanonicalEventsAreFullyDeclared(t *testing.T) {
kernelevents.PackInstalled: "pack.installed",
kernelevents.PackUpgraded: "pack.upgraded",
kernelevents.PackDisabled: "pack.disabled",
kernelevents.ItemReminderDue: "item.reminder_due",
} {
if constant != wire {
t.Errorf("event name constant = %q, want %q on the wire", constant, wire)
+133
View File
@@ -241,6 +241,42 @@ func (s *Store) ExportWorkspace(slug string) (*models.WorkspaceExport, error) {
return nil, err
}
// Reminders — exported with their lifecycle marks intact, and ONLY for
// items this bundle actually carries.
//
// The comment that stood here said soft-deleted items' reminders were
// included so "a restore that brings the item back brings its reminder
// with it", copying the item_links rationale. That was false for this
// table: the items section filters on `deleted_at IS NULL`, so the item is
// NOT in the bundle, and there is no restore that could ever reunite them
// — the import simply drops the orphan on its itemMap lookup. Exporting
// them shipped rows that could only ever be discarded, under a comment
// asserting a benefit the bundle cannot deliver.
//
// item_links can carry soft-deleted endpoints because a link is a row
// ABOUT two items and the graph is worth round-tripping raw; a reminder
// whose item is absent is not a relationship, it is a dangling schedule.
reminderRows, err := s.db.Query(s.q(`
SELECT r.item_id, r.remind_at, COALESCE(r.fired_at, ''), COALESCE(r.acked_at, ''), r.created_at, r.updated_at
FROM item_reminders r
JOIN items i ON i.id = r.item_id AND i.workspace_id = r.workspace_id
WHERE r.workspace_id = ? AND i.deleted_at IS NULL
ORDER BY r.created_at, r.id`), ws.ID)
if err != nil {
return nil, fmt.Errorf("export reminders: %w", err)
}
defer reminderRows.Close()
for reminderRows.Next() {
var rm models.ReminderExport
if err := reminderRows.Scan(&rm.ItemID, &rm.RemindAt, &rm.FiredAt, &rm.AckedAt, &rm.CreatedAt, &rm.UpdatedAt); err != nil {
return nil, fmt.Errorf("scan reminder: %w", err)
}
export.Reminders = append(export.Reminders, rm)
}
if err := reminderRows.Err(); err != nil {
return nil, err
}
// Item versions
versionRows, err := s.db.Query(s.q(`
SELECT v.id, v.item_id, v.content, v.change_summary, v.created_by, v.source, v.is_diff, v.created_at
@@ -461,6 +497,20 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
coercedTags := make(map[string]string, len(data.Items))
// Slugs this import has already written. See the collision note in the loop.
claimedSlugs := make(map[string]bool, len(data.Items))
// itemMap records the id an item WOULD get; insertedItems records the ones
// that actually landed. The two differ for an orphaned item — one whose
// collection is missing from the bundle — because the map entry is written
// before the skip below, and it has to be: parent resolution inside this
// same loop reads itemMap for items it has not reached yet.
//
// So a later section resolving an id through itemMap alone can get one
// that names no row, and inserting a foreign key to it fails (SQLite
// enforces FKs here — `_pragma=foreign_keys(on)` in the DSN — and Postgres
// always does). item_links and item_versions survive that by skipping on
// error; the reminder loop below checks this set instead, which refuses
// the row for the right reason rather than letting the database refuse it
// for an incidental one.
insertedItems := make(map[string]bool, len(data.Items))
var nextItemNumber int
for _, it := range data.Items {
newItemID := newID()
@@ -571,6 +621,7 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
if err != nil {
return nil, fmt.Errorf("import item %s: %w", it.Title, err)
}
insertedItems[newItemID] = true
}
// Second pass: remap parent_id and relation fields (now all items exist).
@@ -640,6 +691,88 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
}
}
// Import reminders. NULL rather than empty string for the unset marks —
// the lifecycle is defined by NULL-ness (models.Reminder), and an empty
// string would make a never-fired reminder read as fired at "".
for _, rm := range data.Reminders {
newItemID := itemMap[rm.ItemID]
// TWO GUARDS, AND NEITHER ALONE IS OBSERVABLE — measured, not assumed.
// Reverting either one on its own leaves the test green: with the map
// gate restored, the skip-on-error below survives the FK failure; with
// the fatal return restored, this gate means the insert never fails.
// Removing BOTH is what fails it. They are kept as a pair because they
// defend the same failure at different depths — this one prevents the
// bad write, the one below survives a bad write that arrives some
// other way — and the pair is recorded here so a future reader does
// not delete one as dead code after watching its mutant survive.
//
// insertedItems, not just a non-empty mapping: an ORPHANED item — one
// whose collection is missing from the bundle — still gets a map entry
// (it is written before the skip, because parent resolution needs it),
// so `!= ""` is satisfied by an id that names no row. Inserting a
// foreign key to it fails, and this loop used to treat that as fatal,
// so ONE orphaned item with a reminder aborted the entire workspace
// restore. Codex round 10.
if !insertedItems[newItemID] {
continue
}
// NORMALIZE ON THE WAY IN. Import is a WRITER like any other, and a
// bundle is not necessarily one this server produced — it can be
// hand-edited, or come from another instance. Inserting a raw
// remind_at would let a bare date or a local offset into the one
// column every comparison downstream treats as a UTC instant, where
// it fires early, late, or never. Every other door normalizes; this
// one was writing underneath them.
//
// A value that will not parse is SKIPPED, not fatal: the import-side
// precedent here is lenient (coerce or drop, keep the import alive)
// rather than failing a whole workspace restore over one row.
remindAt, err := normalizeRemindAt(rm.RemindAt)
if err != nil {
slog.Warn("workspace import: skipping reminder with an unparseable remind_at",
"workspace_id", ws.ID, "item_id", newItemID, "raw_len", len(rm.RemindAt))
continue
}
var firedAt, ackedAt any
if rm.FiredAt != "" {
firedAt = rm.FiredAt
}
if rm.AckedAt != "" {
// ACKED WITHOUT FIRED IS NOT A STATE (codex round 11). The
// lifecycle has three: armed, fired-unacked, fired-acked. A bundle
// carrying an acknowledgement with no fire — which this server's
// export cannot produce, but a hand-edited or foreign one can —
// would import a reminder that fires, is excluded from the pending
// surface because it is already acked, and can never be
// acknowledged because AckReminder requires acked_at IS NULL. It
// would emit an event and then be invisible forever.
//
// The acknowledgement is dropped rather than the row: the user's
// SCHEDULE is the part worth keeping, and an ack of something that
// never fired means nothing. Lenient, matching the import-side
// precedent in this file.
if firedAt == nil {
slog.Warn("workspace import: dropping an acknowledgement on a reminder that never fired",
"workspace_id", ws.ID, "item_id", newItemID)
} else {
ackedAt = rm.AckedAt
}
}
if _, err := tx.Exec(s.q(`
INSERT INTO item_reminders (id, workspace_id, item_id, remind_at, fired_at, acked_at, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`),
newID(), ws.ID, newItemID, remindAt, firedAt, ackedAt, rm.CreatedAt, rm.UpdatedAt); err != nil {
// SKIP, not fatal — matching item_links and item_versions, whose
// loops both survive a bad row. A reminder is the least critical
// thing in a bundle, and failing a 900-item restore over one of
// them is the wrong trade; this was the aggravating half of the
// round-10 finding, and it was mine, not the pre-existing mapping.
slog.Warn("workspace import: skipping reminder that failed to insert",
"workspace_id", ws.ID, "item_id", newItemID, "error", err)
continue
}
}
// Import item versions
for _, ver := range data.ItemVersions {
newItemID := itemMap[ver.ItemID]
@@ -0,0 +1,99 @@
-- Item reminders: the fire-at-a-time primitive (IDEA-2641, GitHub #1010).
--
-- The gap this closes is that Pad's date handling was entirely REACTIVE. A
-- `due_date` field makes an item show up as overdue once someone asks the
-- dashboard, but nothing in the server ever ACTS at a target time, so
-- "revisit TASK-X on 2026-08-01" had to live in an external cron. This table
-- is the state a scheduler tick reads.
--
-- WHY A TABLE AND NOT AN ANNOTATION ON THE SCHEMA FIELD, which is the shape
-- the design sketch proposed and recon overturned: an annotation stored as a
-- new key on models.FieldDef does not survive an ordinary collection edit.
-- The web editor destructures each field into an EditableField and rebuilds a
-- fresh definition key-by-key on save (EditCollectionModal.svelte), so any key
-- it does not know about is dropped — `pattern` and `unique_scope` survive
-- only because two lines were hand-added for them. Independently,
-- models.CollectionSchema has fixed fields and no catch-all, so any Go
-- unmarshal+marshal round-trip strips unknown properties; that is the hazard
-- retargetRelationFieldsTx mutates raw JSON to avoid, and it names it in its
-- own comment. Both failures are SILENT and both take out a whole
-- collection's reminders at once. It is the same defect class that moved
-- traits out of the schema column in TASK-2657.
--
-- The table also gives the two semantics the annotation shape would have had
-- to invent somewhere a natural home: a reminder has a LIFECYCLE (armed,
-- fired, acknowledged, re-armed) and that lifecycle is per-reminder state, not
-- a property of a field definition.
--
-- SCOPE: one-shot reminders only. Recurrence multiplies the re-arm semantics
-- and is a separate item, not effort deferred.
--
-- WHAT THIS TABLE IS NOT: it is not where `due_date` lives. Due dates stay
-- ordinary schema date fields and keep their existing reactive behaviour;
-- `due` (a state surfaces react to) and `remind_at` (an instant the server
-- acts on) are different primitives and this is only the second one.
CREATE TABLE IF NOT EXISTS item_reminders (
id TEXT PRIMARY KEY,
-- Denormalized from the item so the scheduler tick can claim and scope
-- work without joining items on every pass, and so a workspace-scoped
-- read stays a single-table query.
workspace_id TEXT NOT NULL,
-- ON DELETE CASCADE, unlike event_outbox's deliberate absence of foreign
-- keys. An outbox row must outlive its subject because an item.deleted
-- event is dispatched after the item is gone. A reminder is the opposite:
-- it is an instruction to say something about an item LATER, and once the
-- item is gone there is nothing to say. Firing a reminder for a deleted
-- item would be a notification a user can do nothing with.
item_id TEXT NOT NULL REFERENCES items(id) ON DELETE CASCADE,
-- The instant to fire at: RFC3339, UTC, always. Deliberately NOT a
-- `date`-typed schema value, which admits both YYYY-MM-DD and full
-- RFC3339 (internal/items/validate.go) and is compared elsewhere against
-- the SERVER'S LOCAL calendar day. A fire-at time cannot inherit that
-- ambiguity: "2026-08-01" does not name an instant, and the difference
-- between the two shapes is a whole day of drift. The remaining
-- timezone question for due_date is tracked as its own item.
remind_at TEXT NOT NULL,
-- Lifecycle. NULL fired_at is the ARMED set — the only rows a tick
-- considers. Set once the tick has emitted the event.
fired_at TEXT,
-- Explicit acknowledgement, and the reason it is a separate column rather
-- than clearing fired_at: the three states (armed / fired-unacked /
-- fired-acked) are distinguishable only if firing and acking are recorded
-- separately. Clearing fired_at on ack would return the row to the armed
-- set and fire it again on the next tick.
--
-- Acking is EXPLICIT and nothing else acks. In particular an item
-- reaching a terminal status does not: that would make every status write
-- a reminder mutation, and it would silently consume a reminder a user
-- may have set precisely to fire after the work was finished. The poll
-- surface instead FILTERS fired-unacked reminders whose item is terminal,
-- without mutating the row — the user's intent stays in the table and the
-- agent stops being shown a dead item.
acked_at TEXT,
created_at TEXT NOT NULL,
updated_at TEXT NOT NULL
);
-- The tick's index: armed rows in fire order. Partial, so it holds only the
-- pending set rather than every reminder ever created — the armed set is
-- transient while the table retains fired rows as the record that a reminder
-- existed and went out.
CREATE INDEX IF NOT EXISTS idx_item_reminders_armed
ON item_reminders(remind_at)
WHERE fired_at IS NULL;
-- The poll surface's index: fired-but-unacked rows, which is what `pad
-- project next` / `ready` reads.
CREATE INDEX IF NOT EXISTS idx_item_reminders_unacked
ON item_reminders(workspace_id, fired_at)
WHERE fired_at IS NOT NULL AND acked_at IS NULL;
-- Per-item reads (an item's own reminders, and the cascade's lookup path).
CREATE INDEX IF NOT EXISTS idx_item_reminders_item
ON item_reminders(item_id, remind_at);
@@ -99,6 +99,14 @@ item_links.source_id
item_links.target_id
item_links.user_id
item_links.workspace_id
item_reminders.acked_at
item_reminders.created_at
item_reminders.fired_at
item_reminders.id
item_reminders.item_id
item_reminders.remind_at
item_reminders.updated_at
item_reminders.workspace_id
item_stars.created_at
item_stars.item_id
item_stars.user_id
+15
View File
@@ -112,6 +112,21 @@ func TestNULColumnCensus(t *testing.T) {
//
// Regenerate with GEN_NUL_BASELINE=1 only AFTER deciding each new column's
// class; the file is evidence of a judgement, not a snapshot to refresh.
// The regeneration path the comment above promises. It lived only in
// prose until IDEA-2641 hit the guard and found the flag did nothing —
// an instruction naming a mechanism that does not exist sends the next
// reader to hand-edit the file, which is the one form of "regeneration"
// that can silently drop an entry it did not mean to. It writes the
// CURRENT unaccounted set, so it records the judgement the developer just
// made rather than merging into whatever was there before.
if os.Getenv("GEN_NUL_BASELINE") == "1" {
if err := os.WriteFile("nul_unprotected_baseline.txt", []byte(strings.Join(unaccounted, "\n")+"\n"), 0o644); err != nil {
t.Fatalf("write baseline: %v", err)
}
t.Logf("regenerated nul_unprotected_baseline.txt with %d entries — review the diff before committing", len(unaccounted))
return
}
baseline, err := os.ReadFile("nul_unprotected_baseline.txt")
if err != nil {
t.Fatalf("read baseline: %v", err)
+8 -1
View File
@@ -290,7 +290,7 @@ func nulKeyPredicate(key map[string]string) (string, []any) {
// MigratedTables names the tables `pad db migrate-to-pg` actually copies.
//
// The migration is application-level: it walks workspaces and runs
// ExportWorkspace / ImportWorkspace on each. That reads six tables and no
// ExportWorkspace / ImportWorkspace on each. That reads seven tables and no
// others — the command's own help says users, platform settings and auth data
// are NOT migrated — so a NUL in users.name, platform_settings.value,
// sessions.user_agent or any oauth table cannot break it.
@@ -320,5 +320,12 @@ func MigratedTables() map[string]bool {
"comments": true,
"item_links": true,
"item_versions": true,
// item_reminders joined the export in IDEA-2641. Its columns are all
// machine-produced (ids, a re-parsed RFC3339 instant, server clocks),
// so a NUL here is not reachable through any writer — it is listed for
// COVERAGE, not because the preflight expects to find anything. The
// alternative is a table the migration copies and the preflight does
// not know about, which is the exact gap this list exists to close.
"item_reminders": true,
}
}
+1
View File
@@ -265,6 +265,7 @@ func TestMigratedTablesCoversTheExport(t *testing.T) {
"Comments": "comments",
"ItemLinks": "item_links",
"ItemVersions": "item_versions",
"Reminders": "item_reminders",
}
migrated := MigratedTables()
@@ -0,0 +1,34 @@
-- Item reminders — Postgres mirror of
-- internal/store/migrations/085_item_reminders.sql. See the SQLite migration
-- for the full rationale (why a table rather than a schema-field annotation;
-- why ON DELETE CASCADE here where event_outbox deliberately has no foreign
-- keys; why remind_at is an RFC3339 UTC instant rather than a `date` value;
-- why ack is its own column and nothing implicit acks).
--
-- One dialect note: timestamps stay TEXT, matching every other table in this
-- schema (items, watches, activities, event_outbox). Not a preference —
-- remind_at is compared against values the Go layer formats, and a TIMESTAMPTZ
-- here would silently change comparison semantics on the one column the
-- scheduler tick's claim predicate depends on. event_outbox's migration made
-- the same call on occurred_at for the same reason.
CREATE TABLE IF NOT EXISTS item_reminders (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL,
item_id TEXT NOT NULL REFERENCES items(id) ON DELETE CASCADE,
remind_at TEXT NOT NULL,
fired_at TEXT,
acked_at TEXT,
created_at TEXT NOT NULL,
updated_at TEXT NOT NULL
);
CREATE INDEX IF NOT EXISTS idx_item_reminders_armed
ON item_reminders(remind_at)
WHERE fired_at IS NULL;
CREATE INDEX IF NOT EXISTS idx_item_reminders_unacked
ON item_reminders(workspace_id, fired_at)
WHERE fired_at IS NOT NULL AND acked_at IS NULL;
CREATE INDEX IF NOT EXISTS idx_item_reminders_item
ON item_reminders(item_id, remind_at);
+801
View File
@@ -0,0 +1,801 @@
package store
import (
"database/sql"
"errors"
"fmt"
"time"
"github.com/PerpetualSoftware/pad/internal/kernelevents"
"github.com/PerpetualSoftware/pad/internal/models"
)
// Item reminders — the fire-at-an-instant primitive (IDEA-2641, GitHub #1010).
//
// See migration 085 for why this is a table rather than an annotation on a
// schema field, and models.Reminder for the three-state lifecycle.
// reminderFireable is the single definition of "this reminder may fire",
// referenced by BOTH the candidate scan and the fire UPDATE's arbiter.
//
// ONE STRING, because the drift between those two is a defect class this unit
// hit three times: the scan filtered something the arbiter did not revalidate,
// so a change committed between them fired a reminder that no longer
// qualified. Round 3 was a re-armed instant, round 7 a workspace deleted
// mid-pass, and the round-1 soft-deleted item was the same shape caught from
// the other side. Each was fixed as an instance; this is the shape.
//
// Written as a correlated EXISTS on item_reminders.item_id — rather than as a
// JOIN — precisely so the identical text is valid in a SELECT and in an
// UPDATE. The scan deliberately does NOT alias item_reminders, so the two uses
// are the same characters and a new condition is one edit in one place.
//
// The two predicates that are NOT here (`fired_at IS NULL`, `remind_at <= ?`)
// are the ones that live on the reminder row itself and are already spelled
// identically at both sites; folding them in would need a parameter order this
// shared form cannot fix.
//
// THE ROW'S OWN workspace_id MUST AGREE WITH ITS ITEM'S (codex round 13).
// CreateReminder writes the pair from the item, and import writes it from
// its own in-workspace mapping, so no door produces a disagreement today —
// but the table has an FK to the item and no constraint tying the two
// columns, and every reader scopes by r.workspace_id and then joins the item.
// A row that ever disagreed (a hand-edited bundle, a future move door, a
// direct write) would carry one workspace's item into another's dashboard
// and webhooks. The identity is asserted in the predicate, so the scan, the
// arbiter, the pin and the reads all refuse the row rather than one of them
// deciding it is "unreachable" on the others' behalf.
const reminderFireable = `EXISTS (
SELECT 1 FROM items i
JOIN workspaces w ON w.id = i.workspace_id
WHERE i.id = item_reminders.item_id
AND i.workspace_id = item_reminders.workspace_id
AND i.deleted_at IS NULL
AND w.deleted_at IS NULL
)`
const reminderColumns = `id, workspace_id, item_id, remind_at, fired_at, acked_at, created_at, updated_at`
// reminderOwned is the identity half of reminderFireable on its own — "this
// row's workspace is its item's workspace" — for the reads that do not care
// about liveness (a fired reminder on an archived item is still history worth
// showing) but must still refuse a row whose two columns disagree. Referenced
// by GetReminder and ListRemindersForItem (codex round 14); the write paths
// reach a row only through GetReminder, so scoping it scopes them. A row no
// door can write needs no door to delete it.
const reminderOwned = `EXISTS (
SELECT 1 FROM items i
WHERE i.id = item_reminders.item_id
AND i.workspace_id = item_reminders.workspace_id
)`
// defaultReminderFireLimit bounds one tick's work. Reminders arrive at a rate
// set by users arming them, not by traffic, so a tick has no reason to be
// large — but a backlog is possible after downtime, and an unbounded pass
// would try to fire every overdue reminder in one transaction storm. The
// remainder is not lost: it is still armed, and the next tick takes the next
// batch, oldest first.
const defaultReminderFireLimit = 100
// defaultPendingReminderLimit bounds the poll surface's window.
//
// THE RECEIPT: this is a NOTIFICATION list a human or an agent reads at a
// glance, not a queue to drain, so the bound is set by what is worth showing
// rather than by what the database can return. Fifty unacknowledged reminders
// already means the surface is not being used as intended; showing five
// hundred would not help, and the payload is embedded in every dashboard
// response, which is the hottest read in the product. The truncation is
// REPORTED rather than silent, so a caller that genuinely has more can tell.
const defaultPendingReminderLimit = 50
func scanReminder(row interface{ Scan(...any) error }) (*models.Reminder, error) {
var r models.Reminder
var firedAt, ackedAt sql.NullString
if err := row.Scan(&r.ID, &r.WorkspaceID, &r.ItemID, &r.RemindAt, &firedAt, &ackedAt, &r.CreatedAt, &r.UpdatedAt); err != nil {
return nil, err
}
if firedAt.Valid {
r.FiredAt = &firedAt.String
}
if ackedAt.Valid {
r.AckedAt = &ackedAt.String
}
return &r, nil
}
// normalizeRemindAt re-parses and re-formats an instant, and REFUSES anything
// that is not RFC3339.
//
// The HTTP edge already normalizes, so on that path this is a second check of
// a value that is already correct — and it is here anyway, because the
// alternative was a doc comment saying "the caller normalizes", which protects
// nothing: the store is callable from anywhere in the process and a doc
// comment does not travel with the argument. Enforcing it means the stored
// value is always machine-produced from a parsed time, so no caller bytes
// reach the column at all. That is what lets remind_at sit outside the NUL
// census's protected set on a positive argument rather than an assumption.
func normalizeRemindAt(remindAt string) (string, error) {
t, err := time.Parse(time.RFC3339, remindAt)
if err != nil {
return "", fmt.Errorf("remind_at must be an RFC3339 instant: %w", err)
}
return NormalizeInstant(t), nil
}
// ErrReminderItemGone is CreateReminder's answer when the item is not a live
// item of the given workspace. Missing, soft-deleted, and belonging to another
// workspace are indistinguishable on purpose: telling them apart would make the
// store answer "does this item id exist somewhere on the instance", which is
// the existence-oracle shape GetReminder already refuses to be.
var ErrReminderItemGone = errors.New("reminder item is not live in this workspace")
// CreateReminder arms a reminder on an item.
//
// THE ITEM MUST BE A LIVE ITEM OF THIS WORKSPACE, and that is asserted by the
// INSERT itself rather than by a read before it (codex round 12). The table
// has a foreign key to items but no same-workspace constraint, so a plain
// INSERT accepts a (workspace, item) pair that names another workspace's item
// — and every reader then scopes by r.workspace_id and joins the item, which
// hands the first workspace's dashboard and webhooks the second one's title.
// The HTTP door resolves the item inside the workspace before calling here;
// the store refuses regardless, because a door is not the only caller the
// process can grow and a doc comment does not travel with the argument (the
// same argument normalizeRemindAt makes one function up).
//
// Liveness is part of the same predicate: arming a reminder on an archived
// item would write a row the scan filters out forever, which reads to the
// caller as a reminder that silently never fires.
func (s *Store) CreateReminder(workspaceID, itemID, remindAt string) (*models.Reminder, error) {
remindAt, err := normalizeRemindAt(remindAt)
if err != nil {
return nil, err
}
id := newID()
ts := now()
res, err := s.db.Exec(s.q(`
INSERT INTO item_reminders (id, workspace_id, item_id, remind_at, fired_at, acked_at, created_at, updated_at)
SELECT ?, i.workspace_id, i.id, ?, NULL, NULL, ?, ?
FROM items i
WHERE i.id = ? AND i.workspace_id = ? AND i.deleted_at IS NULL
`), id, remindAt, ts, ts, itemID, workspaceID)
if err != nil {
return nil, fmt.Errorf("create reminder: %w", err)
}
n, err := res.RowsAffected()
if err != nil {
return nil, fmt.Errorf("create reminder: %w", err)
}
if n == 0 {
return nil, ErrReminderItemGone
}
return s.GetReminder(workspaceID, id)
}
// GetReminder returns one reminder scoped to a workspace, or (nil, nil) when
// no such row exists.
//
// WORKSPACE-SCOPED ON PURPOSE, even though the id is a UUID and collisions are
// not the concern: an unscoped lookup would answer "does this id exist" for
// every workspace on the instance, which is the existence-oracle shape a
// sibling handler family already had to be fixed for. The caller has the
// workspace; requiring it costs nothing.
func (s *Store) GetReminder(workspaceID, id string) (*models.Reminder, error) {
row := s.db.QueryRow(s.q(`SELECT `+reminderColumns+` FROM item_reminders WHERE id = ? AND workspace_id = ? AND `+reminderOwned), id, workspaceID)
r, err := scanReminder(row)
if err == sql.ErrNoRows {
return nil, nil
}
if err != nil {
return nil, fmt.Errorf("get reminder: %w", err)
}
return r, nil
}
// ListRemindersForItem returns every reminder on an item, armed or not,
// soonest first. History is included because a fired reminder is the record
// that a reminder existed and went out.
//
// Takes the workspace as well as the item (codex round 14): the caller has
// already resolved the item inside the workspace, so the argument costs
// nothing, and it lets the query refuse a row stamped with a different
// workspace than the item it points at — the same identity every other read
// asserts, rather than the one read that trusted the item_id alone.
func (s *Store) ListRemindersForItem(workspaceID, itemID string) ([]*models.Reminder, error) {
rows, err := s.db.Query(s.q(`SELECT `+reminderColumns+` FROM item_reminders WHERE item_id = ? AND workspace_id = ? AND `+reminderOwned+` ORDER BY remind_at, id`), itemID, workspaceID)
if err != nil {
return nil, fmt.Errorf("list reminders: %w", err)
}
defer rows.Close()
var out []*models.Reminder
for rows.Next() {
r, err := scanReminder(rows)
if err != nil {
return nil, fmt.Errorf("scan reminder: %w", err)
}
out = append(out, r)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("iterate reminders: %w", err)
}
return out, nil
}
// RearmReminder moves a reminder's instant and clears BOTH fire marks, so a
// reminder that already fired becomes armed again.
//
// The clear is unconditional rather than "only when fired_at is set" because
// the two cases must not diverge: on an armed row the marks are already NULL
// and the write is a no-op, and making it conditional would create a path
// where a re-arm leaves a stale acked_at behind on a row that is armed —
// a state models.Reminder's lifecycle does not have a name for.
func (s *Store) RearmReminder(workspaceID, id, remindAt string) (*models.Reminder, error) {
remindAt, err := normalizeRemindAt(remindAt)
if err != nil {
return nil, err
}
res, err := s.db.Exec(s.q(`
UPDATE item_reminders
SET remind_at = ?, fired_at = NULL, acked_at = NULL, updated_at = ?
WHERE id = ? AND workspace_id = ?
`), remindAt, now(), id, workspaceID)
if err != nil {
return nil, fmt.Errorf("rearm reminder: %w", err)
}
if n, err := res.RowsAffected(); err == nil && n == 0 {
return nil, nil
}
return s.GetReminder(workspaceID, id)
}
// AckReminder acknowledges a FIRED reminder.
//
// The `fired_at IS NOT NULL` predicate is what makes acking an armed reminder
// impossible rather than merely discouraged: an acked-but-never-fired row
// would sit in a state the lifecycle has no name for, and it would be
// invisible — the poll surface reads fired-unacked, so the row would simply
// never appear again.
//
// THE STATEMENT MATCHES EVERY FIRED ROW, acknowledged or not, so that a
// non-match means exactly one thing: at the instant of the ack, the reminder
// had not fired (or does not exist, which the handler tells apart by
// re-reading). The previous form also excluded already-acked rows, which left
// a no-match ambiguous — "too early" and "already done" need opposite
// reactions from a caller — and the handler resolved the ambiguity from a row
// it had read BEFORE the ack. A fire or re-arm landing between that read and
// the UPDATE made it answer 409 for a reminder it had just acknowledged, or
// 200 for one it had not (codex round 12). Folding the distinction into the
// statement removes the read the race needed.
//
// IDEMPOTENT by construction: COALESCE keeps the first acknowledgement's
// instant, and updated_at moves only when acked_at does, so a second ack
// matches, returns the row, and rewrites nothing.
func (s *Store) AckReminder(workspaceID, id string) (*models.Reminder, error) {
ts := now()
res, err := s.db.Exec(s.q(`
UPDATE item_reminders
SET updated_at = CASE WHEN acked_at IS NULL THEN ? ELSE updated_at END,
acked_at = COALESCE(acked_at, ?)
WHERE id = ? AND workspace_id = ? AND fired_at IS NOT NULL
`), ts, ts, id, workspaceID)
if err != nil {
return nil, fmt.Errorf("ack reminder: %w", err)
}
if n, err := res.RowsAffected(); err == nil && n == 0 {
return nil, nil
}
return s.GetReminder(workspaceID, id)
}
// DeleteReminder removes a reminder outright. Disarming by deletion is the
// only disarm: there is no "cancelled" state, because a cancelled reminder and
// an absent one are indistinguishable to every surface that reads them.
func (s *Store) DeleteReminder(workspaceID, id string) (bool, error) {
res, err := s.db.Exec(s.q(`DELETE FROM item_reminders WHERE id = ? AND workspace_id = ?`), id, workspaceID)
if err != nil {
return false, fmt.Errorf("delete reminder: %w", err)
}
n, err := res.RowsAffected()
if err != nil {
return false, nil
}
return n > 0, nil
}
// ListPendingReminders returns a workspace's fired-and-unacked reminders,
// joined to the items they are about, soonest-fired first.
//
// This is the AGENT POLL SURFACE, and it is not optional. The outbox drain
// acks an event immediately when no webhook dispatcher is configured — the
// common self-hosted shape — so a webhook-only reminder would be a no-op on
// most installs. The row this query returns is the only thing that survives
// on those instances.
//
// Terminal-item filtering happens in the CALLER, not here, because terminality
// is schema-defined (a collection's terminal_options) and lives in JSON the
// SQL layer would have to parse. The caller already builds that context for
// the dashboard; ItemFields and CollectionID are carried for it.
// PendingReminderScope narrows the query to what a caller may see. Nil
// CollectionIDs means unrestricted; a NON-NIL EMPTY slice with no ItemIDs
// means nothing is visible, matching models.ItemListParams so the two
// visibility paths cannot drift into opposite readings of the same value.
type PendingReminderScope struct {
CollectionIDs []string
ItemIDs []string
}
// The window is BOUNDED because this list feeds a payload that is otherwise
// capped: every pending reminder became a suggestion prepended to a
// three-entry list, so a workspace with five hundred unacknowledged reminders
// returned five hundred suggestions and grew without limit until somebody
// acknowledged them (codex round 3). Oldest-fired first, so the window holds
// the reminders that have been waiting longest rather than an arbitrary slice.
//
// The caller is told when the window was not the whole set — but as a BOOLEAN,
// not a count. A count would have to be stated post-visibility-filter to be
// true for the caller reading it, and this query cannot compute that: the
// filter runs above, per item. "There are more than you can see here" is the
// strongest claim the data supports, so it is the one made.
// VISIBILITY IS SCOPED IN SQL, not filtered afterwards, and that is the
// round-4 correction. The round-3 bound took the first N rows and let the
// caller discard the ones it could not show — which recreated, in the READ
// path, the exact starvation the round-1 fix removed from the FIRE path:
// fifty rows the caller must drop can hide a visible reminder behind them
// forever, with no continuation to reach it. A bounded window is only safe
// when the discarding happens BEFORE the bound.
//
// One filter necessarily stays above: terminality is defined by a collection's
// schema, which SQL cannot read. That one is handled by paging (the caller
// asks for the next page when a page comes back short), which is why this
// takes an offset at all.
func (s *Store) ListPendingReminders(workspaceID string, scope PendingReminderScope, limit, offset int) ([]*models.PendingReminder, bool, error) {
if limit <= 0 {
limit = defaultPendingReminderLimit
}
if offset < 0 {
offset = 0
}
// Nothing visible at all: answer without touching the database, and say
// there is no more — a truncation flag here would send the caller paging
// through a set it can never see into.
if scope.CollectionIDs != nil && len(scope.CollectionIDs) == 0 && len(scope.ItemIDs) == 0 {
return nil, false, nil
}
query := `
SELECT r.id, r.workspace_id, r.item_id, r.remind_at, r.fired_at, r.acked_at, r.created_at, r.updated_at,
i.slug, i.title, i.fields, i.collection_id, c.slug, c.prefix, i.item_number
FROM item_reminders r
JOIN items i ON i.id = r.item_id AND i.workspace_id = r.workspace_id
JOIN collections c ON c.id = i.collection_id
JOIN workspaces w ON w.id = r.workspace_id
WHERE r.workspace_id = ?
AND r.fired_at IS NOT NULL
AND r.acked_at IS NULL
AND i.deleted_at IS NULL
AND w.deleted_at IS NULL`
args := []any{workspaceID}
// Same three-way shape as models.ItemListParams: a guest may hold
// collection-level grants, item-level grants, or both, and "both" is an OR
// rather than an AND — an item in a fully granted collection qualifies
// even when it is not individually granted.
switch {
case len(scope.CollectionIDs) > 0 && len(scope.ItemIDs) > 0:
query += " AND (i.collection_id IN (" + placeholders(len(scope.CollectionIDs)) +
") OR i.id IN (" + placeholders(len(scope.ItemIDs)) + "))"
for _, id := range scope.CollectionIDs {
args = append(args, id)
}
for _, id := range scope.ItemIDs {
args = append(args, id)
}
case len(scope.CollectionIDs) > 0:
query += " AND i.collection_id IN (" + placeholders(len(scope.CollectionIDs)) + ")"
for _, id := range scope.CollectionIDs {
args = append(args, id)
}
case len(scope.ItemIDs) > 0:
query += " AND i.id IN (" + placeholders(len(scope.ItemIDs)) + ")"
for _, id := range scope.ItemIDs {
args = append(args, id)
}
}
query += " ORDER BY r.fired_at, r.id LIMIT ? OFFSET ?"
args = append(args, limit+1, offset)
rows, err := s.db.Query(s.q(query), args...)
if err != nil {
return nil, false, fmt.Errorf("list pending reminders: %w", err)
}
defer rows.Close()
var out []*models.PendingReminder
for rows.Next() {
var p models.PendingReminder
var firedAt, ackedAt sql.NullString
var prefix string
// items.item_number IS NULLABLE (migration 006 added the column to
// existing rows), and scanning NULL into an int fails the Scan — which
// fails the query, which degrades the whole pending-reminder section
// and hides EVERY reminder in the workspace, not just the one legacy
// item's. ListWatchesForUser, which this query was modelled on, uses
// exactly this type; I copied its shape and dropped the part that
// handles the column's actual nullability.
var number sql.NullInt64
if err := rows.Scan(
&p.ID, &p.WorkspaceID, &p.ItemID, &p.RemindAt, &firedAt, &ackedAt, &p.CreatedAt, &p.UpdatedAt,
&p.ItemSlug, &p.ItemTitle, &p.ItemFields, &p.CollectionID, &p.CollectionSlug, &prefix, &number,
); err != nil {
return nil, false, fmt.Errorf("scan pending reminder: %w", err)
}
if firedAt.Valid {
p.FiredAt = &firedAt.String
}
if ackedAt.Valid {
p.AckedAt = &ackedAt.String
}
// No ref rather than a wrong one: "PREFIX-0" would name a different
// item, and every consumer of this list can render a title without a
// ref (same disposition as ListWatchesForUser).
if prefix != "" && number.Valid {
p.ItemRef = fmt.Sprintf("%s-%d", prefix, number.Int64)
}
out = append(out, &p)
}
if err := rows.Err(); err != nil {
return nil, false, fmt.Errorf("iterate pending reminders: %w", err)
}
// The extra row is the probe, never a result.
if len(out) > limit {
return out[:limit], true, nil
}
return out, false, nil
}
// dueReminderCandidates returns the ids of armed reminders whose instant has
// arrived, oldest first.
//
// Split from FireDueReminders so a test can drive the arbiter below with a
// deliberately STALE candidate list — the race this shape exists for. Through
// the public entry point that race is unobservable, because this query has
// already filtered the rows it is about to hand over. Same split, and the same
// reason, as the outbox claim's pendingClaimCandidates / claimOutboxIDs.
func (s *Store) dueReminderCandidates(nowTS string, limit int) ([]string, error) {
if limit <= 0 {
limit = defaultReminderFireLimit
}
// SOFT-DELETED ITEMS ARE EXCLUDED HERE, not merely skipped downstream
// (codex round 1). fireOneReminder rolls back when it finds the item gone,
// which leaves the reminder ARMED and therefore a candidate again on the
// next pass — so a batch bounded at `limit` and ordered oldest-first can be
// filled entirely by archived items, and no live reminder ever fires. The
// starvation is permanent and silent: the tick reports zero fired and looks
// idle. Filtering in the candidate query means those rows never occupy a
// slot, while the reminders themselves are kept, so restoring the item
// restores its reminder with it.
// THE WORKSPACE IS CHECKED TOO, not only the item (codex round 6).
// Workspace soft-delete deliberately leaves items in place for the 30-day
// restore window, so a workspace-level filter on the ITEM finds nothing
// wrong — and the tick kept firing, emitting outbound webhook events for a
// workspace whose owner had deleted it, possibly as part of deleting their
// account. That is the one failure mode here that reaches outside the
// process, which is why it outranks the starvation cases even though the
// SQL change is the same size.
//
// A restored workspace resumes normally: nothing is destroyed, the
// reminders simply stop being candidates while it is gone.
rows, err := s.db.Query(s.q(`
SELECT id FROM item_reminders
WHERE fired_at IS NULL AND remind_at <= ?
AND `+reminderFireable+`
ORDER BY remind_at, id
LIMIT ?
`), nowTS, limit)
if err != nil {
return nil, fmt.Errorf("due reminder candidates: %w", err)
}
defer rows.Close()
var ids []string
for rows.Next() {
var id string
if err := rows.Scan(&id); err != nil {
return nil, fmt.Errorf("scan due reminder id: %w", err)
}
ids = append(ids, id)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("iterate due reminders: %w", err)
}
return ids, nil
}
// THE FIRE-PATH INVARIANT, stated once so the next change is measured against
// it rather than against the last bug:
//
// THE CANDIDATE SCAN IS A HINT AND MAY BE ASSUMED TO PROVE NOTHING. Every
// condition that made a row a candidate must be re-asserted inside the
// transaction that marks it fired, in the SAME statement that does the
// marking, so that checking and writing are one atomic act. A reminder may
// be marked fired, and its event emitted, only if at that instant: its
// fired_at is still NULL, its remind_at is still at or before the pass's
// nowTS, its item is still not soft-deleted, and its workspace is still not
// soft-deleted.
//
// The reason it is worded as "the scan proves nothing" rather than as a list:
// a list invites the next person to add a condition to the scan and stop. Four
// separate defects in this unit were exactly that — a filter added to the scan
// while the arbiter went on not knowing about it — and each was fixed as an
// instance until the third made the shape visible. reminderFireable exists so
// the two sites cannot spell the shared half differently; this paragraph
// exists so nobody adds a fifth condition to one of them alone.
//
// ITEM LIVENESS IS DEFENDED TWICE, and the pin cannot tell the two apart —
// stated because the first version of this paragraph claimed the item load was
// "for the payload, not for the check", and the mutation matrix falsified that
// in one run. Removing the item half of reminderFireable alone changes NO
// observable behaviour: the UPDATE then matches, the load returns nil for the
// soft-deleted item, and the deferred rollback undoes the write. So the
// invariant holds either way and a single-mutant experiment cannot say which
// guard is carrying it. Removing BOTH kills the test, which is the experiment
// that shows they are a genuine pair rather than one of them being dead.
//
// They are kept as a pair on purpose, and the predicate is the primary: it
// means the row never matches, so no write happens at all, where the load
// means a write happens and is undone. The load is needed regardless — the
// payload carries an item snapshot — so the redundancy costs nothing beyond
// this paragraph. Workspace liveness has no such second READ, which is why
// dropping ITS half of the predicate does fail the pin.
//
// A READ IS NOT A HOLD (codex round 12, found independently by two runs on
// the same line). Everything above re-asserts liveness at the instant the
// predicate is evaluated; nothing above keeps it true until the transaction
// commits. On SQLite that gap does not exist — the DSN's _txlock=immediate
// makes every db.Begin() a BEGIN IMMEDIATE, so an archival cannot even open
// its transaction while a fire is in flight. On Postgres under READ COMMITTED
// the UPDATE locks only the reminder row: DeleteItem or DeleteWorkspace can
// commit its deleted_at after the predicate passed and before the outbox
// write commits, and the event then leaves the process describing a resource
// that was archived before the event existed. fireOneReminder therefore pins
// the item and workspace rows (FOR NO KEY UPDATE) as its first statement on
// Postgres, so the archival waits for the fire to commit — delayed, never
// lost — or, having committed first, makes the pin's re-read miss and the
// fire return without emitting. "At that instant" in the invariant means the
// commit instant, and the pin is what makes the predicate's instant and the
// commit instant the same one.
//
// Emission happens after the predicate passed and inside the same transaction,
// so an event cannot describe a state that no longer held when it was written.
//
// FireDueReminders marks every arrived reminder as fired and writes its event,
// returning the reminders this pass actually fired.
//
// ONE TRANSACTION PER REMINDER, carrying both the fired_at write and the
// outbox insert. That pairing is the whole point and it is not an efficiency
// choice: a fired_at committed without its event is a reminder that silently
// never notifies anyone and can never be retried, because the row has left the
// armed set. An event committed without fired_at fires again every tick. The
// transaction is what makes both unrepresentable — the same discipline the
// outbox itself exists to provide for ordinary mutations.
//
// Per-reminder rather than per-batch so one unfireable row (a deleted item
// racing the tick, a payload that will not marshal) cannot hold back every
// other reminder in the pass.
//
// The UPDATE re-checks the FULL candidate condition — the fire mark, the
// instant, and the shared reminderFireable predicate — so it arbitrates
// against every actor the scan filtered for — and getting only the first was the round-3
// defect. Against a concurrent TICK, `fired_at IS NULL` means both instances
// see the same candidate and exactly one gets RowsAffected 1; the loser does
// no work and emits nothing. Against a concurrent USER, `remind_at <= nowTS`
// means a reminder deferred between the scan and the fire is not fired — which
// the fire mark alone could not catch, because a re-arm CLEARS that mark.
//
// The distinction is worth keeping in view: an arbiter is only an arbiter with
// respect to the writers it can see, and this one was written with ticks in
// mind while a user edit went straight past it.
func (s *Store) FireDueReminders(nowTS string, limit int) ([]*models.Reminder, error) {
ids, err := s.dueReminderCandidates(nowTS, limit)
if err != nil {
return nil, err
}
if len(ids) == 0 {
return nil, nil
}
// ONE FAILURE DOES NOT END THE PASS (codex round 1). The per-reminder
// transaction above exists precisely so that one unfireable row cannot
// hold back the rest — and returning on the first error made that comment
// false, since candidates are ordered oldest-first and a persistently
// broken old reminder would then block every newer one forever. The errors
// are collected rather than dropped: a pass that failed on three rows and
// fired seven must report both halves, or the tick's log reads like a
// clean pass.
return fireEachReminder(ids, nowTS, s.fireOneReminder)
}
// fireEachReminder is the pass's isolation property, split out so a test can
// inject a failing fire for one id and observe that the ids after it still
// run. Through the public entry point that is not reachable: making a real
// reminder fail mid-transaction requires corrupting a row the database
// refuses to store corrupt. Same split, and the same reason, as
// dueReminderCandidates / fireOneReminder.
func fireEachReminder(ids []string, nowTS string, fire func(id, nowTS string) (*models.Reminder, error)) ([]*models.Reminder, error) {
var fired []*models.Reminder
var errs []error
for _, id := range ids {
r, err := fire(id, nowTS)
if err != nil {
errs = append(errs, err)
continue
}
if r != nil {
fired = append(fired, r)
}
}
return fired, errors.Join(errs...)
}
// fireOneReminder is the arbiter plus the emission, in one transaction.
// Returns (nil, nil) when another pass won the row or its item is gone.
func (s *Store) fireOneReminder(id, nowTS string) (*models.Reminder, error) {
tx, err := s.db.Begin()
if err != nil {
return nil, fmt.Errorf("fire reminder: %w", err)
}
defer tx.Rollback()
// PIN THE ITEM AND WORKSPACE ROWS FOR THE REST OF THE TRANSACTION on
// Postgres (codex round 12). The predicate below READS liveness, and a read
// is not a hold: under READ COMMITTED the UPDATE locks only the reminder
// row, so DeleteItem / DeleteWorkspace can commit deleted_at between the
// predicate's evaluation and this transaction's commit, and the event goes
// out about a resource archived before the event existed — a webhook to a
// deleted workspace's endpoint is the one failure here that reaches outside
// the process. See the invariant paragraph on FireDueReminders.
//
// FOR NO KEY UPDATE, as CreateAttachmentForLiveItem: both archival UPDATEs
// touch no key column, so they take FOR NO KEY UPDATE and conflict with
// this holder — the archival blocks until the fire commits and then
// proceeds, delayed but never lost. In the other interleaving the archival
// commits first; the locked re-read is re-evaluated after the wait, no
// longer matches deleted_at IS NULL, and this call returns without firing.
// FK-share readers on the item (comments, the Yjs op-log) are not blocked.
// The workspace join goes through the ITEM's workspace_id, exactly as
// reminderFireable does, so the two cannot disagree about which row.
//
// SQLite skips the pin: its DSN sets _txlock=immediate, so db.Begin() is a
// BEGIN IMMEDIATE and writers already serialize — the interleaving is
// unrepresentable there, and the locking clause is a syntax error.
if s.dialect.Driver() == DriverPostgres {
var pinned string
err := tx.QueryRow(s.q(`
SELECT i.id FROM item_reminders r
JOIN items i ON i.id = r.item_id AND i.workspace_id = r.workspace_id
JOIN workspaces w ON w.id = i.workspace_id
WHERE r.id = ? AND i.deleted_at IS NULL AND w.deleted_at IS NULL
FOR NO KEY UPDATE OF i, w
`), id).Scan(&pinned)
switch {
case err == sql.ErrNoRows:
// Archived, or gone, since the scan. Leave the reminder as it is
// and emit nothing — the same outcome the predicate produces.
return nil, nil
case err != nil:
return nil, fmt.Errorf("pin reminder %s item and workspace: %w", id, err)
}
}
// THE INSTANT IS REVALIDATED HERE, not just the fire mark (codex round 3).
// A re-arm can move this reminder into the future between the candidate
// scan and this UPDATE — it clears fired_at, so a predicate that checked
// only `fired_at IS NULL` still matched, and the pass fired a reminder the
// user had just deferred and emitted its event. The re-arm cannot undo
// that: it can clear the mark, but the event is already on the outbox.
//
// Same nowTS the candidate scan used, deliberately: the arbiter and the
// scan must agree about when this pass is, or a reminder could pass one
// and fail the other for no reason but clock drift within the pass.
res, err := tx.Exec(s.q(`
UPDATE item_reminders SET fired_at = ?, updated_at = ?
WHERE id = ? AND fired_at IS NULL AND remind_at <= ?
AND `+reminderFireable+`
`), nowTS, now(), id, nowTS)
if err != nil {
return nil, fmt.Errorf("fire reminder %s: %w", id, err)
}
n, err := res.RowsAffected()
if err != nil {
return nil, fmt.Errorf("fire reminder %s: %w", id, err)
}
if n == 0 {
// Another instance's tick won this row. Not an error, and emitting
// nothing is the correct outcome: the winner emits.
return nil, nil
}
row := tx.QueryRow(s.q(`SELECT `+reminderColumns+` FROM item_reminders WHERE id = ?`), id)
r, err := scanReminder(row)
if err != nil {
return nil, fmt.Errorf("reload fired reminder %s: %w", id, err)
}
// READ THE ITEM ON THE TRANSACTION, never on s.db. A pool read inside a
// transaction that holds a write lock deadlocks on a single-connection
// pool, which is exactly how this store is configured under SQLite.
item, err := s.GetItemQ(tx, r.ItemID)
if err != nil {
return nil, fmt.Errorf("load item for reminder %s: %w", id, err)
}
if item == nil {
// The item was soft-deleted between the candidate scan and now.
// Roll the fire back rather than emitting an event about an item a
// consumer cannot fetch: the deferred Rollback does it, and the
// reminder stays armed. A hard delete cascades the row away instead.
return nil, nil
}
if err := s.emitReminderEventTx(tx, r, item); err != nil {
return nil, fmt.Errorf("emit reminder event %s: %w", id, err)
}
if err := tx.Commit(); err != nil {
return nil, fmt.Errorf("fire reminder %s: %w", id, err)
}
return r, nil
}
// reminderEventPayload is the PayloadReminder shape: the reminder that fired,
// and the item it is about.
//
// The item is a scrubbed snapshot, matching every other item-carrying payload
// — a reminder event travels the same webhook wire as item.created and must
// not be the one door that ships PII the others strip.
type reminderEventPayload struct {
Reminder *models.Reminder `json:"reminder"`
Item *models.Item `json:"item"`
}
// emitReminderEventTx writes one item.reminder_due event on the caller's
// transaction.
func (s *Store) emitReminderEventTx(tx *sql.Tx, r *models.Reminder, item *models.Item) error {
if r == nil || item == nil {
return fmt.Errorf("outbox: %s has no reminder or item snapshot", kernelevents.ItemReminderDue)
}
payload, err := marshalEventPayload(reminderEventPayload{Reminder: r, Item: scrubItemPII(item)})
if err != nil {
return err
}
return writeOutboxTx(tx, s, OutboxEvent{
WorkspaceID: r.WorkspaceID,
EventType: kernelevents.ItemReminderDue,
SubjectID: r.ID,
Payload: payload,
PayloadFamily: kernelevents.PayloadReminder,
})
}
// NormalizeInstant renders a parsed time as the RFC3339 second it must not
// fire before, in UTC.
//
// SECONDS ARE THE STORED RESOLUTION: the column is compared as a string
// against a whole-second clock, and the tick runs on a 30s interval, so
// sub-second precision is not a thing this system can honour. The question is
// only which way to resolve it, and truncating was wrong (codex round 2):
// `09:00:00.900Z` truncated to `09:00:00Z` fires 900ms BEFORE the moment the
// caller named, and it does so silently, having rewritten their value on the
// way in.
//
// Rounding UP costs at most a second of lateness and makes the guarantee
// stateable: a reminder never fires before the instant it was set for. Late is
// a reminder; early is a wrong answer.
//
// Whole seconds are unchanged, so the ordinary case round-trips exactly.
func NormalizeInstant(t time.Time) string {
u := t.UTC()
if trunc := u.Truncate(time.Second); !trunc.Equal(u) {
u = trunc.Add(time.Second)
}
return u.Format(time.RFC3339)
}
+158
View File
@@ -0,0 +1,158 @@
package store
import (
"os"
"testing"
"github.com/PerpetualSoftware/pad/internal/kernelevents"
)
// Postgres-only pins for the fire path's row pin (IDEA-2641, codex round 12).
//
// reminderFireable READS liveness; the pin HOLDS it. TestFirePathInvariant
// covers the interleaving where the archival commits before the fire — the
// predicate misses and nothing fires. These two cover the interleaving the
// predicate cannot: the archival is IN FLIGHT, uncommitted, when the fire
// begins. Without the pin the fire's predicate reads the pre-archival row
// (READ COMMITTED sees only committed state), the UPDATE and the outbox write
// land, and the archival commits a moment later — an event about a resource
// that no longer exists. With the pin the fire blocks on the archival's row
// lock, and once the archival commits, the re-evaluated re-read no longer
// matches and the fire returns without emitting.
//
// "Blocked" is verified in the database via pg_stat_activity (waitForLockWait),
// not by elapsed time — a bare sleep would pass just as green if the goroutine
// were merely unscheduled.
//
// SQLite is excluded rather than skipped for convenience: its DSN sets
// _txlock=immediate, so the fire cannot even open its transaction while the
// archival is live. The interleaving these tests construct is unrepresentable
// there, which is why the pin is dialect-gated.
//
// MUTANT: removing the `if s.dialect.Driver() == DriverPostgres` pin block
// makes both tests fail at waitForLockWait — the fire completes instead of
// blocking, and emits.
func firePathPGStore(t *testing.T) (*Store, string, string, string) {
t.Helper()
pgURL := os.Getenv("PAD_TEST_POSTGRES_URL")
if pgURL == "" {
t.Skip("PAD_TEST_POSTGRES_URL not set — the row pin only exists on Postgres")
}
s := testStorePostgres(t, pgURL)
ws := createTestWorkspace(t, s, "Pin")
col := createTestCollection(t, s, ws.ID, "Tasks")
item := createTestItem(t, s, ws.ID, col.ID, "Ship it", "")
id := armReminder(t, s, ws.ID, item.ID, past)
ids, err := s.dueReminderCandidates(nowTS(), 0)
if err != nil {
t.Fatalf("dueReminderCandidates: %v", err)
}
if len(ids) != 1 || ids[0] != id {
t.Fatalf("setup: expected the armed reminder as the only candidate, got %v", ids)
}
return s, ws.ID, item.ID, id
}
func TestFireOneReminderBlocksOnAnArchivingWorkspace(t *testing.T) {
s, wsID, _, id := firePathPGStore(t)
tx, err := s.db.Begin()
if err != nil {
t.Fatalf("begin archiving tx: %v", err)
}
defer tx.Rollback() // no-op after Commit
ts := now()
if _, err := tx.Exec(s.q(`UPDATE workspaces SET deleted_at = ?, updated_at = ? WHERE id = ?`), ts, ts, wsID); err != nil {
t.Fatalf("archive workspace in tx: %v", err)
}
type fireResult struct {
fired bool
err error
}
done := make(chan error, 1)
results := make(chan fireResult, 1)
go func() {
r, err := s.fireOneReminder(id, nowTS())
results <- fireResult{fired: r != nil, err: err}
done <- err
}()
waitForLockWait(t, s, "FOR NO KEY UPDATE OF", done)
if err := tx.Commit(); err != nil {
t.Fatalf("commit archival: %v", err)
}
res := <-results
if res.err != nil {
t.Fatalf("fireOneReminder: %v", res.err)
}
if res.fired {
t.Error("fired a reminder whose workspace was archived by the writer it was blocked on")
}
assertNothingLeft(t, s, wsID, id)
}
func TestFireOneReminderBlocksOnAnArchivingItem(t *testing.T) {
s, wsID, itemID, id := firePathPGStore(t)
tx, err := s.db.Begin()
if err != nil {
t.Fatalf("begin archiving tx: %v", err)
}
defer tx.Rollback() // no-op after Commit
ts := now()
if _, err := tx.Exec(s.q(`UPDATE items SET deleted_at = ?, updated_at = ? WHERE id = ?`), ts, ts, itemID); err != nil {
t.Fatalf("archive item in tx: %v", err)
}
type fireResult struct {
fired bool
err error
}
done := make(chan error, 1)
results := make(chan fireResult, 1)
go func() {
r, err := s.fireOneReminder(id, nowTS())
results <- fireResult{fired: r != nil, err: err}
done <- err
}()
waitForLockWait(t, s, "FOR NO KEY UPDATE OF", done)
if err := tx.Commit(); err != nil {
t.Fatalf("commit archival: %v", err)
}
res := <-results
if res.err != nil {
t.Fatalf("fireOneReminder: %v", res.err)
}
if res.fired {
t.Error("fired a reminder whose item was archived by the writer it was blocked on")
}
assertNothingLeft(t, s, wsID, id)
}
// assertNothingLeft: no event left the process and the reminder is still
// armed — the same three assertions TestFirePathInvariant makes, so the pin
// and the predicate are held to one standard.
func assertNothingLeft(t *testing.T, s *Store, wsID, reminderID string) {
t.Helper()
var events int
if err := s.db.QueryRow(s.q(`SELECT COUNT(*) FROM event_outbox WHERE workspace_id = ? AND event_type = ?`),
wsID, kernelevents.ItemReminderDue).Scan(&events); err != nil {
t.Fatalf("count events: %v", err)
}
if events != 0 {
t.Errorf("%d reminder event(s) left the process", events)
}
var firedAt *string
if err := s.db.QueryRow(s.q(`SELECT fired_at FROM item_reminders WHERE id = ?`), reminderID).Scan(&firedAt); err != nil {
t.Fatalf("read reminder: %v", err)
}
if firedAt != nil {
t.Errorf("reminder carries fired_at = %q after a fire that must not have happened", *firedAt)
}
}
File diff suppressed because it is too large Load Diff
+5
View File
@@ -116,6 +116,7 @@ Interpret the user's intent and route to the appropriate action. Here are common
**Querying:**
- "what's on my plate?" → role-filtered queue if a role is active, otherwise `pad project next`
- "remind me about this on <date>" / "revisit TASK-5 next Tuesday" → `pad item remind` (IDEA-2641). The time is an **instant**, not a date — ask for a time of day rather than picking one, since the server refuses a bare date on purpose. A fired reminder shows up in `pad project next` / `ready` until someone runs `pad item ack <reminder-id>``next` prints the exact ack command under the entry, so you never have to go looking for the id; **finishing the item does not acknowledge it**, because a reminder is often armed precisely to fire after the work is done
- "what should I work on?" / "what's ready?" → `pad project ready` (actionable backlog); "what's stuck?" / "what needs attention?" → `pad project stale`
- "show me status" / "how are we doing?" → `pad project dashboard`
- "show me all tasks" / "list bugs" → `pad item list <collection>`
@@ -193,6 +194,10 @@ pad item update TASK-5 [--status X] [--role X] [--assign X] [--comment "..."] [-
pad item delete TASK-5
pad item search "query"
pad item comment TASK-5 "..." [--reply-to <comment-id>]
pad item remind TASK-5 --remind-at 2026-08-01T09:00:00Z # arm a one-shot reminder (RFC3339 INSTANT; a bare date is refused)
pad item reminders TASK-5 # armed / fired / acknowledged
pad item ack <reminder-id> # acknowledge a fired reminder
pad item unremind <reminder-id>
pad item comments TASK-5
pad item note TASK-5 "what you did" [--details "..." | --stdin]
pad item decide TASK-5 "what you chose" [--rationale "..." | --stdin]