From e55491ad092211867aba7d009d1c1d6d2eb6557f Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Mon, 17 Aug 2026 23:14:21 -0400 Subject: [PATCH 01/23] chore(porch): 1450 init pir --- .../status.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml diff --git a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml new file mode 100644 index 000000000..6affd36d7 --- /dev/null +++ b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml @@ -0,0 +1,18 @@ +id: '1450' +title: dashboard-make-the-held-mail-c +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-08-18T03:14:21.212Z' +updated_at: '2026-08-18T03:14:21.213Z' From b6c15774e8bddeee4f402ce80987ca76b5ded170 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Mon, 17 Aug 2026 23:18:42 -0400 Subject: [PATCH 02/23] [PIR #1450] Plan draft Co-Authored-By: Claude Opus 5 (1M context) --- .../1450-dashboard-make-the-held-mail-c.md | 233 ++++++++++++++++++ codev/state/pir-1450_thread.md | 64 +++++ 2 files changed, 297 insertions(+) create mode 100644 codev/plans/1450-dashboard-make-the-held-mail-c.md create mode 100644 codev/state/pir-1450_thread.md diff --git a/codev/plans/1450-dashboard-make-the-held-mail-c.md b/codev/plans/1450-dashboard-make-the-held-mail-c.md new file mode 100644 index 000000000..e8ec31383 --- /dev/null +++ b/codev/plans/1450-dashboard-make-the-held-mail-c.md @@ -0,0 +1,233 @@ +# PIR Plan: Clickable held-mail counter with a held-messages popover + +Issue: [#1450](https://github.com/cluesmith/codev/issues/1450) — *Dashboard: make the held-mail +counter clickable, showing the held messages (from → to)* + +## Understanding + +The dashboard header renders a count-only held-mail indicator. `HeldCountBadge` +(`apps/web/src/components/HeldCountBadge.tsx:31-40`) is an inert ``: a dot, the text +`N held`, and a `title` tooltip whose only remedy is *"Review with: afx inbox"*. It is mounted +once, in the desktop header (`apps/web/src/components/App.tsx:360`), fed by +`OverviewData.heldCount` / `mailboxEscalated`. + +So the user sees "2 held" and must drop to a terminal to learn *who is held from whom*. The ask +is two things: (1) an affordance that reads as clickable, and (2) a panel on click listing each +held message with at least `from → to`. + +### Verifying the issue's claims against current `main` + +The issue was written 2026-08-17; I re-checked every claim against the tree at +`origin/main` (`9129ab81c`): + +| Issue claim | Verdict | +|---|---| +| The counter is inert text | **True.** `HeldCountBadge.tsx:31-40` — a `` with a `title`, no handler, no affordance. Last touched by Spec 1313's `0bbea9de4`; nothing since. | +| Finding out *what* is held needs `afx inbox -w ` | **True.** No dashboard surface lists held rows. | +| "Data is already available server-side (the same store `afx inbox` reads)" | **True.** `GET /api/inbox` → `handleInboxList` (`tower-routes.ts:199`, impl `:2134`) projects exactly the CLI's fields: `id`, `workspacePath`, `toAgent`, `fromAgent`, `reason`, `escalated`, `createdAt`, `notBefore`. | +| "a small read endpoint/**reuse of an existing one**" | **Needs one correction.** `GET /api/inbox` is registered only on the *Tower-level* route table. The dashboard is served under `/workspace//` and calls its API with relative `./api/...` (`getApiBase()` returns `'./'`, `apps/web/src/lib/constants.ts`), which lands in the **workspace-scoped** dispatcher (`tower-routes.ts:2484-2723`). That dispatcher has no `inbox` branch, so `./api/inbox` currently 404s. The fix is a three-line branch that reuses `handleInboxList` — the same pattern `overview`, `analytics`, and `architects/:name` already use. No new handler, no new projection. | + +One more fact that shapes the design: **the dashboard does not know its own workspace path.** +It never reads the encoded prefix out of `window.location` for API purposes — the server +resolves the workspace from the URL prefix. That is why calling the Tower-level +`/api/inbox?workspace=` from the browser is not an option, and why the workspace-scoped +branch (which passes `workspacePath` as an override) is the right seam. + +## Proposed Change + +### Server — reuse `handleInboxList` under the workspace prefix + +Give `handleInboxList` an optional `workspaceOverride` third parameter, exactly mirroring +`handleOverview(res, url, workspaceOverride?, ctx?)` (`tower-routes.ts:1108-1110`) and +`handleAnalytics` (`:1382`). Resolution order becomes: explicit override → `?workspace=` → +all workspaces. The Tower-level registration is unchanged, so `afx inbox` and any direct caller +keep their exact current semantics. + +Then add one branch to the workspace-scoped API dispatcher, next to the existing `overview` one: + +```ts +// GET /api/inbox — held mailbox rows for THIS workspace (Issue 1450). Reuses the +// Tower-level handler with the workspace resolved from the /workspace// prefix, +// the same way `overview` and `analytics` do. Metadata-only projection: never bodies. +if (req.method === 'GET' && apiPath === 'inbox') { + return handleInboxList(res, url, workspacePath); +} +``` + +Deliberately **exact-match `'inbox'` only**: `inbox/:id` (show, carries the body) and +`inbox/:id/dismiss` (mutating) stay off the dashboard surface and keep 404-ing there. That +preserves two Spec 1313 rules the current badge docstring already states — the redaction rule +(bodies never leave the CLI/terminal path) and decision 8 (dismissal is CLI-only; this surface +never mutates state). `afx inbox show ` remains the deep-dive path, as the issue itself +allows. + +### Types — one shared projection type + +Add `HeldMessage` to `packages/types/src/api.ts` describing the `handleInboxList` projection. +Both sides of the server/client boundary may import `codev-types` (arch invariant #1189), so the +web app gets the shape without reaching into `codev-core`. The CLI's private `InboxRow` +(`commands/inbox.ts:21-35`) is left alone — retyping it is a separate cleanup, not this issue. + +### Web — the badge becomes a disclosure button with a popover + +`HeldCountBadge` keeps its presentational character (its docstring calls that out explicitly, and +it is why the component unit-tests in isolation), but gains disclosure state and a **`loadMessages` +loader prop** rather than importing `fetchInbox` directly. Tests inject a fake loader; `App.tsx` +passes the real one. New shape: + +```ts +export interface HeldCountBadgeProps { + count: number; + escalated: boolean; + /** Fetches the workspace's held rows. Called lazily, on each open. */ + loadMessages: () => Promise; +} +``` + +Behaviour: + +- **Affordance.** The `` becomes a ` + , + ); + + await open(); + fireEvent.mouseDown(screen.getByText('elsewhere')); + + expect(screen.queryByTestId('held-popover')).toBeNull(); + }); + + it('stays open when clicking inside the panel', async () => { + render( Promise.resolve([row()])} />); + const panel = await open(); + + fireEvent.mouseDown(panel); + + expect(screen.getByTestId('held-popover')).toBeTruthy(); + }); + + it('detaches its document listeners on unmount', async () => { + const removeSpy = vi.spyOn(document, 'removeEventListener'); + const { unmount } = render( + Promise.resolve([row()])} />, + ); + await open(); + unmount(); + + const removed = removeSpy.mock.calls.map((c) => c[0]); + expect(removed).toContain('keydown'); + expect(removed).toContain('mousedown'); + removeSpy.mockRestore(); + }); }); diff --git a/apps/web/__tests__/heldMail.test.ts b/apps/web/__tests__/heldMail.test.ts new file mode 100644 index 000000000..13a73fca9 --- /dev/null +++ b/apps/web/__tests__/heldMail.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from 'vitest'; +import { formatHeldAge, formatHeldDuration, isScheduled } from '../src/lib/heldMail.js'; + +describe('formatHeldDuration', () => { + it('renders sub-minute deltas in seconds', () => { + expect(formatHeldDuration(0)).toBe('0s'); + expect(formatHeldDuration(5_000)).toBe('5s'); + expect(formatHeldDuration(59_000)).toBe('59s'); + }); + + it('rolls over to minutes at 60s', () => { + expect(formatHeldDuration(60_000)).toBe('1m'); + expect(formatHeldDuration(59 * 60_000)).toBe('59m'); + }); + + it('rolls over to hours at 60m', () => { + expect(formatHeldDuration(60 * 60_000)).toBe('1h'); + expect(formatHeldDuration(23 * 60 * 60_000)).toBe('23h'); + }); + + it('rolls over to days at 24h', () => { + expect(formatHeldDuration(24 * 60 * 60_000)).toBe('1d'); + expect(formatHeldDuration(3 * 24 * 60 * 60_000)).toBe('3d'); + }); + + it('clamps a negative delta to 0s rather than rendering "-1s"', () => { + // Clock skew between the server's createdAt and the browser's Date.now() is real; + // a negative age must not leak into the UI. + expect(formatHeldDuration(-5_000)).toBe('0s'); + }); +}); + +describe('formatHeldAge', () => { + it('is the delta between now and createdAt', () => { + expect(formatHeldAge(1_000_000, 1_000_000 + 90_000)).toBe('1m'); + }); +}); + +describe('isScheduled', () => { + const now = 1_000_000; + + it('is false for a deliver-ASAP row (null notBefore)', () => { + expect(isScheduled(null, now)).toBe(false); + }); + + it('is false for a row whose due time has passed', () => { + expect(isScheduled(now - 1, now)).toBe(false); + }); + + it('is false exactly at the due time (matches the SQL boundary not_before <= now)', () => { + // The count query uses `not_before <= now`, so a row due exactly now IS eligible and + // IS counted. This predicate must agree, or the Held group would drift from the badge. + expect(isScheduled(now, now)).toBe(false); + }); + + it('is true for a pre-due row', () => { + expect(isScheduled(now + 15_000, now)).toBe(true); + }); +}); diff --git a/apps/web/src/components/App.tsx b/apps/web/src/components/App.tsx index 1f50e8310..e5de5ca4d 100644 --- a/apps/web/src/components/App.tsx +++ b/apps/web/src/components/App.tsx @@ -4,7 +4,7 @@ import { useTabs, type Tab } from '../hooks/useTabs.js'; import { useMediaQuery } from '../hooks/useMediaQuery.js'; import { useOverview } from '../hooks/useOverview.js'; import { MOBILE_BREAKPOINT } from '../lib/constants.js'; -import { getTerminalWsPath, createFileTab, removeArchitect as removeArchitectApi } from '../lib/api.js'; +import { getTerminalWsPath, createFileTab, fetchInbox, removeArchitect as removeArchitectApi } from '../lib/api.js'; import { readActiveArchitect, writeActiveArchitect } from '../lib/architectPersistence.js'; import { SplitPane } from './SplitPane.js'; import { TabBar } from './TabBar.js'; @@ -357,7 +357,11 @@ export function App() { {overviewTitle}
- + {state?.version && v{state.version}}
diff --git a/apps/web/src/components/HeldCountBadge.tsx b/apps/web/src/components/HeldCountBadge.tsx index e107b8fed..81b919153 100644 --- a/apps/web/src/components/HeldCountBadge.tsx +++ b/apps/web/src/components/HeldCountBadge.tsx @@ -1,41 +1,212 @@ /** - * Spec 1313 Phase 8: compact held-mail count indicator for the dashboard header. + * Spec 1313 Phase 8 / Issue 1450: the dashboard header's held-mail indicator. * - * Read-only and count-only. It renders the number of currently-*held* (undelivered) - * mailbox rows in the workspace, fed by `OverviewData.heldCount` (which the overview - * refetches live on the `overview-changed` broadcast). When at least one held row has - * crossed the escalation age (`OverviewData.mailboxEscalated`) the badge enters an - * attention state — a pulsing amber dot — and clears back to normal when the row - * resolves. Dismissal stays CLI-only (`afx inbox`); this surface never mutates state - * (spec Decision 8). Renders nothing when the count is zero, so it stays out of the - * way until there is held mail. + * Renders the number of currently-*held* (undelivered) mailbox rows in the workspace, fed by + * `OverviewData.heldCount` (which the overview refetches live on the `overview-changed` + * broadcast). When at least one held row has crossed the escalation age + * (`OverviewData.mailboxEscalated`) the badge enters an attention state — a pulsing amber dot — + * and clears back to normal when the row resolves. Renders nothing when the count is zero, so + * it stays out of the way until there is held mail. * - * Presentational only (takes its data as props) so it unit-tests in isolation, mirroring - * `CloudStatus`. + * Issue 1450 made it a **disclosure button**: clicking it opens a panel listing each held + * message as `from → to` with its age and why-held reason, so "2 held" stops sending the user + * to a terminal to find out who is stuck. Still strictly READ-ONLY — dismissal remains CLI-only + * (`afx inbox dismiss`, spec Decision 8), and the list carries no message bodies (the redaction + * rule; `afx inbox show ` is the body path). + * + * ## Held vs Scheduled — why the panel groups rows + * + * The badge count and the list come from DIFFERENT queries and legitimately disagree. + * `heldSummaryForWorkspace` (the count) requires `not_before IS NULL OR not_before <= now`, so a + * pre-due `--delay` send is "scheduled, not stuck" and does not inflate the attention count. + * `listHeld` (behind `GET /api/inbox`) has no such filter and returns every held row. So + * `count <= messages.length`, always. + * + * Rather than hide that, the panel groups: **Held (N)** — where N is exactly the badge count — + * and a secondary **Scheduled (M)** section for pre-due rows, with their due countdown. Each + * group renders only when non-empty, so the ordinary case (no `--delay` in flight) looks like a + * single plain list. This mirrors `afx inbox`, which lists both and labels the pre-due ones + * `scheduled`. + * + * Consequence worth knowing: with 0 due and 1 scheduled row the badge does not render at all, + * so a scheduled-only state is invisible here. That is the existing contract — the badge is an + * *attention* indicator — and `afx inbox` remains the surface that sees it. + * + * Presentational: it takes a `loadMessages` loader rather than importing `fetchInbox`, so it + * unit-tests in isolation with a fake loader, mirroring `CloudStatus`. */ +import { useCallback, useEffect, useId, useRef, useState } from 'react'; +import type { HeldMessage } from '../lib/api.js'; +import { formatHeldAge, formatHeldDuration, isScheduled } from '../lib/heldMail.js'; + export interface HeldCountBadgeProps { - /** Count of currently-held rows across the workspace (`OverviewData.heldCount`). */ + /** Count of currently-held ELIGIBLE rows across the workspace (`OverviewData.heldCount`). */ count: number; /** True when at least one held row has crossed the escalation age. */ escalated: boolean; + /** + * Fetches the workspace's held rows. Called on open, and again whenever `count` changes + * while open. Injected so the component stays presentational and testable. + */ + loadMessages: () => Promise; } -export function HeldCountBadge({ count, escalated }: HeldCountBadgeProps) { - if (count <= 0) { +/** What the panel is currently showing. */ +type LoadState = + | { kind: 'loading' } + | { kind: 'error'; message: string } + | { kind: 'ready'; messages: HeldMessage[] }; + +function HeldRow({ message, now }: { message: HeldMessage; now: number }) { + const scheduled = isScheduled(message.notBefore, now); + // `?` for a missing sender matches how `afx inbox` renders the same row. + const fromTo = `${message.fromAgent ?? '?'} → ${message.toAgent}`; + // A scheduled row shows its countdown to due time; a stuck one shows how long it has waited. + const when = scheduled + ? `→${formatHeldDuration(message.notBefore! - now)}` + : formatHeldAge(message.createdAt, now); + const reason = scheduled ? 'scheduled' : (message.reason ?? 'held'); + return ( +
  • + {fromTo} + + {when} · {reason} + {message.escalated && !scheduled ? '!' : ''} + +
    • + ); +} + +export function HeldCountBadge({ count, escalated, loadMessages }: HeldCountBadgeProps) { + const [open, setOpen] = useState(false); + const [state, setState] = useState({ kind: 'loading' }); + const buttonRef = useRef(null); + const wrapperRef = useRef(null); + const panelId = useId(); + + // Discards a response whose request is no longer the current one. Without this, a fast + // open → close → open lands the FIRST (slower) response over the second's data; React 19 + // does not warn about setState on an unmounted/stale path, so the bug would be silent. + const generationRef = useRef(0); + + const load = useCallback(() => { + const generation = ++generationRef.current; + setState({ kind: 'loading' }); + loadMessages().then( + (messages) => { + if (generationRef.current !== generation) return; + setState({ kind: 'ready', messages }); + }, + (err: unknown) => { + if (generationRef.current !== generation) return; + setState({ kind: 'error', message: err instanceof Error ? err.message : String(err) }); + }, + ); + }, [loadMessages]); + + // Load on open, and again when `count` changes while open. Refetch rather than snapshot: + // the count is SSE-driven, so a change means the mailbox actually moved — precisely when a + // user staring at the open panel would expect it to follow. + useEffect(() => { + if (!open) return; + load(); + }, [open, count, load]); + + // Escape closes and returns focus to the button (the disclosure pattern's keyboard contract). + // Click-outside closes without moving focus, since the user is already looking elsewhere. + useEffect(() => { + if (!open) return; + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + setOpen(false); + buttonRef.current?.focus(); + } + }; + const onPointerDown = (e: MouseEvent) => { + if (!wrapperRef.current?.contains(e.target as Node)) setOpen(false); + }; + document.addEventListener('keydown', onKeyDown); + document.addEventListener('mousedown', onPointerDown); + return () => { + document.removeEventListener('keydown', onKeyDown); + document.removeEventListener('mousedown', onPointerDown); + }; + }, [open]); + + // Normally the badge disappears at zero. But `useOverview` polls every 2.5s, so the last + // held row being delivered WHILE the panel is open would unmount the button mid-interaction + // and drop focus to . So when open, stay mounted and say the mail cleared; the user's + // own close unmounts us. The closed-at-zero contract is unchanged. + if (count <= 0 && !open) { return null; } + const label = `${count} held`; const title = escalated - ? `${count} held message${count === 1 ? '' : 's'} — at least one past the escalation age. Review with: afx inbox` - : `${count} held message${count === 1 ? '' : 's'} awaiting a clear prompt. Review with: afx inbox`; + ? `${count} held message${count === 1 ? '' : 's'} — at least one past the escalation age. Click to list them.` + : `${count} held message${count === 1 ? '' : 's'} awaiting a clear prompt. Click to list them.`; + + // `now` is sampled per render rather than ticked on a timer: the panel is a triage glance, + // and every refetch re-renders anyway. A live-ticking age would be motion for its own sake. + const now = Date.now(); + const messages = state.kind === 'ready' ? state.messages : []; + const heldRows = messages.filter((m) => !isScheduled(m.notBefore, now)); + const scheduledRows = messages.filter((m) => isScheduled(m.notBefore, now)); + return ( - - - {label} - +
    + + {open && ( +
    + {state.kind === 'loading' &&

    Loading…

    } + {state.kind === 'error' && ( +

    + Could not load held messages: {state.message} +

    + )} + {state.kind === 'ready' && messages.length === 0 && ( +

    + {count <= 0 ? 'Held mail cleared.' : 'No held messages.'} +

    + )} + {heldRows.length > 0 && ( +
    +

    Held ({heldRows.length})

    +
      + {heldRows.map((m) => ( + + ))} +
    +
    + )} + {scheduledRows.length > 0 && ( +
    +

    Scheduled ({scheduledRows.length})

    +

    + Waiting for their due time — not counted above. +

    +
      + {scheduledRows.map((m) => ( + + ))} +
    +
    + )} +

    Dismiss with afx inbox dismiss <id>

    +
    + )} +
    ); } diff --git a/apps/web/src/index.css b/apps/web/src/index.css index ffbcc68b6..27b22b731 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -894,6 +894,13 @@ body { /* Spec 1313 Phase 8: held-mail count indicator in the dashboard header. Count-only, read-only; enters an attention state (amber pulse, reusing @keyframes cloud-pulse) when a held row has crossed the escalation age. */ +/* Issue 1450: the badge is a disclosure button anchoring a popover. `.header-controls` is a + plain flex row with no positioning context, so the wrapper provides one. */ +.held-badge-wrapper { + position: relative; + display: inline-flex; +} + .held-badge { display: inline-flex; align-items: center; @@ -901,6 +908,26 @@ body { font-size: 12px; color: var(--text-secondary); white-space: nowrap; + /* Button reset — this was a until Issue 1450. */ + background: none; + border: none; + padding: 0; + font-family: inherit; + cursor: pointer; + /* The affordance: dotted underline reads as "expands to reveal", not as a link. */ + text-decoration: underline dotted; + text-underline-offset: 3px; +} + +.held-badge:hover { + color: var(--text-primary); + text-decoration: underline solid; +} + +.held-badge:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; + border-radius: 3px; } .held-badge--attention { @@ -908,6 +935,104 @@ body { font-weight: 600; } +/* The popover sits at the 1000 tier (with the modal backdrop): xterm's WebGL/canvas panes + paint over anything at the lower 20/100 tiers. */ +.held-popover { + position: absolute; + top: calc(100% + 8px); + right: 0; + z-index: 1000; + min-width: 280px; + max-width: 380px; + max-height: 320px; + overflow-y: auto; + padding: 10px 12px; + background: var(--bg-secondary); + border: 1px solid var(--border-color); + border-radius: 6px; + box-shadow: 0 8px 24px rgba(0, 0, 0, 0.35); + font-size: 12px; + text-align: left; + cursor: default; +} + +.held-popover-note { + margin: 0; + color: var(--text-secondary); +} + +.held-popover-note--error { + color: var(--status-error); +} + +.held-group + .held-group { + margin-top: 12px; + padding-top: 10px; + border-top: 1px solid var(--border-color); +} + +.held-group-title { + margin: 0 0 6px; + font-size: 11px; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.04em; + color: var(--text-muted); +} + +.held-group-note { + margin: -2px 0 6px; + font-size: 11px; + font-style: italic; + color: var(--text-muted); +} + +.held-list { + margin: 0; + padding: 0; + list-style: none; +} + +.held-row { + display: flex; + flex-direction: column; + gap: 1px; + padding: 4px 0; +} + +.held-row + .held-row { + border-top: 1px solid var(--border-color); +} + +.held-row-addresses { + color: var(--text-primary); + font-family: ui-monospace, monospace; + word-break: break-word; +} + +.held-row-meta { + font-size: 11px; + color: var(--text-muted); +} + +.held-row--attention .held-row-meta { + color: var(--status-waiting); + font-weight: 600; +} + +.held-popover-foot { + margin: 10px 0 0; + padding-top: 8px; + border-top: 1px solid var(--border-color); + font-size: 11px; + color: var(--text-muted); +} + +.held-popover-foot code { + font-family: ui-monospace, monospace; + font-size: 11px; +} + .held-dot { display: inline-block; width: 8px; diff --git a/apps/web/src/lib/api.ts b/apps/web/src/lib/api.ts index 7a2787997..25cd04dbb 100644 --- a/apps/web/src/lib/api.ts +++ b/apps/web/src/lib/api.ts @@ -91,6 +91,7 @@ export type { OverviewBacklogItem, OverviewRecentlyClosed, OverviewData, + HeldMessage, ProtocolStats, AnalyticsResponse, } from '@cluesmith/codev-types'; @@ -100,6 +101,7 @@ import type { AnalyticsResponse, TeamApiResponse, OverviewData, + HeldMessage, DashboardState, } from '@cluesmith/codev-types'; @@ -123,6 +125,23 @@ export async function fetchOverview(): Promise { return res.json(); } +/** + * Issue 1450: the workspace's currently-held mailbox rows — the payload behind the + * held-mail popover, and the same projection `afx inbox` renders. + * + * Hits the workspace-scoped `GET /api/inbox`, which resolves the workspace server-side from + * the `/workspace//` URL prefix (the dashboard has no absolute workspace path of its + * own). Metadata only — never message bodies. + * + * Called lazily, on popover open, rather than folded into the 2.5s overview poll: this is a + * cold path that only matters when the user asks. + */ +export async function fetchInbox(): Promise { + const res = await fetch(apiUrl('api/inbox'), { headers: getAuthHeaders() }); + if (!res.ok) throw new Error(`Failed to fetch held messages: ${res.status}`); + return res.json(); +} + export async function refreshOverview(): Promise { await fetch(apiUrl('api/overview/refresh'), { method: 'POST', diff --git a/apps/web/src/lib/heldMail.ts b/apps/web/src/lib/heldMail.ts new file mode 100644 index 000000000..a06d5e769 --- /dev/null +++ b/apps/web/src/lib/heldMail.ts @@ -0,0 +1,48 @@ +/** + * Issue 1450: age/countdown formatting for the dashboard's held-mail popover. + * + * Ported — deliberately, not imported — from the `afx inbox` CLI renderer + * (`packages/codev/src/agent-farm/commands/inbox.ts:77-90`). The web app must not import + * from `@cluesmith/codev-core` (server/client isolation, #1189), and `@cluesmith/codev-types` + * is a types-only devDependency, the wrong home for a runtime helper. Ten lines duplicated + * beats a boundary violation; keeping the output identical to the CLI's is what lets a + * reviewer check the popover against `afx inbox` row for row. + * + * Named `formatHeldAge` rather than `formatDuration` because + * `apps/web/src/lib/open-files-shells-utils.ts` already exports a `formatDuration` with + * DIFFERENT semantics (minute granularity, `<1m` floor) and existing callers. Two + * same-named formatters with different output in one `lib/` is a trap. + */ + +/** + * Compact human duration ("5s", "3m", "2h", "1d") from a millisecond delta. + * Second-granularity, matching the CLI — held mail is often seconds old, and "<1m" + * would erase the distinction between "just held" and "held for most of a minute". + * Negative deltas clamp to `0s`. + */ +export function formatHeldDuration(ms: number): string { + const secs = Math.max(0, Math.floor(ms / 1000)); + if (secs < 60) return `${secs}s`; + const mins = Math.floor(secs / 60); + if (mins < 60) return `${mins}m`; + const hours = Math.floor(mins / 60); + if (hours < 24) return `${hours}h`; + return `${Math.floor(hours / 24)}d`; +} + +/** Compact human age ("5s", "3m", "2h", "1d") from an epoch-ms timestamp. */ +export function formatHeldAge(createdAt: number, now: number): string { + return formatHeldDuration(now - createdAt); +} + +/** + * Is this row a pre-due `--delay` send — scheduled rather than stuck? + * + * The single predicate that splits the popover's two groups, and the same test the CLI + * applies (`inbox.ts:135`). A scheduled row is excluded from `OverviewData.heldCount` by + * `heldSummaryForWorkspace`, so grouping on exactly this predicate is what makes the + * "Held" group's length equal the badge count. + */ +export function isScheduled(notBefore: number | null, now: number): boolean { + return notBefore != null && notBefore > now; +} diff --git a/codev/state/pir-1450_thread.md b/codev/state/pir-1450_thread.md index ce05065b9..ea5b98c7e 100644 --- a/codev/state/pir-1450_thread.md +++ b/codev/state/pir-1450_thread.md @@ -124,3 +124,74 @@ argument was weak — real reasons are cached-aggregate cost + VSCode overview f Open note back to architect: couldn't find pir-1365's artifacts in this worktree, so the isolated-Tower recipe is reconstructed from the CLI rather than copied from that precedent. + +**Resolved:** architect pointed at `spec-1365-e2e-evidence.mts`. pir-1365 did NOT redirect HOME — +it spawns the built `tower-server.js` with `NODE_ENV=test` + `AF_TEST_DB`, the dedicated seam at +`db/index.ts:117-127`. Strictly better than my HOME idea (isolates the db without touching the +environment's home), so the plan was amended before implementing. Plan gate approved on rev 2. + +## Implement phase (2026-08-18) + +Shipped. Server + types + web + tests + a real-browser evidence script. + +### Shape of the change + +- `handleInboxList(res, url, workspaceOverride?)` — `??` ordering so the override WINS over + `?workspace=`; the Tower-level registration is untouched. +- New `apiPath === 'inbox'` GET branch in the workspace-scoped dispatcher. **Exact match**, so + `inbox/:id` (body) and `inbox/:id/dismiss` (mutating) fall through to 404 and stay off the + dashboard. Three tests pin that non-reachability — it's what the redaction argument rests on. +- `HeldMessage` in `packages/types`, with the Held-vs-Scheduled asymmetry documented ON the type + and a cross-reference added to `OverviewData.heldCount`. The next person to wire a UI to these + two fields shouldn't have to rediscover it from SQL. +- Badge → disclosure button (dotted underline, `aria-expanded` + `aria-controls`, real `
      `), + generation-guarded lazy fetch, grouped popover, stays mounted while open at count 0. + +### Things worth recording + +- **`user-event` is not a dependency here.** I wrote the first test pass against it out of habit; + the repo convention is `fireEvent` from `@testing-library/react`. Rewrote rather than add a + devDependency for one test file. +- **A `cd` in a Bash call persists across calls.** I cd'd into `apps/web/src` for a sed-style + edit and then spent two tool calls confused about why `apps/web/__tests__` "didn't exist". + Use absolute paths or cd back in the same command. +- **Nearly wrote to main's tree.** An Edit call with a path missing the `.builders/pir-1450/` + segment was blocked by the guard. The nesting hazard in the role doc is real. +- My first `heldCount` docstring edit spliced a note into the MIDDLE of the neighbouring + `queuedFeedback` comment, breaking its sentence. Caught on reread and moved. + +### Evidence (the part that actually proves this works) + +`packages/codev/scripts/issue-1450-dashboard-evidence.mts` — 23/23 checks, committed. + +Isolated Tower on **14700**, `NODE_ENV=test` + `AF_TEST_DB=test-1450-14700.db`, so the cohort's +live `global.db` is never touched and no second delivery loop runs against their held mail. +Real workspace, real shellper PTYs painted with an occupied composer, real `POST /api/send` +held by the render gate, real built SPA in real Chromium. + +The run produced the exact scenario the blocking finding predicted: **badge says "2 held" while +the mailbox has 3 rows**, and the popover renders `Held (2)` + `Scheduled (1)`. The +`heldRows === badgeCount` assertion is in the script. + +`playwright-core` isn't a repo dependency — installed out-of-tree in the scratchpad and passed +via `PW_CORE`/`PW_CHROMIUM` rather than adding a heavy devDependency for one script. + +Two flaws in my *own* evidence surfaced and were fixed rather than papered over: +1. The z-index check used `querySelector('.xterm')`, which returned the LEFT pane's architect + terminal — never overlapping a top-right popover. The check passed while proving nothing. + Now it opens a builder terminal in the right pane, checks every mounted `.xterm`, and asserts + a terminal genuinely overlaps before testing what paints on top. +2. Panel text was read before the lazy fetch resolved, so it was asserting against "Loading…". + +Also: `waitUntil: 'networkidle'` can never fire on this dashboard — the SSE stream stays open +for the page's lifetime. Used `domcontentloaded`. + +### Test results + +- Full `pnpm test` (@cluesmith/codev): **4916 passed**, 48 skipped, 0 failures. +- `apps/web`: **371 passed**, 1 skipped, 0 failures (33 files). Note the root `test` script only + runs the codev package — web tests need `pnpm --filter @cluesmith/codev-web test`. +- New: 20 web component tests, 4 formatter tests, 13 route tests. +- No pre-existing failures encountered, so nothing to quarantine. + +Awaiting `dev-approval`. PR gets parked open at the end — maintainer merges. diff --git a/packages/codev/scripts/issue-1450-dashboard-evidence.mts b/packages/codev/scripts/issue-1450-dashboard-evidence.mts new file mode 100644 index 000000000..4d56050ee --- /dev/null +++ b/packages/codev/scripts/issue-1450-dashboard-evidence.mts @@ -0,0 +1,370 @@ +/** + * Issue #1450 — dev-approval evidence for the clickable held-mail counter. + * + * This is a DASHBOARD change, so "tests pass" is not evidence: the affordance, the popover, + * its grouping and its stacking only exist in a browser. This script produces that browser + * run against a real, ISOLATED Tower and saves screenshots. + * + * ## Why a second Tower, and why it is safe + * + * `afx dev` deliberately reuses the live Tower's ports, and restarting the live Tower kills + * every builder session — neither is acceptable from inside a builder worktree. So this + * follows the pattern `send-integration.e2e.test.ts` and pir-1365's evidence script use: + * spawn THIS worktree's built `tower-server.js` on a private port with + * `NODE_ENV=test` + `AF_TEST_DB`, which redirects the mailbox to its own db file inside + * `~/.agent-farm/` (`db/index.ts` getGlobalDbPath). That isolation is load-bearing, not + * tidiness: a second Tower reading the real `global.db` would run its own mailbox-delivery + * loop against the cohort's live held mail. The shared Tower on 4100 is never touched. + * + * ## What is real here + * + * Nothing about the path under test is stubbed. Real Tower process, real SQLite mailbox, + * real `POST /api/send` going through the render gate (which HOLDS, because the recipient's + * composer is painted occupied), real `GET /workspace//api/inbox` — the route this + * issue adds — and the real built SPA driven by a real Chromium. + * + * The `GET .../api/inbox` response status is asserted to be 200. That assertion is the point: + * the route is key-authenticated, and a 401 renders in the popover as a tidy error state that + * looks like a working UI. "The panel showed something" is not evidence. + * + * ## Running it + * + * pnpm build + * node --experimental-strip-types packages/codev/scripts/issue-1450-dashboard-evidence.mts + * + * Needs a Playwright browser driver. `playwright-core` is not a dependency of this repo (it + * would be a heavy devDependency for one script), so point PW_CORE at an installed copy and + * PW_CHROMIUM at a browser binary: + * + * npm install playwright-core --prefix /tmp/pw + * PW_CORE=/tmp/pw/node_modules/playwright-core \ + * PW_CHROMIUM=~/.cache/ms-playwright/chromium-*\/chrome-linux64/chrome \ + * node --experimental-strip-types packages/codev/scripts/issue-1450-dashboard-evidence.mts + */ + +import { spawn, type ChildProcess } from 'node:child_process'; +import { resolve } from 'node:path'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync } from 'node:fs'; +import { homedir, tmpdir } from 'node:os'; +import net from 'node:net'; + +const PORT = 14700; // private to this script (14500 / 14600 / 14650 are taken) +const BASE = `http://localhost:${PORT}`; +const TOWER = resolve(import.meta.dirname, '../dist/agent-farm/servers/tower-server.js'); +// Defaults outside the repo so a bare run cannot leave untracked PNGs in the working tree. +const SHOTS = process.env.SHOT_DIR || resolve(tmpdir(), 'codev-evidence-1450'); + +const ESC = '\x1b'; +const RULE = '─'.repeat(22); +const CLEAR = `${ESC}[2J${ESC}[H`; +/** An OCCUPIED composer: a draft at normal intensity → the render gate HOLDS the send. */ +const DRAFT_COMPOSER = `${CLEAR}❯ ${ESC}[0mdeploy the hotfix to prod\r\n${RULE}\r\n`; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +let failures = 0; +let checks = 0; +function check(ok: boolean, label: string, detail = ''): void { + checks++; + if (!ok) failures++; + console.log(` ${ok ? 'PASS' : 'FAIL'} ${label}${detail ? ` — ${detail}` : ''}`); +} +function section(title: string): void { + console.log(`\n${'='.repeat(78)}\n${title}\n${'='.repeat(78)}`); +} + +/** The shared local key Tower expects on API calls (advisory GHSA-xvjp-7748-v88v). */ +const KEY = readFileSync(resolve(homedir(), '.agent-farm', 'local-key'), 'utf-8').trim(); +const AUTH = { 'codev-tower-key': KEY }; + +// ---------------------------------------------------------------- Tower lifecycle + +async function portListening(port: number): Promise { + return new Promise((r) => { + const s = new net.Socket(); + s.setTimeout(1000); + s.on('connect', () => { s.destroy(); r(true); }); + s.on('timeout', () => { s.destroy(); r(false); }); + s.on('error', () => r(false)); + s.connect(port, '127.0.0.1'); + }); +} + +async function startTower(): Promise { + if (!existsSync(TOWER)) throw new Error(`Tower build missing at ${TOWER} — run \`pnpm build\` first.`); + const proc = spawn('node', [TOWER, String(PORT)], { + stdio: ['ignore', 'pipe', 'pipe'], + // THE isolation seam — see the header. Without AF_TEST_DB this would attach to the + // cohort's live global.db and start delivering their held mail. + env: { ...process.env, NODE_ENV: 'test', AF_TEST_DB: `test-1450-${PORT}.db` }, + }); + let stderr = ''; + proc.stderr?.on('data', (d: Buffer) => (stderr += d.toString())); + for (let i = 0; i < 75; i++) { + if (await portListening(PORT)) return proc; + await sleep(200); + } + proc.kill(); + throw new Error(`Tower did not start on ${PORT}. stderr:\n${stderr}`); +} + +async function stopTower(proc: ChildProcess | null): Promise { + if (!proc) return; + proc.kill('SIGTERM'); + await new Promise((r) => { + proc.on('exit', () => r()); + setTimeout(() => { proc.kill('SIGKILL'); r(); }, 3000); + }); +} + +// ---------------------------------------------------------------- workspace + terminals + +const enc = (p: string) => Buffer.from(p).toString('base64url'); + +function makeWorkspace(): string { + const base = resolve(homedir(), '.agent-farm', 'test-workspaces'); + mkdirSync(base, { recursive: true }); + const ws = mkdtempSync(resolve(base, 'issue1450-')); + for (const d of ['codev', '.agent-farm', '.codev']) mkdirSync(resolve(ws, d), { recursive: true }); + writeFileSync( + resolve(ws, '.codev', 'config.json'), + JSON.stringify({ shell: { architect: 'sh -c "sleep 3600"', builder: 'bash', shell: 'bash' } }), + ); + // Lets `resolveProfileForSession` recover a harness for the wrapped launch, so sends are + // held for `busy` (an occupied composer) rather than short-circuiting on `no-profile`. + writeFileSync(resolve(ws, '.builder-start.sh'), '#!/usr/bin/env bash\nexec claude --dangerously-skip-permissions\n'); + return ws; +} + +async function activate(ws: string): Promise { + for (let i = 0; i < 30; i++) { + const res = await fetch(`${BASE}/api/workspaces/${enc(ws)}/activate`, { method: 'POST', headers: AUTH }); + if (res.ok) break; + await sleep(500); + } + for (let i = 0; i < 60; i++) { + const list = await (await fetch(`${BASE}/api/workspaces`, { headers: AUTH })).json(); + if (list.workspaces?.some((w: { path: string }) => w.path === ws)) return; + await sleep(500); + } + throw new Error('workspace never activated'); +} + +/** A real shellper-backed PTY that echoes its input, so a composer can be painted onto it. */ +async function registerEchoTerminal(ws: string, roleId: string): Promise { + const res = await fetch(`${BASE}/api/terminals`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...AUTH }, + body: JSON.stringify({ + command: 'sh', + args: ['-c', 'stty raw -echo 2>/dev/null; exec cat'], + cwd: ws, cols: 110, rows: 32, + workspacePath: ws, type: 'builder', roleId, persistent: true, + }), + }); + if (res.status !== 201) throw new Error(`terminal register failed for ${roleId}: ${res.status}`); + return (await res.json()).id; +} + +async function paint(terminalId: string, screen: string): Promise { + await fetch(`${BASE}/api/terminals/${terminalId}/write`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...AUTH }, + body: JSON.stringify({ data: screen }), + }); + await sleep(250); +} + +async function send(ws: string, to: string, message: string, options: Record = {}) { + const res = await fetch(`${BASE}/api/send`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...AUTH }, + body: JSON.stringify({ to, workspace: ws, from: 'architect', message, options }), + }); + return { status: res.status, body: await res.json().catch(() => ({})) }; +} + +// ---------------------------------------------------------------- main + +async function main(): Promise { + mkdirSync(SHOTS, { recursive: true }); + + // playwright-core is CJS: an ESM `import()` of it puts the exports on `.default`, and a + // bare directory path needs its entry file spelled out. Accept either shape so PW_CORE can + // be a package name, a directory, or a file. + const pwCore = process.env.PW_CORE || 'playwright-core'; + const candidates = pwCore.endsWith('.js') || !pwCore.startsWith('/') + ? [pwCore] + : [`${pwCore}/index.js`, pwCore]; + let chromium: any; + for (const candidate of candidates) { + try { + const mod: any = await import(candidate); + chromium = mod.chromium ?? mod.default?.chromium; + if (chromium) break; + } catch { /* try the next shape */ } + } + if (!chromium) { + console.error( + `\nCould not load playwright-core from "${pwCore}".\n` + + `Install it out-of-tree and set PW_CORE / PW_CHROMIUM — see this file's header.\n`, + ); + process.exit(2); + } + + let tower: ChildProcess | null = null; + try { + section('SETUP — isolated Tower, workspace, held mail'); + tower = await startTower(); + check(true, `Tower up on ${PORT} with AF_TEST_DB=test-1450-${PORT}.db (live global.db untouched)`); + + const ws = makeWorkspace(); + await activate(ws); + check(true, `workspace activated`, ws); + + // Two recipients with OCCUPIED composers → the gate holds every send. + const cost = await registerEchoTerminal(ws, 'cost'); + const docs = await registerEchoTerminal(ws, 'docs'); + await paint(cost, DRAFT_COMPOSER); + await paint(docs, DRAFT_COMPOSER); + + const r1 = await send(ws, 'cost', 'the cost report needs a second look'); + const r2 = await send(ws, 'docs', 'please refresh the install docs'); + // A pre-due --delay row: scheduled, NOT counted by heldCount. This is the row that makes + // the badge count and the list length disagree, which the popover has to group. + const r3 = await send(ws, 'cost', 'nightly summary', { deliverAfter: 3600 }); + + check(r1.body.held === true, 'send → cost was HELD (occupied composer)', String(r1.body.reason ?? '')); + check(r2.body.held === true, 'send → docs was HELD', String(r2.body.reason ?? '')); + check(r3.status === 200, 'delayed send accepted', `status ${r3.status}`); + + const inbox = await (await fetch(`${BASE}/api/inbox?workspace=${encodeURIComponent(ws)}`, { headers: AUTH })).json(); + console.log(` inbox rows: ${JSON.stringify(inbox.map((r: any) => `${r.fromAgent}→${r.toAgent}${r.notBefore ? ' (scheduled)' : ''}`))}`); + check(inbox.length >= 2, 'held rows are in the mailbox', `${inbox.length} rows`); + + section('BROWSER — the real built SPA in Chromium'); + const browser = await chromium.launch({ + headless: true, + ...(process.env.PW_CHROMIUM ? { executablePath: process.env.PW_CHROMIUM } : {}), + }); + const page = await browser.newPage({ viewport: { width: 1280, height: 800 } }); + + // Capture the status of the route this issue adds. A 401 renders as a benign-looking + // error state, so the status is the assertion that matters. + const inboxStatuses: number[] = []; + page.on('response', (res) => { + if (new URL(res.url()).pathname.endsWith('/api/inbox')) inboxStatuses.push(res.status()); + }); + const consoleErrors: string[] = []; + page.on('console', (m) => { if (m.type() === 'error') consoleErrors.push(m.text()); }); + + // NOT `networkidle`: the dashboard holds an SSE stream (/api/events) open for its + // lifetime, so the network never goes idle and the wait would always time out. + await page.goto(`${BASE}/workspace/${enc(ws)}/`, { waitUntil: 'domcontentloaded' }); + + // 1 — the counter, closed. The affordance must be visible without interacting. + const badge = page.getByTestId('held-badge'); + await badge.waitFor({ state: 'visible', timeout: 20_000 }); + check(true, 'held badge is rendered', await badge.textContent() ?? ''); + const decoration = await badge.evaluate((el) => getComputedStyle(el).textDecorationLine); + check(decoration.includes('underline'), 'counter is underlined (reads as clickable)', decoration); + check(await badge.evaluate((el) => el.tagName) === 'BUTTON', 'counter is a real + {/* aria-live: the rows arrive asynchronously after the panel opens, so without it a + screen reader announces an empty container and never mentions the messages. */} {open && ( -
      +
      {state.kind === 'loading' &&

      Loading…

      } {state.kind === 'error' && (

      Could not load held messages: {state.message}

      )} - {state.kind === 'ready' && messages.length === 0 && ( + {/* Keyed on heldRows, not messages: when the held rows drain but a SCHEDULED row + remains, the panel is not empty yet there is still nothing held — and the + Scheduled group's "not counted above" needs something above it to refer to. */} + {state.kind === 'ready' && heldRows.length === 0 && (

      - {count <= 0 ? 'Held mail cleared.' : 'No held messages.'} + {count <= 0 && messages.length > 0 ? 'No held messages — the rows below are scheduled.' + : count <= 0 ? 'Held mail cleared.' + : 'No held messages.'}

      )} {heldRows.length > 0 && ( @@ -204,7 +241,10 @@ export function HeldCountBadge({ count, escalated, loadMessages }: HeldCountBadg
    )} -

    Dismiss with afx inbox dismiss <id>

    + {/* Points at `afx inbox` rather than `afx inbox dismiss `: dismissal needs a row + id, and this panel deliberately shows none (a full uuid would dominate the row + and this surface never mutates anyway). `afx inbox` is where the ids live. */} +

    Ids and dismissal: afx inbox

    )} diff --git a/codev/resources/arch.md b/codev/resources/arch.md index 8756aa032..8bc80d928 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -1839,7 +1839,9 @@ rather than reconcile it — `afx inbox` labels pre-due rows `scheduled`, and th groups `Held (N)` (N === the badge count) above a separate `Scheduled (M)`. The corollary is that with 0 due and 1 scheduled row the badge does not render at all, so a scheduled-only state is visible **only** in `afx inbox`; changing that means changing what the badge counts, not how the -list is filtered. Terminal rows (delivered/superseded/dismissed) are pruned after `mailbox.retentionDays` (default 30) by the drainer; **held rows are never pruned**. Cron delivers through the same gate via `deliverCronMessage` (`cron-delivery.ts`) with a per-task supersede key (a newer run replaces the older *held* row) and logs the real outcome. +list is filtered. + +Terminal rows (delivered/superseded/dismissed) are pruned after `mailbox.retentionDays` (default 30) by the drainer; **held rows are never pruned**. Cron delivers through the same gate via `deliverCronMessage` (`cron-delivery.ts`) with a per-task supersede key (a newer run replaces the older *held* row) and logs the real outcome. #### Address Resolution diff --git a/codev/reviews/1450-dashboard-make-the-held-mail-c.md b/codev/reviews/1450-dashboard-make-the-held-mail-c.md index 245c38b43..dc0621abb 100644 --- a/codev/reviews/1450-dashboard-make-the-held-mail-c.md +++ b/codev/reviews/1450-dashboard-make-the-held-mail-c.md @@ -24,9 +24,9 @@ The one genuinely subtle part is that **the badge count and the list disagree by - `packages/codev/src/agent-farm/servers/tower-routes.ts` (+30 / -3) — `workspaceOverride` param + workspace-scoped branch - `packages/types/src/api.ts` (+46 / -0) — `HeldMessage` - `packages/types/src/index.ts` (+1 / -0) — export it -- `apps/web/__tests__/HeldCountBadge.test.tsx` (+386 / -…) — 20 new cases, 5 originals kept unchanged -- `apps/web/__tests__/heldMail.test.ts` (+59 / -0) — new -- `packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts` (+168 / -0) — 13 new route cases +- `apps/web/__tests__/HeldCountBadge.test.tsx` — 32 cases total: 5 originals kept unchanged, **27 new** +- `apps/web/__tests__/heldMail.test.ts` — new, **10 cases** +- `packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts` — **12 new** route cases (14 → 26 in the file) - `packages/codev/scripts/issue-1450-dashboard-evidence.mts` (+370 / -0) — new; real-browser evidence harness - `codev/plans/1450-dashboard-make-the-held-mail-c.md`, `codev/state/pir-1450_thread.md`, `codev/resources/arch.md`, `codev/resources/lessons-learned.md` — artifacts and governance @@ -41,8 +41,9 @@ The one genuinely subtle part is that **the badge count and the list disagree by - `pnpm build`: ✓ pass - `pnpm test` (`@cluesmith/codev`): ✓ 4916 passed, 48 skipped, **0 failures** -- `apps/web` (`pnpm --filter @cluesmith/codev-web test`): ✓ 371 passed, 1 skipped, 33 files — **37 new tests** +- `apps/web` (`pnpm --filter @cluesmith/codev-web test`): ✓ 33 files — **37 new web tests** (27 component + 10 formatter) - Note: the root `test` script runs only the `codev` package; web tests need the filtered command. +- **49 new tests total** across the three files (27 + 10 + 12). - **Manual, real browser**: `packages/codev/scripts/issue-1450-dashboard-evidence.mts` — 23/23 checks in headless Chromium against an isolated Tower (worktree build, port 14700, `NODE_ENV=test` + `AF_TEST_DB`). Real workspace, real shellper PTYs painted with an occupied composer, real @@ -158,9 +159,31 @@ announced inconsistently, and this panel should not steal focus from the termina - On a normally-running dashboard, the union of both groups matches `afx inbox -w ` row for row, and the Held group alone matches the badge count. +## Post-Consultation Fixes + +The 3-way consult and the architect's integration review both landed as non-blocking +(APPROVE / COMMENT / APPROVE). Every finding was verified against the files before acting; all +were real and all are fixed in `dd7a6b1` (see the PR's commit list). + +| Finding | Source | Disposition | +|---|---|---| +| `handleInboxList`'s docstring claimed an empty `workspaceOverride` stays scoped, but `rawWorkspace ? … : undefined` widened it to **all workspaces** | Codex + architect (3) | **Real.** Unreachable today (the dispatcher 400s a missing/relative prefix first), but the comment promised a guarantee the code did not make. Made the code true rather than weakening the comment: a scoped call with a blank override now scopes to `''`, which matches no rows. The safe failure for a scoped call is zero rows, never every row. | +| Review file's test counts were wrong | Codex | **Real.** Recounted from the merge-base: **27** new component + **10** formatter + **12** route = **49**, not the 37 originally claimed. Corrected above. | +| `count → 0` with scheduled rows left the panel with no "cleared" notice, and `Scheduled`'s "not counted above" had nothing above it | Codex | **Real.** The notice now keys on `heldRows.length === 0` rather than `messages.length === 0`, with copy that distinguishes "cleared" from "nothing held, rows below are scheduled". | +| arch.md edit swallowed the pre-existing pruning/cron sentences into the new count-vs-list paragraph | Claude + architect (2) | **Real** — content survived but read as part of the Held/Scheduled discussion. Paragraph break restored. (Second splice of this kind in this project; the first was caught in `packages/types/src/api.ts` before commit.) | +| `loadMessages` prop identity drove the refetch effect — an inline lambda from a future caller would loop | Claude + architect (4) | **Real footgun.** Latched in a ref; `load` now has empty deps, so behaviour no longer depends on a caller remembering to memoize. | +| Popover lacked `aria-live`, so asynchronously-loaded rows were never announced | Claude | **Real.** Added `aria-live="polite"` + `aria-busy`. | +| Footer said `afx inbox dismiss ` but no id is rendered anywhere | Claude | **Real.** Reworded to `Ids and dismissal: afx inbox`. Rendering a full uuid per row would dominate the row, and this surface never mutates. | +| Server projection was untyped — `HeldMessage` was client-side decoration only | architect (1) | **Real.** Annotated `const projected: HeldMessage[]`, so a drifting projection (dropped field, or a `body` slipping in) fails the server build. | +| Keep prior rows during refetch instead of blanking to "Loading…" | architect (5), optional | **Taken.** A refetch fires on every `count` change while open; flashing the list away is the worst moment to do it. `aria-busy` carries the in-flight state; only a cold open shows the spinner. An **error** still replaces the rows — once a refetch fails, the old list is no longer known to be current. | +| `HeldMessage` jsdoc typo `/api/inbox:id` | architect (6) | **Not reproduced.** The jsdoc already reads `GET /api/inbox/:id` (`packages/types/src/api.ts:597`). No change made. | + +Re-verified after the fixes: `pnpm build` ✓, web suite ✓ (373 passed), route suite ✓ (26 passed), +and the browser evidence re-run ✓ **23/23**. + ## Flaky Tests -None. No pre-existing failures were encountered in the full suite (4916 passed), so nothing was +None. No pre-existing failures were encountered in the full suite, so nothing was skipped or quarantined. ## Scope Note diff --git a/codev/state/pir-1450_thread.md b/codev/state/pir-1450_thread.md index cd39ff184..b880170d3 100644 --- a/codev/state/pir-1450_thread.md +++ b/codev/state/pir-1450_thread.md @@ -234,5 +234,48 @@ and nothing here justifies displacing an existing entry: wait on loaded content, and assert the status code because auth failures render as tidy error states. -Next: open the PR, record with porch, `porch done` triggers the single 3-way consult, then the -`pr` gate. **Park it open — never merge, never close #1450.** We are not maintainers here. +PR #1510 opened and recorded with porch. `Fixes #1450` kept per architect (matches program +precedent and the #1483 two-phase signal — the issue closing exactly at maintainer merge is the +intended semantics). + +## Consult round (2026-08-18) — APPROVE / COMMENT / APPROVE, all non-blocking + +Gemini APPROVE, Codex COMMENT, Claude APPROVE; plus the architect's own integration review +(COMMENT). Nine distinct findings, **every one real**, all fixed. Verified each against the files +before acting rather than taking the summaries at face value — which mattered, because the one I +could *not* reproduce was the architect's jsdoc typo (`/api/inbox:id`); the file already read +`/api/inbox/:id`, so I reported "not reproduced" instead of making a cosmetic no-op edit. + +The two that actually mattered: + +1. **`handleInboxList` doc/code disagreement (Codex + architect).** My docstring bragged that an + empty `workspaceOverride` stays scoped. It didn't — `rawWorkspace ? … : undefined` turned it + into an all-workspaces query. Unreachable today (the dispatcher 400s a missing prefix), but I + had written a security guarantee the code did not make. Fixed by making the code true rather + than softening the comment: a scoped call with a blank override scopes to `''`, matching no + rows. Needed a narrow `export` of the handler to test it, since the branch is unreachable + through `handleRequest` — documented as a test seam at the export. +2. **My review file's test counts were wrong.** Claimed 20 new component tests / 37 total; the + real numbers from the merge-base are 27 + 10 + 12 = **49**. I had eyeballed rather than + counted. Corrected. A retrospective is durable team knowledge — wrong numbers in it are worse + than no numbers. + +Also fixed: arch.md paragraph splice (the pruning/cron sentences got glued onto my new +paragraph — **second splice of this kind this project**, after the api.ts docstring; I should +reread the surrounding block after every insertion into prose, not just after code edits); +`loadMessages` identity driving the refetch effect (latched in a ref, so an inline lambda from a +future caller can't cause a refetch loop); missing `aria-live`; a footer naming an id the panel +never renders; the untyped server projection (now `const projected: HeldMessage[]`, so drift +fails the build); and blanking to "Loading…" on refetch (now keeps rows, `aria-busy` carries the +in-flight state — but an *error* still replaces them, because a failed refetch means the old list +is no longer known to be current). + +One self-inflicted detour: my `aria-live` edit put a JSX comment as a sibling expression inside +`{open && ( … )}`, which doesn't parse. Caught by the build immediately. + +Re-verified after all fixes: build ✓, full suite **4917 passed / 0 failures**, web 373, routes 26, +and the **browser evidence re-run 23/23** — the component changed, so re-running it was not +optional. + +Sitting at the `pr` gate. **Park it open — never merge, never close #1450.** We are not +maintainers here. diff --git a/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts b/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts index f9dd6fb16..fe7fb00a4 100644 --- a/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts +++ b/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts @@ -13,7 +13,7 @@ import { EventEmitter } from 'node:events'; import Database from 'better-sqlite3'; import { GLOBAL_SCHEMA } from '../db/schema.js'; import * as mailbox from '../db/mailbox.js'; -import { handleRequest } from '../servers/tower-routes.js'; +import { handleRequest, handleInboxList } from '../servers/tower-routes.js'; import type { RouteContext } from '../servers/tower-routes.js'; // The one db seam tower-routes uses: return a real in-memory DB, reseeded per test. @@ -409,6 +409,19 @@ describe('GET /workspace//api/inbox (Issue 1450)', () => { expect(rows[0].workspacePath).toBe(WS); }); + it('an EMPTY override scopes to nothing rather than widening to every workspace', async () => { + // Defensive: the dispatcher 400s a missing prefix, so a blank override is unreachable + // from the route today. But "unreachable" is a property of the caller, and the safe + // failure for a scoped call is zero rows, never every workspace's held mail. + seedHeld(); + seedHeld({ workspacePath: '/home/user/other-project', toAgent: 'other-1' }); + + const res = makeRes(); + handleInboxList(res, new URL('http://localhost/api/inbox'), ''); + + expect(JSON.parse(res._body)).toEqual([]); + }); + it('lists pre-due --delay rows too, so the popover can group them as scheduled', async () => { // This is the asymmetry Issue 1450's popover has to render: `listHeld` (this route) has // no not_before filter, while `heldSummaryForWorkspace` (the badge count) does. The diff --git a/packages/codev/src/agent-farm/servers/tower-routes.ts b/packages/codev/src/agent-farm/servers/tower-routes.ts index 01492805e..49f889d7e 100644 --- a/packages/codev/src/agent-farm/servers/tower-routes.ts +++ b/packages/codev/src/agent-farm/servers/tower-routes.ts @@ -27,7 +27,7 @@ import { version } from '../../version.js'; const execAsync = promisify(exec); import type { SessionManager } from '../../terminal/session-manager.js'; import type { PtySessionInfo } from '../../terminal/pty-session.js'; -import type { BuilderSpawnedPayload, DashboardState, ArchitectState, TowerVersionInfo } from '@cluesmith/codev-types'; +import type { BuilderSpawnedPayload, DashboardState, ArchitectState, TowerVersionInfo, HeldMessage } from '@cluesmith/codev-types'; import { getBuilders, setArchitectByName } from '../state.js'; import { DEFAULT_COLS, defaultSessionOptions } from '../../terminal/index.js'; import type { SSEClient, WorkspaceTerminals } from './tower-types.js'; @@ -2134,23 +2134,43 @@ async function handleSend( * Issue 1450: `workspaceOverride` lets the workspace-scoped route * (`/workspace//api/inbox`, which backs the dashboard's held-mail popover) pass the * workspace resolved from the URL prefix, exactly as `handleOverview` / `handleAnalytics` do. - * The override WINS over `?workspace=` (`??`, so even an empty-string override is honored - * rather than falling through) — a workspace-scoped call must never be redirected to another - * workspace's held mail by a query parameter. The override arrives already normalized by the - * prefix decoder, so `normalizeWorkspacePath` re-running on it is a safe no-op. + * The override arrives already normalized by the prefix decoder, so `normalizeWorkspacePath` + * re-running on it is a safe no-op. + * + * Two separate widening hazards are closed here, because a scoped caller must never receive + * another workspace's held mail: + * - `??` (not `||`) means `?workspace=` can never take effect once an override was passed. + * - An EMPTY override does not fall through to the unscoped all-workspaces listing; it + * scopes to a path that matches nothing. Today the dispatcher 400s a missing prefix so + * this is unreachable, but "unreachable" is a property of the caller, not of this + * function, and the safe failure for a scoped call is zero rows, never every row. + * The all-workspaces listing therefore remains reachable only when NO override was passed — + * the Tower-level route without `?workspace=`, the direct-caller convenience the CLI never uses. * * NOTE: this lists ALL held rows, including pre-due `--delay` rows, while the badge count * (`heldSummaryForWorkspace`) excludes them — see the `HeldMessage` type for why the two * legitimately disagree. */ -function handleInboxList(res: http.ServerResponse, url: URL, workspaceOverride?: string): void { +// Exported as a narrow test seam (Issue 1450): the empty-override branch below cannot be +// reached through `handleRequest` — the workspace dispatcher rejects a missing or non-absolute +// prefix with a 400 before this runs — so the only way to pin that guard's behaviour is to call +// the handler directly. Production callers go through the two route tables, not this export. +export function handleInboxList(res: http.ServerResponse, url: URL, workspaceOverride?: string): void { + const scoped = workspaceOverride !== undefined; const rawWorkspace = workspaceOverride ?? url.searchParams.get('workspace'); // Normalize to the stored realpath key (mailbox workspace_path is normalized at // enqueue — tower-routes handleSend / holdAndRespond — matching overview.ts). Without // this a symlinked workspace root would miss its own held rows. - const workspace = rawWorkspace ? normalizeWorkspacePath(rawWorkspace) : undefined; + // `scoped ? '' : undefined` on the empty branch: '' matches no workspace_path, so a scoped + // call with a blank override returns nothing rather than widening to every workspace. + const workspace = rawWorkspace + ? normalizeWorkspacePath(rawWorkspace) + : (scoped ? '' : undefined); const rows = listHeldMailbox(getGlobalDb(), workspace); - const projected = rows.map((r) => ({ + // Annotated, not inferred: `HeldMessage` is the shared contract this route owes its + // clients, so a projection that drifts from it (a dropped field, or a `body` slipping in) + // fails the server build rather than only surprising the dashboard at runtime. + const projected: HeldMessage[] = rows.map((r) => ({ id: r.id, workspacePath: r.workspace_path, toAgent: r.to_agent, From a1fb516ecf83b4b68cfd71daa47250cc3a4da2b3 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:15:17 -0400 Subject: [PATCH 16/23] [PIR #1450] Review: correct the fix-commit sha --- codev/reviews/1450-dashboard-make-the-held-mail-c.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codev/reviews/1450-dashboard-make-the-held-mail-c.md b/codev/reviews/1450-dashboard-make-the-held-mail-c.md index dc0621abb..f3e336930 100644 --- a/codev/reviews/1450-dashboard-make-the-held-mail-c.md +++ b/codev/reviews/1450-dashboard-make-the-held-mail-c.md @@ -163,7 +163,7 @@ announced inconsistently, and this panel should not steal focus from the termina The 3-way consult and the architect's integration review both landed as non-blocking (APPROVE / COMMENT / APPROVE). Every finding was verified against the files before acting; all -were real and all are fixed in `dd7a6b1` (see the PR's commit list). +were real and all are fixed in `2cb2b3623` (see the PR's commit list). | Finding | Source | Disposition | |---|---|---| From 6986d49eb156823c75d231f34107f671ba86e2f5 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:15:33 -0400 Subject: [PATCH 17/23] [PIR #1450] Review: refresh suite total and commit list --- codev/reviews/1450-dashboard-make-the-held-mail-c.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/reviews/1450-dashboard-make-the-held-mail-c.md b/codev/reviews/1450-dashboard-make-the-held-mail-c.md index f3e336930..374180a45 100644 --- a/codev/reviews/1450-dashboard-make-the-held-mail-c.md +++ b/codev/reviews/1450-dashboard-make-the-held-mail-c.md @@ -36,11 +36,13 @@ The one genuinely subtle part is that **the badge count and the list disagree by - `934e9354e` [PIR #1450] Plan revised: Held/Scheduled split, a11y disclosure pattern, isolated-Tower Playwright - `5a782c0b7` [PIR #1450] Plan: use AF_TEST_DB isolation seam for the evidence Tower (not a HOME redirect) - `d70e3503f` [PIR #1450] feat: clickable held-mail counter with a held-messages popover +- `7483bcce9` [PIR #1450] Review + retrospective +- `2cb2b3623` [PIR #1450] Address 3-way consult + architect integration review ## Test Results - `pnpm build`: ✓ pass -- `pnpm test` (`@cluesmith/codev`): ✓ 4916 passed, 48 skipped, **0 failures** +- `pnpm test` (`@cluesmith/codev`): ✓ 4917 passed, 48 skipped, **0 failures** - `apps/web` (`pnpm --filter @cluesmith/codev-web test`): ✓ 33 files — **37 new web tests** (27 component + 10 formatter) - Note: the root `test` script runs only the `codev` package; web tests need the filtered command. - **49 new tests total** across the three files (27 + 10 + 12). From 5020c35fc83a561331137f952522e36ff454ab00 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:15:40 -0400 Subject: [PATCH 18/23] chore(porch): 1450 pr gate-requested --- .../1450-dashboard-make-the-held-mail-c/status.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml index 207a3d187..900df9a4f 100644 --- a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml +++ b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml @@ -15,13 +15,15 @@ gates: approved_at: '2026-08-18T04:03:07.050Z' pr: status: pending + requested_at: '2026-08-18T04:15:40.636Z' iteration: 1 -build_complete: true +build_complete: false history: [] started_at: '2026-08-18T03:14:21.212Z' -updated_at: '2026-08-18T04:06:20.144Z' +updated_at: '2026-08-18T04:15:40.636Z' pr_history: - phase: review pr_number: 1510 branch: builder/pir-1450 created_at: '2026-08-18T04:06:13.523Z' +pr_ready_for_human: true From 8eec82743fc01d3020d93160e271f18979756126 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:17:21 -0400 Subject: [PATCH 19/23] chore(porch): 1450 pr gate-approved --- .../1450-dashboard-make-the-held-mail-c/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml index 900df9a4f..831efe1cb 100644 --- a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml +++ b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml @@ -14,16 +14,17 @@ gates: requested_at: '2026-08-18T03:50:13.800Z' approved_at: '2026-08-18T04:03:07.050Z' pr: - status: pending + status: approved requested_at: '2026-08-18T04:15:40.636Z' + approved_at: '2026-08-18T04:17:21.430Z' iteration: 1 build_complete: false history: [] started_at: '2026-08-18T03:14:21.212Z' -updated_at: '2026-08-18T04:15:40.636Z' +updated_at: '2026-08-18T04:17:21.430Z' pr_history: - phase: review pr_number: 1510 branch: builder/pir-1450 created_at: '2026-08-18T04:06:13.523Z' -pr_ready_for_human: true +pr_ready_for_human: false From b7de5213f8321b71735d9f451bf9a29020749122 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:17:28 -0400 Subject: [PATCH 20/23] chore(porch): 1450 review build-complete --- .../projects/1450-dashboard-make-the-held-mail-c/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml index 831efe1cb..da88a6550 100644 --- a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml +++ b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml @@ -18,10 +18,10 @@ gates: requested_at: '2026-08-18T04:15:40.636Z' approved_at: '2026-08-18T04:17:21.430Z' iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-08-18T03:14:21.212Z' -updated_at: '2026-08-18T04:17:21.430Z' +updated_at: '2026-08-18T04:17:28.829Z' pr_history: - phase: review pr_number: 1510 From afeec14cee8740d4fdebdd8078e808e8c126c08f Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:17:30 -0400 Subject: [PATCH 21/23] chore(porch): 1450 protocol complete --- .../projects/1450-dashboard-make-the-held-mail-c/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml index da88a6550..fb1ae39f3 100644 --- a/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml +++ b/codev/projects/1450-dashboard-make-the-held-mail-c/status.yaml @@ -1,7 +1,7 @@ id: '1450' title: dashboard-make-the-held-mail-c protocol: pir -phase: review +phase: verified plan_phases: [] current_plan_phase: null gates: @@ -21,7 +21,7 @@ iteration: 1 build_complete: true history: [] started_at: '2026-08-18T03:14:21.212Z' -updated_at: '2026-08-18T04:17:28.829Z' +updated_at: '2026-08-18T04:17:30.488Z' pr_history: - phase: review pr_number: 1510 From 5f3a710b946e84838a32211c869d0ceaf37862b7 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:17:57 -0400 Subject: [PATCH 22/23] [PIR #1450] Thread: protocol complete, PR parked for maintainer --- codev/state/pir-1450_thread.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/codev/state/pir-1450_thread.md b/codev/state/pir-1450_thread.md index b880170d3..2e84fe1bc 100644 --- a/codev/state/pir-1450_thread.md +++ b/codev/state/pir-1450_thread.md @@ -279,3 +279,29 @@ optional. Sitting at the `pr` gate. **Park it open — never merge, never close #1450.** We are not maintainers here. + +## Protocol complete (2026-08-18) + +`pr` gate approved by the human; `porch next 1450` reports `status: complete`, `phase: verified`. + +**Deliberately did NOT run the protocol's final task.** Porch's completion task says "Merge the +pull request" — that is the PIR default for a repo the cohort owns. It does not apply here: we +are contributors to cluesmith/codev, not maintainers, and the architect's standing instruction +across four separate messages was park / never merge / never close #1450. A standing instruction +from the human beats a protocol default, and merging is both irreversible and outward-facing. +`Fixes #1450` in the PR body closes the issue at the maintainer's merge, which is the intended +two-phase signal (#1483 precedent). + +Final state: PR #1510 OPEN, issue #1450 OPEN, branch `builder/pir-1450` pushed, worktree clean. +No `porch done --merged` recorded, because nothing was merged — recording it would put a false +lifecycle event in `status.yaml`. + +### What I'd tell the next builder on this code + +- The whole issue existed because `tower-routes.ts` has **two** route tables and `GET /api/inbox` + was only in the CLI-facing one. Check `handleWorkspaceRoutes` before assuming the dashboard can + reach an endpoint. Now in arch.md. +- `heldCount` and `GET /api/inbox` disagree on purpose. Don't "fix" it. Also in arch.md. +- The evidence script (`packages/codev/scripts/issue-1450-dashboard-evidence.mts`) is reusable for + any dashboard change: isolated Tower via `AF_TEST_DB`, real PTYs, real render gate, real SPA. It + needs an out-of-tree `playwright-core` via `PW_CORE`/`PW_CHROMIUM`. From e81f8223fe24157472a7c12585b92a107ccdac38 Mon Sep 17 00:00:00 2001 From: Mohid Makhdoomi Date: Tue, 18 Aug 2026 00:18:55 -0400 Subject: [PATCH 23/23] [PIR #1450] Thread: record the post-merge follow-through --- codev/state/pir-1450_thread.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/codev/state/pir-1450_thread.md b/codev/state/pir-1450_thread.md index 2e84fe1bc..9ad634a3b 100644 --- a/codev/state/pir-1450_thread.md +++ b/codev/state/pir-1450_thread.md @@ -296,6 +296,15 @@ Final state: PR #1510 OPEN, issue #1450 OPEN, branch `builder/pir-1450` pushed, No `porch done --merged` recorded, because nothing was merged — recording it would put a false lifecycle event in `status.yaml`. +### PENDING — post-merge follow-through + +Architect confirmed: leave `status.yaml` at `verified` / created-but-not-merged. **When the +maintainer merges PR #1510, run `porch done 1450 --merged 1510`** to close the lifecycle record. +Nothing else is owed. Do not merge or close anything yourself — we are contributors here, not +maintainers. + +(Recorded here rather than only in the session, so a crash-and-relaunch doesn't lose it.) + ### What I'd tell the next builder on this code - The whole issue existed because `tower-routes.ts` has **two** route tables and `GET /api/inbox`