diff --git a/data/outstanding-issues-snapshot.json b/data/outstanding-issues-snapshot.json index d82a6140a..a6477dcba 100644 --- a/data/outstanding-issues-snapshot.json +++ b/data/outstanding-issues-snapshot.json @@ -10,7 +10,7 @@ "p2": 49, "p3": 31, "queued": 7, - "pending": 4, + "pending": 5, "resolved": 431 }, "queue": [ @@ -842,6 +842,12 @@ "summary": "mode-home-page-skeleton still subtracts a chrome estimate from 100dvh, the pattern invariant 24 retired everywhere else", "created_at": "2026-08-27" }, + { + "request_id": "964e1147-56b2-4750-8794-4b8b1d8e4e04", + "action": "add", + "summary": "Ward Flow pinned-clock fix is committed only to an unpushed local branch, and the Phase 6 morning page still carries the defect, its workaround, and an untested D5 branch", + "created_at": "2026-08-27" + }, { "request_id": "af8b8fb0-3930-43a6-8d06-c897866a86b9", "action": "add", diff --git a/data/repo-awareness-snapshot.json b/data/repo-awareness-snapshot.json index 91cf04cfb..61f627c62 100644 --- a/data/repo-awareness-snapshot.json +++ b/data/repo-awareness-snapshot.json @@ -1,8 +1,8 @@ { "version": "repo-awareness-snapshot-v1", "captured_revision": { - "sha": "072aba85fedc9a237d9a18ac2b256fb6cc92cd38", - "committed_at": "2026-08-27T13:11:23+00:00" + "sha": "2717fc58379236dbc41f6755cf77d55c9f6d4b12", + "committed_at": "2026-08-27T22:51:37+08:00" }, "routes": { "modes": [ @@ -3746,6 +3746,11 @@ "section": "root", "catalogued": false }, + { + "path": "docs/ward-flow-pinned-clock-handover.md", + "section": "root", + "catalogued": false + }, { "path": "docs/ward-flow-roadmap.md", "section": "root", @@ -3842,9 +3847,9 @@ } ], "counts": { - "documents": 481, + "documents": 482, "catalogued": 107, - "uncatalogued": 374, + "uncatalogued": 375, "sections": 19 } }, diff --git a/docs/outstanding-issues-inbox/964e1147-56b2-4750-8794-4b8b1d8e4e04.json b/docs/outstanding-issues-inbox/964e1147-56b2-4750-8794-4b8b1d8e4e04.json new file mode 100644 index 000000000..c00c3c3c8 --- /dev/null +++ b/docs/outstanding-issues-inbox/964e1147-56b2-4750-8794-4b8b1d8e4e04.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "964e1147-56b2-4750-8794-4b8b1d8e4e04", + "createdOn": "2026-08-27", + "action": "add", + "payload": { + "pri": "P2", + "type": "task", + "summary": "Ward Flow pinned-clock fix is committed only to an unpushed local branch, and the Phase 6 morning page still carries the defect, its workaround, and an untested D5 branch", + "detail": "WardFlowProvider's initialNow prop silently discarded its value: the render body forced elapsed=0 when pinned, so now was always NOW_ANCHOR (642, 10:42) whatever instant was passed. Latent, never active - all 40 initialNow call sites pass NOW_ANCHOR, and the live app never passes the prop - but it made every time-of-day branch untestable through the real provider. FIXED and PROVEN in commit 62f798c2a on branch claude/serene-heyrovsky-7a5e53 (base be65b8a1b), worktree D:\\Repos\\Database\\.claude\\worktrees\\nostalgic-vaughan-7ee231: 40/40 existing call sites stayed green (92 tests), whole ward suite 519 passed, typecheck/lint/prettier pass, and a mutation test showed 3 of 4 new tests going red with 'Expected 450 / Received 642'. RISK: that branch has never been pushed, has no upstream and no PR, so the work exists on one machine only. NEXT ACTIONS, all on claude/ward-flow-phases-6-7-design (which was ACTIVELY ADVANCING when this was written - a53a3a994 to 0d29dd734 in one session, so confirm ownership before editing): (1) cherry-pick 62f798c2a or re-apply the two-line change; (2) write the spec D5 pre-08:00 test through the real provider by rendering MorningPage inside WardFlowProvider initialNow={7*60+30} - today D5 is proven at the pure-function level only, not through the rendered page; (3) remove the now-misleading NullHandoverHarness and its doc comments in tests/ward-morning-page.dom.test.tsx, which describe the defect as present-tense fact (DirectFrozenHarness may still earn its place as a unit-level check - judgement call, not a mechanical delete); (4) decide whether claude/serene-heyrovsky-7a5e53 should be closed as redundant or pushed as its own low-risk two-file PR - owner's call. Full handover with evidence, traps and do-not-reopen list: docs/ward-flow-pinned-clock-handover.md (committed 3fd861418 on the same branch, so it is also unpushed). Offline only - no provider, Supabase, OpenAI or CI access needed for any step.", + "source": "Claude Code session 2026-08-27; handover doc docs/ward-flow-pinned-clock-handover.md", + "issueUlid": "01M11QZ5X4YTR84P2K9TGQ5RQB" + } +} diff --git a/docs/ward-flow-pinned-clock-handover.md b/docs/ward-flow-pinned-clock-handover.md new file mode 100644 index 000000000..7c5ce3966 --- /dev/null +++ b/docs/ward-flow-pinned-clock-handover.md @@ -0,0 +1,190 @@ +# Ward Flow — the pinned-clock defect: session handover + +> **STATUS as at 2026-08-27: the fix is written, proven and committed — but it is committed +> ONLY to a local branch that has never been pushed, and the branch that actually needed it +> does not have it.** Nothing here is merged. Nothing here is on `main`. If this file is more +> than a few days old, re-verify every SHA and branch name below before acting on them; the +> Phase 6 branch was moving while this was written. + +**Why this file exists.** The fix was finished and then sat. It lives on a branch nobody else +can see, and the one place it was needed — the Phase 6 morning page — is on a different +branch that is still being worked. A session with no memory of any of this needs to be able +to pick it up without re-deriving it, and without re-opening the parts that are settled. + +Companion documents, none of which this file duplicates: + +| For | Read | +| ---------------------------------------- | ---------------------------------------------------------------------------- | +| The Phase 6 specification, including D5 | `docs/superpowers/specs/2026-08-27-ward-flow-phase-6-morning-page-design.md` | +| Direction, phase order, settled refusals | `docs/ward-flow-roadmap.md` | +| Phase 5's handover, for the house style | `docs/ward-flow-phase-5-handover.md` | + +The Phase 6 spec is **not on this branch** — it exists only on +`claude/ward-flow-phases-6-7-design`. Read it there +(`git show claude/ward-flow-phases-6-7-design:docs/superpowers/specs/2026-08-27-ward-flow-phase-6-morning-page-design.md`) +rather than concluding it is missing. The other two are here. + +--- + +## 1. The defect, in one paragraph + +`WardFlowProvider` takes an optional `initialNow` prop, documented as pinning the clock at +that instant so tests and deterministic renders are reproducible. **Its value was silently +discarded.** The render body set `elapsed = 0` whenever `initialNow` was defined and then +computed `now = NOW_ANCHOR + elapsed + clockOffsetMinutes`, so a pinned provider always +served `NOW_ANCHOR` (642 — 10:42) no matter what instant it was handed. The prop was doing +two real jobs — acting as a "do not tick" flag, and seeding a clock checkpoint the pinned +path never reads again — and one job it only appeared to do. + +**It was latent, not active.** Every `initialNow=` call site in the suite passes `NOW_ANCHOR`, +and `NOW_ANCHOR + 0` equals `initialNow` precisely when the pinned instant _is_ `NOW_ANCHOR`. +So no test was ever asserting against a clock it did not receive, and no screen was ever +wrong at runtime — the live app never passes the prop at all. + +**Why it still mattered.** It made every time-of-day branch in the ward screens unreachable +through the real provider. A test could not obtain any clock other than the one instant the +fixture happens to be authored around. That is what forced the workaround described in §4. + +--- + +## 2. Where the fix is + +```bash +git log --oneline -1 claude/serene-heyrovsky-7a5e53 +``` + +- **Commit:** `62f798c2a` — "Make WardFlowProvider's pinned clock actually read the instant it + was given". +- **Branch:** `claude/serene-heyrovsky-7a5e53`, based on `be65b8a1b` (`origin/main` at the + time of writing). +- **Worktree:** `D:\Repos\Database\.claude\worktrees\nostalgic-vaughan-7ee231`. +- **NOT pushed. No upstream configured. No pull request. Not on `main`.** This is the single + most important fact in this document — the work exists in exactly one place on one machine. +- **Two files changed**, nothing else: `src/components/ward-management/ward-flow-provider.tsx` + and `tests/ward-flow-provider.dom.test.tsx`. + +The change itself: when pinned, `now` derives from `initialNow + clockOffsetMinutes`. The +unpinned path — `NOW_ANCHOR` plus accumulated real elapsed time, including the midnight-wrap +accumulator added in Phase 3 — is untouched. + +--- + +## 3. What is proven, and by what evidence + +Run in this worktree on the committed tree. All offline; nothing touched a provider. + +| Check | Result | +| --------------------------------------- | ---------------------------------------------- | +| `tests/ward-flow-provider.dom.test.tsx` | 9 passed (5 pre-existing + 4 added) | +| All 17 files passing `initialNow=` | 92 passed | +| Whole ward suite (48 files) | 519 passed | +| `npm run typecheck` | pass | +| `npm run lint` (whole repo) | pass — see the trap in §6, it lied twice first | +| Prettier, both changed files | pass | + +**Blast radius, re-measured.** The original brief said 43 `initialNow=` call sites; the true +count on `be65b8a1b` is **40**, and all 40 pass `NOW_ANCHOR`. All 40 stayed green under the +fix, which is the point: a correct fix had to be a no-op for every existing test, and it was. + +**Mutation test, performed.** The fix was reverted with the new tests kept. Three of the four +new tests went red. The decisive line: + +``` +Expected element to have text content: 450 +Received: 642 +``` + +`642` is `NOW_ANCHOR` — the provider serving the hardcoded anchor while the test pinned 07:30. +The fix was then restored and the suite re-run green. The fourth new test (a pinned provider +never starts its tick interval) passes either way by design; it is a guard against the fix +costing the prop its other job, not a detector of this bug. + +--- + +## 4. What remains — the actual outstanding work + +**All of it is on a different branch.** The morning page does not exist on +`claude/serene-heyrovsky-7a5e53`, so none of this could be done there. + +**Target branch: `claude/ward-flow-phases-6-7-design`** (worktree +`D:\Worktrees\Database\pr-2390-fix`). Confirm before touching it: this branch was +**actively advancing while this document was written** — it moved from `a53a3a994` to +`0d29dd734` inside a single session, so another session is working it. Do not assume it is +idle, and do not edit it concurrently with whoever owns it. The similarly named +`claude/ward-flow-phases-6-7-4358c2` is a different branch sitting at `main` with no morning +page on it; it is not the target. + +At `0d29dd734` that branch still carries the buggy provider and the workaround below. + +- [ ] **Carry the fix across.** Cherry-pick `62f798c2a`, or re-apply the two-line change. It + is self-contained and does not depend on anything else in this branch. + +- [ ] **Write the D5 test through the real provider.** Spec D5 says that when the demo clock + is before 08:00 there is no morning handover for that day yet, so the page must say + _"The 08:00 handover has not been taken for this day"_, offer the live view, and show no + figures — never yesterday's snapshot and never a silent fall back to `now`. With the fix + in place, that branch is reachable by rendering the real `MorningPage` inside + ``. Today it is proven at the pure-function + level only, not through the rendered page. + +- [ ] **Delete the workaround it replaces.** `tests/ward-morning-page.dom.test.tsx` contains + `NullHandoverHarness` and `DirectFrozenHarness`, plus two long doc comments that exist + solely because of this defect and describe it as present-tense fact. Once the fix lands + there, those comments become actively misleading — a later reader will believe the bug + is still there and reach for the harness again. Remove the comments and whichever + harness the new test makes redundant. This is a judgement call, not a mechanical + delete: `DirectFrozenHarness` drives the _real_ `buildFrozenMorning`, so it may still + earn its place as a unit-level check even once the page-level test exists, whereas + `NullHandoverHarness` hand-authors `{ instant: null, rollup: null }` and is the one the + page-level test genuinely supersedes. + +- [ ] **Decide what happens to `claude/serene-heyrovsky-7a5e53`.** Once the fix is on the + Phase 6 branch, this branch is probably redundant and should be closed rather than left + to rot. If instead the fix should reach `main` on its own — it is a clean, low-risk, + two-file change with a mutation-proven test — then it needs a push and a pull request. + **This is the owner's call, not a session's.** + +--- + +## 5. What must NOT be re-opened + +- **Do not "fix" the 40 existing call sites to pass something other than `NOW_ANCHOR`.** They + are correct as they are. Their uniformity is what made the defect latent, and changing them + is churn that proves nothing. +- **Do not weaken or delete any assertion to make something pass.** If a test goes red against + this fix, that test was relying on the bug and needs its own decision recorded here — not a + quiet adjustment. +- **Do not touch the unpinned clock path.** The `elapsedBefore` accumulator and the + midnight-rollover unwrap are Phase 3 work with their own reasoning and their own test (a + 50-tick walk past a full day). This fix deliberately left them alone. +- **Nothing here needs a provider.** No OpenAI, no Supabase, no hosted CI, no live database. + If a step seems to want one, it is the wrong step. + +--- + +## 6. Two traps this work actually hit + +Both cost real time. Both will recur. + +**A gate reported success without running.** `npm run lint` exited `0` twice while doing +nothing — once because another worktree held the repository's heavy-run lease +(`DATABASE_HEAVY_RUN_ADMISSION_BUSY`), once on a Windows `EPERM` collision creating the lock +directory. Piping through `tail` masks the real exit code, so both looked like passes. **Read +the output, not the exit code**, and re-run until the gate prints its own result line. The +third attempt genuinely passed and recorded a gate receipt. + +**A fresh worktree has an empty `node_modules`.** `node scripts/run-vitest.mjs` fails with +`Cannot find module 'vitest/vitest.mjs'` and it looks like a broken checkout. It is not — run +`node scripts/setup-codex-worktree.mjs`, which copies a byte-identical dependency tree from +another worktree in about a minute. Do not reach for `npm ci`; it takes the better part of an +hour on this machine. + +**How to run tests here**, since it is not the obvious command: + +```bash +GATE_RECEIPTS=refresh node scripts/run-vitest.mjs run tests/ward-flow-provider.dom.test.tsx +``` + +Never bare `npx vitest`. `GATE_RECEIPTS=refresh` matters whenever fresh evidence is the point: +receipts are memoised against a content signature, so a plain re-run can exit `0` having +printed no result at all. diff --git a/scripts/check-docs-links.mjs b/scripts/check-docs-links.mjs index c2623fb30..7138c5348 100644 --- a/scripts/check-docs-links.mjs +++ b/scripts/check-docs-links.mjs @@ -101,6 +101,18 @@ const SCOPED_ALLOWLIST = new Map([ // does not resolve as a path. new Set(["tests/caring-contacts-overlay-trigger.dom.test.tsx(107"]), ], + [ + "docs/ward-flow-pinned-clock-handover.md", + // Both files exist on `claude/ward-flow-phases-6-7-design` and not on this branch — + // naming them is the entire point of the handover, which exists to send a later session + // to that branch to finish the work. The document says so where it names them, and gives + // the `git show` command to read the spec from there. Remove this entry once Phase 6 + // lands on `main` and both paths resolve normally. + new Set([ + "docs/superpowers/specs/2026-08-27-ward-flow-phase-6-morning-page-design.md", + "tests/ward-morning-page.dom.test.tsx", + ]), + ], ]); /** True when `repoRelative` is allowed outright, or allowed for the document being scanned. */ diff --git a/src/components/ward-management/ward-flow-provider.tsx b/src/components/ward-management/ward-flow-provider.tsx index d26d93446..1f285f0bf 100644 --- a/src/components/ward-management/ward-flow-provider.tsx +++ b/src/components/ward-management/ward-flow-provider.tsx @@ -104,16 +104,25 @@ export function WardFlowProvider({ children, initialNow }: WardFlowProviderProps return () => clearInterval(id); }, [initialNow]); - // Recomputed from the live wall clock on every unpinned render (not only on a tick, so a - // missed/delayed timer tick still reports the true elapsed minutes rather than a fixed - // 30s-per-tick approximation) — but layered on top of the last checkpoint's accumulated - // total rather than the original mount instant, which is what keeps this correct beyond one - // day. A pinned `initialNow` (tests, deterministic renders) never touches the wall clock. - const elapsed = + // The base instant the demo day is read from, before any in-app `ADVANCE_CLOCK` offset. + // + // Pinned (`initialNow` supplied by a test or any deterministic render): the caller's instant + // IS the clock. It is used verbatim rather than as a mere "do not tick" flag — a pinned + // provider that ignored the value and always read `NOW_ANCHOR` would make every + // time-of-day branch in the screens unreachable from a test, because the only clock a test + // could ever obtain would be the one instant the fixture is authored around. + // + // Unpinned (the live app): `NOW_ANCHOR` plus real elapsed time. Recomputed from the live + // wall clock on every render (not only on a tick, so a missed/delayed timer tick still + // reports the true elapsed minutes rather than a fixed 30s-per-tick approximation) — but + // layered on top of the last checkpoint's accumulated total rather than the original mount + // instant, which is what keeps this correct beyond one day. The pinned path never touches + // the wall clock. + const base = initialNow !== undefined - ? 0 - : clockCheckpoint.elapsedBefore + elapsedMinutesSinceMount(clockCheckpoint.reading, wallClockNow()); - const now = NOW_ANCHOR + elapsed + state.clockOffsetMinutes; + ? initialNow + : NOW_ANCHOR + clockCheckpoint.elapsedBefore + elapsedMinutesSinceMount(clockCheckpoint.reading, wallClockNow()); + const now = base + state.clockOffsetMinutes; const [focusMovementId, setFocusMovementId] = useState(undefined); diff --git a/tests/ward-flow-provider.dom.test.tsx b/tests/ward-flow-provider.dom.test.tsx index ff838dbff..5af004c17 100644 --- a/tests/ward-flow-provider.dom.test.tsx +++ b/tests/ward-flow-provider.dom.test.tsx @@ -51,6 +51,64 @@ describe("WardFlowProvider", () => { expect(screen.getByTestId("rejections")).toHaveTextContent("0"); }); + describe("a pinned clock other than the fixture's own anchor", () => { + // Every other `initialNow` call site in the suite passes `NOW_ANCHOR`, which is exactly why + // the prop's value being discarded was invisible: `NOW_ANCHOR + 0` and `initialNow` agree + // when — and only when — the pinned instant IS `NOW_ANCHOR`. Nothing below may use + // `NOW_ANCHOR` as the pinned value, or it stops testing anything. + const PINNED_BEFORE_ANCHOR = 7 * 60 + 30; // 07:30 — before the 08:00 morning handover. + const PINNED_AFTER_ANCHOR = 21 * 60 + 5; // 21:05 — a late-evening clock. + + it("reports the pinned instant as `now`, not the fixture anchor", () => { + render( + + + , + ); + expect(screen.getByTestId("now")).toHaveTextContent(String(PINNED_BEFORE_ANCHOR)); + // Named explicitly so a regression that silently reverts to the anchor cannot pass by + // coincidence, and so the failure message says which clock was actually served. + expect(screen.getByTestId("now")).not.toHaveTextContent(String(NOW_ANCHOR)); + }); + + it("reports a pinned instant later than the anchor too, so the fix is not a one-sided offset", () => { + render( + + + , + ); + expect(screen.getByTestId("now")).toHaveTextContent(String(PINNED_AFTER_ANCHOR)); + }); + + it("still applies the in-app clock offset on top of the pinned instant", () => { + // The offset must layer onto the pinned clock, not replace it — otherwise pinning would + // work only until the first `ADVANCE_CLOCK`, which is how the demo controls move time. + render( + + + , + ); + fireEvent.click(screen.getByRole("button", { name: "advance" })); + expect(screen.getByTestId("now")).toHaveTextContent(String(PINNED_BEFORE_ANCHOR + 15)); + }); + + it("still refuses to tick when pinned away from the anchor", () => { + // The value being honoured must not have cost the prop its other job: a pinned provider + // never starts the interval, whatever instant it was pinned to. + const setIntervalSpy = vi.spyOn(window, "setInterval"); + try { + render( + + + , + ); + expect(setIntervalSpy).not.toHaveBeenCalled(); + } finally { + setIntervalSpy.mockRestore(); + } + }); + }); + it("refuses to be used outside the provider rather than returning an empty world", () => { // Conservative failure: a component rendered outside the provider must fail loudly, not // silently render zero patients, which would read as a quiet night.