Confirm and approve a duty before it dispatches

src/apps/duly.app.ts

@@ -26,6 +26,11 @@ export const DulyApp = App.create({
       children: [
         { id: 'nav_my_week', type: 'object', objectName: 'duly_task', viewName: 'my_week', label: 'My week', icon: 'calendar-check' },
         { id: 'nav_my_duties', type: 'object', objectName: 'duly_duty', viewName: 'mine', label: 'My duties', icon: 'clipboard-list' },
+        // Directly under "My duties", because it is the same list filtered to
+        // the rows that are waiting on this person — to confirm, or to correct
+        // and send back up. An unconfirmed duty dispatches nothing, so this is
+        // the one entry in the group whose emptiness is the goal.
+        { id: 'nav_to_confirm', type: 'object', objectName: 'duly_duty', viewName: 'to_confirm', label: 'To confirm', icon: 'clipboard-check' },
         { id: 'nav_standing', type: 'object', objectName: 'duly_duty', viewName: 'standing', label: 'Standing duties', icon: 'anchor' },
         { id: 'nav_log', type: 'object', objectName: 'duly_log_entry', label: 'Work log', icon: 'notebook-pen' },
         // The board lives HERE, not under Team, and that placement is a
@@ -72,6 +77,11 @@ export const DulyApp = App.create({
           label: 'People',
           icon: 'users-round',
         },
+        // A reviewer's queue, and the only entry in this group that is a
+        // WRITE surface. It sits with the manager's screens rather than under
+        // "My work" for the same reason the board does not: the rows in it
+        // belong to other people.
+        { id: 'nav_to_review', type: 'object', objectName: 'duly_duty', viewName: 'to_review', label: 'To review', icon: 'clipboard-pen' },
         { id: 'nav_late', type: 'object', objectName: 'duly_task', viewName: 'late', label: 'Late', icon: 'alert-circle' },
         { id: 'nav_stalled', type: 'object', objectName: 'duly_task', viewName: 'stalled', label: 'Not moving', icon: 'pause-circle' },
         { id: 'nav_assignments', type: 'object', objectName: 'duly_assignment', viewName: 'sent_by_me', label: 'Assignments', icon: 'send' },

src/data/demo-catalog.ts

@@ -381,6 +381,19 @@ export interface DemoDuty {
   owner: string;
   source: 'catalog' | 'assigned' | 'self';
   status?: 'active' | 'paused' | 'retired';
+  /**
+   * Where this duty sits in the review pipeline TODAY (#107). Omitted means
+   * `approved` — the steady state of a duty that is actually being worked,
+   * and the only state that dispatches.
+   *
+   * The fixture keeps a handful of rows off `approved` on purpose: an empty
+   * "To confirm" and an empty "To review" are two screens a demo cannot show,
+   * and "unapproved duties do not dispatch" is not a claim anybody can check
+   * against a fixture where every duty is approved.
+   */
+  review?: 'to_confirm' | 'to_review' | 'approved' | 'returned';
+  /** Mandatory on a `returned` row — `returned_needs_note` refuses one without it. */
+  reviewNote?: string;
   /** Self-declared duties carry their own cadence — there is no catalog row behind them. */
   own?: Partial<DemoCatalogItem> & { form: Form };
 }
@@ -403,14 +416,24 @@ export const DUTIES: readonly DemoDuty[] = [
   { name: 'Emissions return — Northgate', item: 'Emissions return', owner: ADMIN, source: 'catalog' },
   { name: 'Waste transfer log review — Northgate', item: 'Waste transfer log review', owner: ADMIN, source: 'catalog' },
   { name: 'Permit condition review — Northgate', item: 'Permit condition review', owner: ADMIN, source: 'catalog' },
-  { name: 'Site environmental audit — Northgate', item: 'Site environmental audit', owner: ADMIN, source: 'catalog' },
+  // Waiting on the evaluator's OWN confirmation, so the pipeline is
+  // clickable on the first record page they open. Its in-flight task stays
+  // where it is — returning or un-confirming a duty stops the NEXT run, it
+  // does not retract work already dispatched.
+  { name: 'Site environmental audit — Northgate', item: 'Site environmental audit', owner: ADMIN, source: 'catalog', review: 'to_confirm' },
   { name: 'Keep the permit register current — Northgate', item: 'Keep the permit register current', owner: ADMIN, source: 'catalog' },
   {
     name: 'Keep up with regulator bulletins',
     item: null,
     owner: ADMIN,
     source: 'self',
     own: { form: 'recurring', frequency: 'monthly', dueAnchor: 'period_start', dueOffsetDays: 7, leadDays: 7, graceDays: 0, description: 'Read the month\'s bulletins and note anything that changes what the site owes.' },
+    // The returned one. Owned by the evaluator, so `returned → to_review` —
+    // the leg only the OWNER may walk — is demonstrable without a second
+    // login. It is also the state that stops dispatch, which is the most
+    // persuasive thing this card has to show.
+    review: 'returned',
+    reviewNote: 'Reading the bulletins is not the duty — the duty is recording what changed and who has to act. Rewrite the acceptance bar and send it back.',
   },
 
   // ── Northgate Quality ─────────────────────────────────────────────────
@@ -422,12 +445,20 @@ export const DUTIES: readonly DemoDuty[] = [
     owner: 'Priya Raman',
     source: 'self',
     own: { form: 'recurring', frequency: 'monthly', dueAnchor: 'period_start', dueOffsetDays: 5, leadDays: 7, graceDays: 0, description: 'Half an hour with the month\'s nonconformances and calibration deviations, looking for the shape rather than the individual events.' },
+    // Self-declared duties are born `to_review`, not `to_confirm` — writing
+    // one down IS the confirmation. This row is what that default looks like
+    // on screen, and it gives the reviewer's queue a second entry that is not
+    // a catalog duty.
+    review: 'to_review',
   },
   { name: 'Calibration verification — Lab 1', item: 'Calibration verification', owner: 'Rosa Delgado', source: 'catalog' },
-  { name: 'Retained sample review — Lab 1', item: 'Retained sample review', owner: 'Rosa Delgado', source: 'catalog' },
+  // A second unconfirmed list, on somebody else — so a manager opening
+  // "To review" sees a queue rather than one row, and so "To confirm" is
+  // visibly a per-person list rather than a global one.
+  { name: 'Retained sample review — Lab 1', item: 'Retained sample review', owner: 'Rosa Delgado', source: 'catalog', review: 'to_confirm' },
   { name: 'Nonconformance log review — Northgate Quality', item: 'Nonconformance log review', owner: 'Rosa Delgado', source: 'catalog' },
   { name: 'Calibration verification — Lab 2', item: 'Calibration verification', owner: 'Ibrahim Chaudhry', source: 'catalog' },
-  { name: 'Instrument drift check — Lab 2', item: 'Instrument drift check', owner: 'Ibrahim Chaudhry', source: 'catalog' },
+  { name: 'Instrument drift check — Lab 2', item: 'Instrument drift check', owner: 'Ibrahim Chaudhry', source: 'catalog', review: 'to_review' },
   {
     name: 'Track my own training hours',
     item: null,

src/data/demo-history.ts

@@ -89,6 +89,20 @@ export const DISPATCH_DUTIES: readonly DispatchDuty[] = DUTIES.map((duty) => {
     owner: duty.owner,
     business_unit: unit,
     source: duty.source,
+    // ── Always `approved`, whatever the duty's review state is TODAY ──────
+    // The planner refuses an unapproved duty (`not_approved`), so passing
+    // each row's real review state would delete the history of every duty
+    // this fixture leaves mid-pipeline — including the two the "Late" and
+    // "Not moving" stories are told with.
+    //
+    // That would also be the wrong history. `review_status` is a fact about
+    // now, not about the six months behind it: those tasks WERE dispatched,
+    // which is precisely why returning a duty stops the next run rather than
+    // retracting the work already owed (pinned in `test/dispatch.test.ts`).
+    // A duty sitting in `to_confirm` with tasks behind it is the import case
+    // this card exists for — the work was always being done; the list is only
+    // now being formalised.
+    review_status: 'approved',
     frequency: cadence.frequency ?? null,
     due_anchor: cadence.due_anchor ?? null,
     due_offset_days: cadence.due_offset_days ?? null,

src/data/duty.seed.ts

@@ -35,6 +35,12 @@ import { HISTORY_FROM } from './demo-history.js';
  *    obligation, and the duty stays on screen.
  *  - **One one-off**, dispatched by hand rather than by the scheduler. Its
  *    single task is seeded directly in `task.seed.ts`.
+ *  - **Five duties off `approved`** (#107) — two waiting on their owner to
+ *    confirm, two waiting on a reviewer, one returned with a reason. They hold
+ *    zero NEW tasks for a third reason (`not_approved`) while keeping the
+ *    history they were dispatched with, which is the import case the review
+ *    pipeline exists for. Two of the five are the demo account's own, so the
+ *    pipeline can be walked on a record page without a second login.
  */
 
 /** A self-declared duty has no catalog row behind it, so it carries its own cadence. */
@@ -65,6 +71,22 @@ export const dutySeed = defineSeed(Duty, {
       // exercises the zone handling rather than leaving every row on UTC.
       timezone: timezoneOf(unit),
       status: duty.status ?? 'active',
+      // ── Review state: the seed states it, and states it on every row ──────
+      // Not left to the field default, for the same reason `source` is not
+      // (see the ⚠️ above): the default keys on `source`, so leaving it out
+      // would silently put every self-declared duty in `to_review` and every
+      // catalog duty in `to_confirm` — a demo where nothing dispatches and
+      // the dashboard reads zero, with nothing erroring anywhere.
+      //
+      // The seed loader writes with `seedReplay: true`, which is what lets it
+      // land `approved` and `returned` rows directly: the platform skips
+      // `state_machine` rules on that context, so the fixture does not have
+      // to walk 30 duties through the pipeline one legal transition at a
+      // time. `returned_needs_note` is an ordinary script rule and is NOT
+      // skipped — a returned row without a reason would be refused, which is
+      // why `reviewNote` is required on that shape in `demo-catalog.ts`.
+      review_status: duty.review ?? 'approved',
+      ...(duty.reviewNote ? { review_note: duty.reviewNote } : {}),
       // The history this seed backfills starts here, so the duties say so.
       // Without it the effective window is open-ended and a later backfill
       // would happily invent obligations that predate the demo.

src/jobs/dispatch.job.ts

@@ -5,6 +5,7 @@ import { defineJob } from '@objectstack/spec';
 import {
   DISPATCH_DUTY_FIELDS,
   DISPATCHABLE_FORM,
+  DISPATCHABLE_REVIEW_STATUS,
   DISPATCHABLE_STATUS,
   FAULT_SKIP_REASONS,
   nextDispatchedPeriod,
@@ -300,22 +301,30 @@ export async function runDispatch(engine: DispatchEngine, options: DispatchOptio
 }
 
 /**
- * Every active recurring duty, paged.
+ * Every active, approved, recurring duty, paged.
  *
- * The filter is deliberately only the two zone-independent facts. The
+ * The filter is deliberately only the three zone-independent facts. The
  * effective-window test is NOT pushed into the query: `effective_from` and
  * `effective_to` are calendar days in the DUTY's own zone, and "today" is a
  * different day in Auckland and in Los Angeles at the same instant, so a single
  * SQL predicate cannot be right for every row it matches. The window test
  * belongs where the zone is known, which is the planner.
+ *
+ * `review_status` is pushed down here AND checked again in the planner, and
+ * that is not a redundant belt: this query is one of two callers. A backfill
+ * driven straight through `planDispatch` — and every planner test — never
+ * passes this filter at all, so the planner's own `not_approved` check is what
+ * makes the rule true for both. What the filter adds is that the nightly sweep
+ * does not read, page and walk every unapproved duty in the tenant to discard
+ * them one at a time.
  */
 async function readDispatchableDuties(engine: DispatchEngine): Promise<DispatchDuty[]> {
   const duties: DispatchDuty[] = [];
   for (let offset = 0; ; offset += DISPATCH_PAGE_SIZE) {
     const page = (await engine.find(
       'duly_duty',
       {
-        where: { status: DISPATCHABLE_STATUS, form: DISPATCHABLE_FORM },
+        where: { status: DISPATCHABLE_STATUS, form: DISPATCHABLE_FORM, review_status: DISPATCHABLE_REVIEW_STATUS },
         fields: [...DISPATCH_DUTY_FIELDS],
         // A stable total order, or paging re-reads and skips rows.
         orderBy: [{ field: 'id', order: 'asc' }],

src/jobs/dispatch.plan.ts

@@ -82,6 +82,21 @@ export const DEFAULT_GRACE_DAYS = 0;
 export const DISPATCHABLE_STATUS = 'active';
 /** The form a duty must hold to dispatch. `standing` and `one_off` never do. */
 export const DISPATCHABLE_FORM = 'recurring';
+/**
+ * The review state a duty must hold to dispatch.
+ *
+ * An unconfirmed, unreviewed or RETURNED duty produces nothing — the day a
+ * manager returns it, tomorrow's tasks stop. That is the product behaviour the
+ * review pipeline exists for, and it is the one place it has teeth.
+ *
+ * The test is `=== 'approved'`, deliberately, rather than "not returned": a
+ * row that carries no review state at all (one that predates the column, or
+ * arrived through a path that skipped the default) is NOT dispatched. Between
+ * "an obligation nobody approved goes out" and "an obligation stops until
+ * somebody looks at it", the second is the recoverable failure, and it is
+ * visible in the run's `skipped` list rather than silent.
+ */
+export const DISPATCHABLE_REVIEW_STATUS = 'approved';
 
 /**
  * The `duly_duty` projection the planner reads.
@@ -100,6 +115,12 @@ export const DISPATCH_DUTY_FIELDS = [
   'owner',
   'business_unit',
   'source',
+  // The dispatch gate (`not_approved`). Read even though the job's own query
+  // already filters on it: `planDispatch` is also driven directly — by a
+  // backfill, and by every planner test — and a projection that omitted this
+  // would hand the planner `undefined` and dispatch an unapproved duty with
+  // nothing erroring anywhere.
+  'review_status',
   'frequency',
   'due_anchor',
   'due_offset_days',
@@ -127,6 +148,7 @@ export interface DispatchDuty {
   owner?: string | null;
   business_unit?: string | null;
   source?: string | null;
+  review_status?: string | null;
   frequency?: string | null;
   due_anchor?: string | null;
   due_offset_days?: number | null;
@@ -171,6 +193,12 @@ export type DispatchSkipReason =
   | 'unknown_form'
   /** `paused` or `retired`. */
   | 'not_active'
+  /**
+   * Not `approved` — awaiting confirmation, awaiting review, returned, or
+   * carrying no review state at all. An ordinary, expected skip: the duty is
+   * fine, it just has not been signed off, so it is NOT a fault reason.
+   */
+  | 'not_approved'
   /** Recurring with no cadence. `recurring_needs_frequency` should prevent it. */
   | 'no_frequency'
   /** A `frequency` value outside `FREQUENCIES`. */
@@ -347,6 +375,9 @@ function planForDuty(duty: DispatchDuty, now: Date, window: BackfillWindow | nul
   if (duty.form === 'one_off') return { reason: 'one_off' };
   if (duty.form !== DISPATCHABLE_FORM) return { reason: 'unknown_form' };
   if (duty.status !== DISPATCHABLE_STATUS) return { reason: 'not_active' };
+  // Third invariant, and the newest: an obligation nobody has approved is not
+  // yet an obligation anybody owes.
+  if (duty.review_status !== DISPATCHABLE_REVIEW_STATUS) return { reason: 'not_approved' };
 
   if (duty.frequency == null || duty.frequency === '') return { reason: 'no_frequency' };
   if (!isFrequency(duty.frequency)) return { reason: 'unknown_frequency' };