fix(sacred-valley): review polish — render-gen guard, auth-boundary tests, PNG sig, dedup note

Addresses final-review findings: I1 render-generation guard prevents a double-mount
/timer leak on rapid re-navigation; I2 adds anonymous-rejection tests for the owner-only
POST /speedtest/run and /health/check; M1 CSS comment; M2 cron↔worker dedup note;
M4 full 8-byte PNG signature check; M5 card-contract unit test for all 7 cards.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

lib/cron/index.js

@@ -23,6 +23,10 @@ export function startCron() {
     catch (e) { log.error({ err: e }, 'cron speedtest failed'); }
   });
 
+  // Health checks every minute. NOTE: this runs checkAll() inline; the same
+  // probe+upsert logic is also exposed on-demand via the `health.check` pg-boss
+  // worker (lib/jobs/workers/health_check.js, triggered by POST /api/health/check).
+  // Keep the two in sync — both rely on lib/health/checker.js as the source of truth.
   cron.schedule('*/1 * * * *', async () => {
     try {
       const results = await checkAll(load());

lib/health/icons.js

@@ -16,7 +16,8 @@ async function defaultFetcher(slug) {
   return Buffer.from(await res.arrayBuffer());
 }
 
-function isPng(buf) { return buf && buf.length > 8 && buf[0] === 0x89 && buf[1] === 0x50 && buf[2] === 0x4e && buf[3] === 0x47; }
+const PNG_SIG = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a];
+function isPng(buf) { return buf && buf.length > 8 && PNG_SIG.every((b, i) => buf[i] === b); }
 
 // Returns a Buffer (cached or freshly fetched) or null if upstream has no icon.
 export async function getIcon(slug) {

public/style.css

@@ -175,7 +175,7 @@ ul.plain li:last-child { border-bottom: none; }
      --hue-dross: #ff4f2e; --hue-yerin: #c45a4a; --hue-orthos: #6fa86a; */
 }
 #sv-cards { display: grid; grid-template-columns: repeat(6, 1fr); gap: 16px; align-items: start; }
-.sv-card { grid-column: span 2; }                /* m default */
+.sv-card { grid-column: span 2; }                /* s / fallback (factory always sets data-size) */
 .sv-card[data-size="s"] { grid-column: span 2; }
 .sv-card[data-size="m"] { grid-column: span 3; }
 .sv-card[data-size="l"] { grid-column: span 6; }

public/views/sacred_valley.js

@@ -14,8 +14,10 @@ import speedtest from './cards/speedtest.js';
 
 const CARD_MODULES = [clock, weather, hostPerf, jobs, inbox, search, speedtest];   // grows in later tasks
 let active = [];                                   // mounted cards needing stop()
+let renderGen = 0;                                 // guards against overlapping async renders
 
 export async function render(main) {
+  const myGen = ++renderGen;
   active.forEach(c => c.stop && c.stop()); active = []; stopHealthBand();
   mount(main,
     el('h1', { class: 'view-h1' }, 'Sacred Valley'),
@@ -26,6 +28,9 @@ export async function render(main) {
 
   let layout = { card_order: [], hidden: [], sizes: {} };
   try { layout = await api.get('/api/dashboard/layout'); } catch { /* defaults */ }
+  // A newer render() started while we awaited — bail before mounting timers/DOM
+  // so we don't double-mount cards or leak intervals onto the live grid.
+  if (myGen !== renderGen) return;
 
   const grid = document.getElementById('sv-cards');
   const ordered = orderCards(CARD_MODULES, layout);

tests/api/health.test.js

@@ -10,6 +10,8 @@ beforeAll(async () => {
 });
 describe('health api', () => {
   it('401 without auth', async () => expect((await request(app).get('/api/health/services')).status).toBe(401));
+  it('POST /check rejects anonymous (owner-only mutation)', async () =>
+    expect((await request(app).post('/api/health/check')).status).toBe(401));
   it('returns groups with counts + merged cached status', async () => {
     const res = await request(app).get('/api/health/services').set(ownerHeaders);
     expect(res.status).toBe(200);

tests/api/speedtest.test.js

@@ -7,6 +7,8 @@ let app, ownerHeaders;
 beforeAll(async () => { ({ app, ownerHeaders } = await setup()); await repo.record({ down_mbps: 50, up_mbps: 10, ping_ms: 12 }); });
 describe('speedtest api', () => {
   it('401 without auth', async () => expect((await request(app).get('/api/speedtest/history')).status).toBe(401));
+  it('POST /run rejects anonymous (auth boundary before enqueue)', async () =>
+    expect((await request(app).post('/api/speedtest/run')).status).toBe(401));
   it('history returns rows', async () => {
     const res = await request(app).get('/api/speedtest/history').set(ownerHeaders);
     expect(res.status).toBe(200);

tests/frontend/card_contract.test.js

@@ -0,0 +1,31 @@
+import { describe, it, expect } from 'vitest';
+import clock from '../../public/views/cards/clock.js';
+import weather from '../../public/views/cards/weather.js';
+import hostPerf from '../../public/views/cards/host_perf.js';
+import jobs from '../../public/views/cards/jobs.js';
+import inbox from '../../public/views/cards/inbox.js';
+import search from '../../public/views/cards/search.js';
+import speedtest from '../../public/views/cards/speedtest.js';
+
+// Every data card must satisfy the uniform contract that sacred_valley.js relies
+// on: { id, title, size: s|m|l, mount(), start(), stop() }. Importing the modules
+// is safe in node — their browser globals (document/localStorage/fetch) are only
+// touched inside mount()/load(), which this test never calls.
+const cards = { clock, weather, hostPerf, jobs, inbox, search, speedtest };
+
+describe('card contract', () => {
+  for (const [name, c] of Object.entries(cards)) {
+    it(`${name} implements the card contract`, () => {
+      expect(typeof c.id).toBe('string');
+      expect(typeof c.title).toBe('string');
+      expect(['s', 'm', 'l']).toContain(c.size);
+      expect(typeof c.mount).toBe('function');
+      expect(typeof c.start).toBe('function');
+      expect(typeof c.stop).toBe('function');
+    });
+  }
+  it('all card ids are unique', () => {
+    const ids = Object.values(cards).map(c => c.id);
+    expect(new Set(ids).size).toBe(ids.length);
+  });
+});