Fix /_/health reporting stale degraded status after schema recovers

[DennisAlund]

Weekly defect-hunting review. One fix qualified to commit directly; two other findings need developer judgment and are posted as PR comments instead of being fixed here.

Fix: /_/health stuck reporting "degraded" after the schema actually recovers

What was wrong: handleHealth() (src/api/health.ts) seeded its error field from the isolate's remembered lastSchemaFailure() before running a live schemaStatus() read, and never cleared it when that live read succeeded.

Impact: An isolate that once failed a migration attempt keeps answering 503 degraded with the stale error forever, even after the schema is genuinely fully applied (e.g. another isolate raced the same cold start and finished it, or a later attempt outside the health/setup exemption succeeded). /_/health and POST /_/setup are both exempt from the normal per-request ensureSchema() call, so nothing else clears the isolate's memo on that path. The response ends up self-contradictory: schema.ready: true alongside status: "degraded". A monitor polling only /_/health sees a false outage after the database is actually healthy.

Fix: when the live schemaStatus() read succeeds and reports every migration applied, clear the stale error before deciding the response status — a live ready: true is direct proof there's no current problem, regardless of what an earlier attempt on this isolate remembered.

Test: src/__tests__/handler/schema-guard.test.ts — new case "recovers once a fresh live read reports the schema ready, even with a failure remembered from before". Simulates a transient failure that sets the isolate's memo, then applies the schema for real via migrate() directly (bypassing ensureSchema(), exactly as a racing isolate would), and asserts /_/health now reports 200 ok instead of the previously stale 503 degraded. Confirmed failing (503 instead of 200) before the fix, passing after.

Full suite: yarn test — 1597 tests passing. yarn tsc --noEmit clean. No changes under src/pages/, src/admin/, src/client.ts, src/styles.ts, or src/index.tsx admin routes, so yarn e2e isn't required for this change.

Flagged for developer judgment (posted as PR comments, not fixed here)

1. src/db/migrate.ts — migrate()'s KV fast-path can make POST /_/setup's documented "forgets the memo and migrates again" recovery a silent no-op if D1 is ever reset or rebound out-of-band while the KV namespace persists. Fixing this means deciding whether the manual retry path should ever bypass the intentional (and explicitly tested) KV cache, and that's a real behavior-boundary call, not a slam-dunk local patch.
2. src/client.ts — the "Add to bundle" modal lets a viewer deselect a bundle they don't own; the server correctly 403s the resulting DELETE, but the UI has no ownership gating (unlike the bundle detail page, which hides "Remove from bundle" for non-owners) and shows a generic failure toast while any other successful adds in the same batch silently persist. A real fix needs a UX decision and a way to expose current-user identity to client.ts, which none of the pages currently do.

🤖 Generated with Claude Code

https://claude.ai/code/session_0181pUqPZvD9NsRhda8LfoAr

---

Generated by Claude Code

[DennisAlund]

Flagged, not fixed: migrate()'s KV fast-path can make POST /_/setup a silent no-op (src/db/migrate.ts:140)

migrate() trusts the KV memo (SCHEMA_VERSION_KEY) unconditionally: if KV already holds the target version, it returns without ever touching D1 (deliberate, and pinned by the test "touches D1 not at all when KV already holds the current schema version" in src/__tests__/integration/schema-migrate.test.ts:120-125).

The gap: if D1 is ever reset or rebound to an empty database out-of-band (disaster recovery, a redeploy that reuses the old KV namespace with a fresh D1) while KV still remembers the old target version, migrate() never recreates the schema. Every real request then hits raw no such table D1 errors instead of the documented schema-guard page. POST /_/setup, documented as "forgets the isolate's memo and migrates again, the manual retry for a deploy whose first request hit a transient error," goes through the same migrate() and hits the same KV short-circuit before ever reading D1 — so in this specific scenario it doesn't actually recover anything.

I didn't fix this because the correct scope is a judgment call: should the manual retry path (retrySchema()/POST /_/setup) always bypass the KV cache to re-verify against D1, while every other request keeps trusting it for the fast path? That changes the contract of an intentional, tested performance optimization on the request hot path, and there may be other ways to detect/handle an out-of-band D1 reset that are less invasive. Worth a decision on whether this disaster-recovery scenario is in scope at all, and if so, how the recovery should be triggered.

---

Generated by Claude Code

[DennisAlund]

Flagged, not fixed: "Add to bundle" modal has no ownership gating on deselect (src/client.ts:1453, toggleAddToBundleRow/saveAddToBundle)

Per the authorization model (CLAUDE.md rule 7 / docs/access-control.md), only the bundle owner can remove a link from a bundle. The bundle detail page respects this: it hides "Remove from bundle" entirely for non-owners (src/pages/bundle-detail.tsx:231, gated by isOwner).

The link page's "Add to bundle" modal doesn't. toggleAddToBundleRow just toggles a CSS class with no ownership check, so a viewer can deselect a bundle they don't own that the link already belongs to (a normal state, since anyone can add a link to any bundle). saveAddToBundle then diffs selections and fires a DELETE /bundles/{id}/links/{linkId} for it, which the server correctly 403s — but the batch runs via Promise.all, so any other successful adds in the same save silently persist server-side while the UI shows only a generic "could not save" toast, with no indication of which part failed or that anything succeeded.

I didn't fix this because it needs a UX decision (disable the row for bundles the viewer doesn't own? show a lock icon/tooltip explaining why? silently keep it selected and skip it from the diff?) and, more substantially, client.ts currently has no way to know the current user's identity — every page computes isOwner server-side at render time (identity prop) and never exposes it to the client-side JS. Wiring that through is a bigger change than a local patch, and per CLAUDE.md any change under src/client.ts needs yarn e2e coverage for the new control behavior.

---

Generated by Claude Code

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	shrtnr	b024859	Sep 25 2026, 07:23 PM