Keep a workspace on schedules that outlive their device group

Deleting a device group converts its group schedules into per-device ones so the screens keep their
programming. That INSERT omitted workspace_id, which is nullable with no default, so every converted
row landed with workspace_id = NULL.

A null workspace does not merely look untidy — it makes the row unreachable in three directions at
once, and they compound into the worst possible combination:

  invisible   the schedule list and the all-screens calendar both filter on workspace_id
  undeletable PUT and DELETE refuse a row with no workspace (403)
  still live  services/scheduler.js has no workspace filter, so it keeps firing every 60 seconds

"I deleted the group but the screens still switch content at 9am, and there is nothing in the
calendar to remove." The only way out was direct database access.

The conversion now carries the workspace, preferring the schedule's own and falling back to the
group's so a legacy group schedule that itself predates workspace_id still converts into a reachable
row. A boot migration repairs rows already orphaned in the field by recovering the workspace from
the device each one targets; anything still unresolvable is left alone rather than guessed at.

4 tests: the converted row keeps its workspace, is visible to the query the list and calendar use,
preserves the actual programming rather than just the ownership, and the repair recovers a row
orphaned before this fix existed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL

server/db/database.js

@@ -310,6 +310,14 @@ const migrations = [
   // first may be pulled back to stable. Without it, publishing a beta would drag every existing
   // pre-release tester backwards, which is the harm the opt-in exists to prevent.
   "ALTER TABLE devices ADD COLUMN ota_channel_served TEXT",
+  // Repair for schedules orphaned by a group deletion before the conversion carried workspace_id.
+  // Such rows are invisible (list/calendar filter on workspace), undeletable (PUT/DELETE 403 on a
+  // null workspace) and still firing (the scheduler has no workspace filter) — so an operator
+  // cannot fix them from the dashboard at all. Recover the workspace from the device the schedule
+  // targets; anything still unresolvable is left alone rather than guessed at.
+  `UPDATE schedules SET workspace_id = (SELECT d.workspace_id FROM devices d WHERE d.id = schedules.device_id)
+     WHERE workspace_id IS NULL AND device_id IS NOT NULL
+       AND (SELECT d.workspace_id FROM devices d WHERE d.id = schedules.device_id) IS NOT NULL`,
   // #161: privilege tier reported by the player (0 unprivileged / 1 device-admin / 2 owner-or-
   // delegated-install) + whether a foreign device owner (MDM) manages it. Drives dashboard gating
   // of Tier-2 controls (reboot/kiosk/time) — shown only for owned panels.

server/routes/device-groups.js

@@ -140,17 +140,27 @@ router.delete('/:id', requireGroupWrite, (req, res) => {
     let converted = 0;
 
     if (groupSchedules.length > 0 && members.length > 0) {
+        // workspace_id MUST be carried over. It is nullable with no default, so omitting it landed
+        // every converted schedule with workspace_id = NULL — and a null workspace does not merely
+        // look untidy, it makes the row unreachable in three directions at once:
+        //   - the schedule list and the all-screens calendar filter on workspace_id: invisible
+        //   - PUT and DELETE refuse a row with no workspace (403): undeletable
+        //   - services/scheduler.js has NO workspace filter: it keeps firing every 60 seconds
+        // i.e. "I deleted the group but the screens still switch at 9am and there is nothing in the
+        // calendar to remove". The only way out was direct database access.
       const insert = db.prepare(`
-        INSERT INTO schedules (id, user_id, device_id, group_id, zone_id, content_id,
+          INSERT INTO schedules (id, user_id, workspace_id, device_id, group_id, zone_id, content_id,
           widget_id, layout_id, playlist_id, title, start_time, end_time, timezone,
           recurrence, recurrence_end, priority, enabled, color, created_at, updated_at)
-        VALUES (?, ?, ?, NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+          VALUES (?, ?, ?, ?, NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
       `);
 
       for (const schedule of groupSchedules) {
         for (const member of members) {
           insert.run(
-            uuidv4(), schedule.user_id, member.device_id,
+            // Prefer the schedule's own workspace, falling back to the group's, so a legacy
+            // group schedule predating workspace_id still converts into a reachable row.
+            uuidv4(), schedule.user_id, schedule.workspace_id || req.group.workspace_id, member.device_id,
             schedule.zone_id, schedule.content_id, schedule.widget_id,
             schedule.layout_id, schedule.playlist_id, schedule.title,
             schedule.start_time, schedule.end_time, schedule.timezone,

server/test/group-delete-schedule-conversion.test.js

@@ -0,0 +1,100 @@
+'use strict';
+
+// Deleting a device group converts its group schedules into per-device ones so the screens keep
+// their programming. That INSERT omitted workspace_id, which is nullable with no default — so every
+// converted row landed with workspace_id = NULL.
+//
+// A null workspace does not merely look untidy. It makes the row unreachable in three directions at
+// once, and they compound into the worst possible combination:
+//
+//   invisible   — the schedule list and the all-screens calendar both filter on workspace_id
+//   undeletable — PUT and DELETE refuse a row with no workspace (403)
+//   still live  — services/scheduler.js has NO workspace filter, so it keeps firing every 60s
+//
+// i.e. "I deleted the group but the screens still switch content at 9am, and there is nothing in
+// the calendar to remove." The only way out was direct database access.
+//
+// The invariant: a schedule that survives a group deletion stays owned by a workspace, so it can be
+// seen and removed by the person whose screens it controls.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+
+const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'st-groupdel-'));
+process.env.DATA_DIR = tmp;
+process.env.JWT_SECRET = 'test-secret-group-delete';
+
+const express = require('express');
+const { db } = require('../db/database');
+const { requireAuth, generateToken } = require('../middleware/auth');
+const { resolveTenancy } = require('../lib/tenancy');
+
+const O = 'o-gd', WS = 'ws-gd', U = 'u-gd', G = 'g-gd', DEV = 'dev-gd';
+db.prepare("INSERT OR IGNORE INTO users (id,email,password_hash,role) VALUES (?,?, 'x','user')").run(U, 'gd@t.local');
+db.prepare('INSERT OR IGNORE INTO organizations (id,name,owner_user_id) VALUES (?,?,?)').run(O, 'Org', U);
+db.prepare('INSERT OR IGNORE INTO workspaces (id,organization_id,name) VALUES (?,?,?)').run(WS, O, 'WS');
+db.prepare("INSERT OR IGNORE INTO organization_members (organization_id,user_id,role) VALUES (?,?, 'org_owner')").run(O, U);
+db.prepare(`INSERT OR IGNORE INTO devices (id,name,workspace_id,created_at,updated_at)
+            VALUES (?,?,?,strftime('%s','now'),strftime('%s','now'))`).run(DEV, 'Screen', WS);
+db.prepare('INSERT OR IGNORE INTO device_groups (id,name,workspace_id,user_id) VALUES (?,?,?,?)').run(G, 'Group', WS, U);
+db.prepare('INSERT OR IGNORE INTO device_group_members (group_id,device_id) VALUES (?,?)').run(G, DEV);
+db.prepare(`INSERT OR IGNORE INTO schedules (id,user_id,workspace_id,group_id,title,start_time,end_time,timezone,priority,enabled)
+            VALUES ('sg-1',?,?,?, 'Morning menu','09:00','17:00','UTC',1,1)`).run(U, WS, G);
+
+const app = express();
+app.use(express.json());
+app.set('io', null);
+app.use('/api/groups', requireAuth, resolveTenancy, require('../routes/device-groups'));
+const server = app.listen(0);
+const token = generateToken(db.prepare('SELECT id,email,role FROM users WHERE id = ?').get(U), WS);
+
+async function deleteGroup() {
+  await new Promise(r => (server.listening ? r() : server.once('listening', r)));
+  const res = await fetch(`http://127.0.0.1:${server.address().port}/api/groups/${G}`, {
+    method: 'DELETE', headers: { Authorization: `Bearer ${token}` },
+  });
+  return { status: res.status, body: await res.json().catch(() => null) };
+}
+
+test('THE BUG: a schedule that survives a group deletion keeps its workspace', async () => {
+  const { status, body } = await deleteGroup();
+  assert.equal(status, 200);
+  assert.ok(body.schedules_converted >= 1, 'the schedule should have been converted, not dropped');
+
+  const converted = db.prepare('SELECT * FROM schedules WHERE device_id = ? AND group_id IS NULL').all(DEV);
+  assert.equal(converted.length, 1);
+  assert.equal(converted[0].workspace_id, WS, 'a null workspace makes the row invisible AND undeletable AND live');
+});
+
+test('the converted schedule is therefore visible to the workspace it controls', () => {
+  // This is the query the schedule list and the calendar both use.
+  const visible = db.prepare('SELECT COUNT(*) n FROM schedules WHERE workspace_id = ? AND device_id = ?').get(WS, DEV).n;
+  assert.equal(visible, 1);
+});
+
+test('the programming itself is preserved, not just the ownership', () => {
+  const s = db.prepare('SELECT * FROM schedules WHERE device_id = ? AND group_id IS NULL').get(DEV);
+  assert.equal(s.title, 'Morning menu');
+  assert.equal(s.start_time, '09:00');
+  assert.equal(s.end_time, '17:00');
+  assert.equal(s.enabled, 1);
+});
+
+test('the repair recovers rows orphaned before this fix existed', () => {
+  // Simulate the old behaviour, then run the same statement the boot migration runs.
+  db.prepare(`INSERT INTO schedules (id,user_id,workspace_id,device_id,title,start_time,end_time,timezone,priority,enabled)
+              VALUES ('legacy-orphan',?,NULL,?, 'Orphan','06:00','08:00','UTC',1,1)`).run(U, DEV);
+  assert.equal(db.prepare("SELECT workspace_id FROM schedules WHERE id='legacy-orphan'").get().workspace_id, null);
+
+  db.prepare(`UPDATE schedules SET workspace_id = (SELECT d.workspace_id FROM devices d WHERE d.id = schedules.device_id)
+              WHERE workspace_id IS NULL AND device_id IS NOT NULL
+                AND (SELECT d.workspace_id FROM devices d WHERE d.id = schedules.device_id) IS NOT NULL`).run();
+
+  assert.equal(db.prepare("SELECT workspace_id FROM schedules WHERE id='legacy-orphan'").get().workspace_id, WS,
+    'an operator must be able to see and delete it from the dashboard');
+});
+
+test.after(() => { server.close(); try { fs.rmSync(tmp, { recursive: true, force: true }); } catch (_) {} });