278 commits

Author	SHA1	Message	Date
screentinker	128a5be1b1	Node 22 preparation: upgrade runbook, changelog notes, and the one test that breaks (#265) ... * Document the Node.js upgrade procedure and this build's reinstall requirement Upgrading the runtime does not go through scripts/upgrade.sh, so nothing reinstalls dependencies — which is precisely when the one remaining native module goes stale. The runbook now covers the version floor imposed by --env-file-if-exists, why the better-sqlite3 pin is exact, and why a version without a matching prebuild can turn Restart=always into a boot loop. Also records that this build changes dependencies in both directions: rolling back past it needs the reinstall too, because earlier builds import sharp at runtime and this one drops it from production dependencies. Kept deployment-neutral — no hostnames, addresses, or environment specifics. * Fix the only test that fails on Node 22 Node 22 added a built-in `navigator` global, defined as a getter with no setter. The test's shim assigned to it, which throws "only a getter" under 'use strict' on 22 while being a normal assignment on Node 20, where the global does not exist at all. It is configurable, so define it instead of assigning. Defining it unconditionally is also the better fixture: Node 22's own navigator reports the HOST locale, so a test reading its language would otherwise depend on the machine or CI runner it happens to run on. This was the single failure in an otherwise clean Node 22 run (1639/1640 with better-sqlite3 12.9.0), and it is confined to test code — no production server or frontend file assigns to globalThis.navigator. 1649/1649 on Node 20.	2026-08-13 12:37:36 -05:00
screentinker	13c9c67335	Drop sharp: pure-JS image ops on a worker thread (#263) ... * spike: replace sharp with pure-JS image ops (jimp + jsquash WASM) Removes the last native dependency from the ingest path, so the server no longer needs a per-platform/per-ABI prebuilt to thumbnail an image. Motivated by getting the server onto hardware with no toolchain, but the ABI tax is paid on every install — it is the same failure class lib/preflight-deps.js exists to explain. lib/image-ops.js is the whole surface: metadata() and writeThumbnail(), which are the only two things ingest ever asked sharp for. Format parity holds. jpeg/png/gif/tiff/bmp are native to Jimp; webp and avif go through @jsquash WASM, whose bundled .wasm must be compiled by hand because the packages locate it with fetch(file://) and Node has no file:// fetch — the only symptom otherwise is a bare "fetch failed". heic is unsupported, as it already was: sharp advertises heif but its prebuilt libvips refuses HEVC. #170 is preserved by a different mechanism. Jimp applies EXIF orientation at decode and rewrites the tag to 1, so metadata() reports display dimensions and imageDisplayDims() runs as a no-op instead of swapping W/H a second time. The helper stays in the path so the rule keeps living in one place. Verified: 1643/1643 tests pass, and ingest was exercised in a child process with node_modules/sharp moved aside — jpeg, EXIF-rotated jpeg, png, webp, avif, gif all measured and thumbnailed correctly, corrupt input still yields nulls with no phantom thumbnail_path. KNOWN BLOCKER, do not ship as-is: Jimp is pure JS on the main thread, where sharp handed work to a libvips threadpool. A 12MP photo goes 65ms -> 1079ms, and the event loop stalls for 1003ms of it (sharp: zero stalls). thumbnail-backfill.js walks a whole library at boot, so this reproduces #240 exactly — blocked loop, missed heartbeats, panels marked offline, reconnect churn. Needs a worker_thread offload before this is viable; image-ops.js is the seam for it. * Run image decoding on a worker thread Fixes the blocker the previous commit shipped with. Pure-JS decoding costs ~1s of solid CPU for a 12MP photo, and in-process that is not a slow upload but a stalled event loop — no heartbeats, no socket traffic. thumbnail-backfill.js walks a whole library at boot, so it reproduced #240 (blocked loop -> missed heartbeats -> panels offline -> reconnect churn) from our own maintenance. sharp never did this because libvips works on a threadpool. image-ops.js is now a dispatcher over image-ops-worker.js; the work moved unchanged to image-ops-core.js, so callers and their failure contract are untouched. Measured on a 12MP photo: 1079ms wall with the loop stalled 1003ms, to 1881ms wall for two ops with ZERO stalls and 185 timer ticks serviced. Wall time is worse and that is fine — it is off the main thread now. Design notes, all load-bearing: - ONE JOB AT A TIME. A decoded 12MP bitmap is ~48MB of RGBA; overlapping jobs multiply peak memory by queue depth, which is the wrong failure on the small targets this change exists to reach. Costs no throughput — the work is CPU-bound and one busy worker already saturates its core. - unref'd while idle, ref'd only in flight. Otherwise scripts/backfill-rotation- dims.js never exits and `node --test` hangs forever. Verified: a CLI-style run exits in 104ms, code 0. - decode failures reply as messages, so one bad upload cannot tear down the worker and take unrelated queued jobs with it. - in-process fallback if a thread cannot be had, warned rather than silent. test/image-ops.test.js pins the loop-liveness property, which no functional test would catch. Its thresholds were mutation-tested against the inline path: the first version passed there too (4MP stalls only ~355ms, under a non-flaky threshold), so the fixture is 12MP and the thresholds sit in the gap between the two behaviours — worker ~90 ticks/~0ms, inline ~3 ticks/~897ms. It now fails inline, as a guard must. 1647/1647 pass. Ingest re-verified with node_modules/sharp moved aside. * Measure and thumbnail an image from a single decode Ingest asked for metadata() then writeThumbnail(), which decoded the file twice. That pairing was free under sharp, whose .metadata() only parses the header, but every decode here is a full one — ~1s for a 12MP photo — so the naive translation doubled the most expensive thing on the ingest path. image-ops.measureAndThumbnail() returns both from one decode. Full ingest of a 12MP photo: 2 decodes/~1.9s -> 1150ms, still with zero event-loop stalls. The subtlety is the failure contract. In the two-call version width and height were assigned BEFORE the thumbnail was attempted, so a failed thumbnail still left usable dimensions on the row — the player needs them to letterbox. Merging naively would have turned any thumbnail failure into total metadata loss. So a WRITE failure is reported ({thumbnailWritten:false, thumbnailError}) with the dimensions intact, and the caller sets thumbnail_path only when the write succeeded, keeping the phantom-path discipline. A DECODE failure still throws — there is nothing to report about an unreadable image. backfill-rotation-dims.js deliberately keeps the separate calls: it probes every image row but regenerates a thumbnail only for the few whose dimensions changed, so pairing them there would decode files it has no reason to thumbnail. Tests count decodes rather than timing them — an exact property, and a wall-clock comparison would be flaky under load. The count filters for reads of the file under test: Node's ESM loader also goes through fs.promises.readFile, so a raw call count picks up jimp's and the WASM codecs' lazy loading and reads 30 instead of 1. 1649/1649 pass. Ingest re-verified across all 7 formats with sharp moved aside. * Dockerfile: sharp is no longer a production dependency --omit=dev now leaves it out entirely; better-sqlite3 is the only native module the builder stage still needs a toolchain for.	2026-08-13 11:40:13 -05:00
ScreenTinker	3617a1a116	Link start cannot be navigated to: a bearer token does not survive it ... "Authentication required" on every click of Link. The Settings button did `location.href = /api/auth/oidc/<slug>/link/start`, which is a top-level navigation -- and this app's session lives in localStorage and travels as an Authorization header, so the request arrived anonymous and requireAuth refused it, correctly. The login /start route works precisely because it needs no session. Copying its shape for a route that does need one was the mistake. The client now FETCHES link start with its token and navigates to the URL it returns. The transaction cookie is still set by that response, because a same-origin fetch stores Set-Cookie normally, so the callback is unchanged. beginOidc grew an asJson flag rather than a second copy of the PKCE/state/nonce setup, so login and link still cannot drift apart. Both mutations fail the new test: navigating straight at the route, and having the server redirect instead of answering with JSON.	2026-08-12 14:44:23 -05:00
ScreenTinker	184ff71dee	Let an existing account move to SSO, and ask who you are before how ... Two halves of the same problem: an account created with a password could never use single sign-on, and the login page offered a credential before it knew which one applied. LINKING. Signing in with a provider never adopts an account that already has a password -- that is the takeover the login path exists to refuse. The README promised the way out ("the owner signs in locally and links from Settings") but nothing had ever been built, so the refusal was a dead end rather than a redirection. Settings now has a Sign-in method block: an account with a password can link an instance-wide provider, and one on a provider can unlink back to a password. The account being linked comes from the SIGNED TRANSACTION -- the session that started it -- never from the email in the returned token. That distinction is the whole feature: taking it from the token would be the same email-keyed takeover under a friendlier name. The email must still match the account's own, because login resolves accounts by the asserted address, and one provider subject may not be linked to two accounts. Linking DELETES the password rather than keeping it alongside. One credential at a time, and the confirmation says so in those words, because a password left behind is a second way in that the user believes they replaced. Unlink therefore takes the new password up front and writes it in the SAME statement as the unlink -- never unlink now and set a password after, which leaves an account briefly, or on failure permanently, with no way in. Instance-wide providers only. An organization's provider is chosen by a customer; letting one attach itself to a platform account would hand that customer whatever the account can do. IDENTIFIER-FIRST. The password box now appears only after an address has been submitted, which is what lets the organization lookup happen before a credential is offered: someone whose company requires its own provider is shown that, rather than a password box that will be refused. Editing the address returns to the identifier step so a corrected domain gets a fresh answer. The per-keystroke lookup is gone with it. It answered for half-typed domains, changed the form under someone mid-address, and spent a 10/min per-IP budget on people who had not finished typing -- an office behind one address could exhaust it without a single sign-in attempt. Instance-wide providers stay visible at all times now, by decision: the server refuses them for an SSO-only organization anyway, and hiding them made the page change shape while typing. Verified in a real browser, not only by rendering: password hidden -> submit -> visible and focused -> edit the address -> hidden again, with no page errors. Four mutations of the linking rules fail the tests (account from the email instead of the session, keeping the password, allowing org providers, dropping requireAuth).	2026-08-12 11:48:11 -05:00
ScreenTinker	e5e5b75b85	Customer Entra tenants: verify the domain, then be believed ... The previous fix let the instance-wide Microsoft button work and left the customer-facing path broken, which is the worst way round. An organization that brings its own Entra tenant would publish the TXT record, watch its domain go green, and still be refused at login with `email_unverified` -- because Entra sends no such claim and rowToProvider pinned the assumption off for every org provider. Requiring a claim Microsoft does not emit is not a security control, it is an outage. What makes it safe to stop requiring it is the proof that already gates these providers: the callback confines an org provider to its DNS-verified domains, and an address only reaches the check after passing that. Whoever controls a domain's DNS controls its mail, which is the same trust that makes a verification link meaningful. So the assumption is DERIVED from proof -- `verified.length > 0` -- rather than pinned off. A provider that has verified nothing still assumes nothing, which is belt and braces: emailAllowedForProvider already refuses it, since an empty allow-list matches no domain, but deriving it here means a future reordering of those checks cannot silently widen it. It is never a column, and there is no column for it to be read from. An organization must not be able to switch this on for itself; it is a consequence of DNS proof, not a setting. A test asserts both -- that the value is derived next to `source: 'org'`, and that no `assume_email_verified` exists in the schema. Domain confinement is untouched. An explicit `email_verified: false` is still refused from anyone. Mutations all fail the tests: assuming unconditionally, never assuming, and reading it from the row.	2026-08-11 23:05:25 -05:00
ScreenTinker	b1a58144bb	Microsoft sign-in could never complete: Entra omits email_verified ... The OIDC callback required `claims.email_verified === true`. Entra ID v2 does not send that claim at all, so every Microsoft login authenticated correctly against the tenant and was then refused with `email_unverified` on the way back. Nothing caught it: the SSO tests assert how the Microsoft issuer string is built but never put a Microsoft-shaped token through the policy. The strict check was itself a fix -- `=== false` had been accepting an omitted claim -- and it is right for a provider a CUSTOMER configured, since such a provider is chosen by the party it vouches for and its bare assertion is worth nothing. What was wrong is treating that as a question about the token when it is a question about who we trusted. `users.email_verified` is our own state; the claim is the IdP's. An instance-wide provider was chosen by the operator -- the same trust that already exempts it from domain confinement -- and Microsoft is additionally pinned to one tenant GUID, so only that directory can issue a token whose `iss` matches. So the policy now depends on the provider, in emailIsVerified(), next to the flag it reads so the two cannot drift: - explicit true -> believed, from anyone - claim absent, operator -> believed (Microsoft; opt-in for other IdPs) - claim absent, org -> refused - explicit false -> refused, always Org providers pin the flag false in rowToProvider and never read it from the row, so the takeover path the strict check existed to close stays closed. Google is left strict: it does send the claim. Also documents MICROSOFT_CLIENT_SECRET (supported in code, missing from the table), that the redirect URI must be registered under Web rather than SPA, and the email optional claim -- the other two ways an Entra setup fails. All three mutations of this policy fail the new tests: reinstating the strict check (3 failures), letting an org provider assume (1), and accepting an explicit false (2).	2026-08-11 22:29:53 -05:00
ScreenTinker	226c96c17e	Keep the widget editor's Preview isolated, whatever the org setting says ... #254 lets an organization opt out of widget iframe isolation so that players can embed origin-strict third-party sites. It applied that opt-out to the widget editor's Preview as well. Preview is framed by the dashboard, from the dashboard's own origin, and the dashboard keeps its session JWT in localStorage. So with the setting on, anyone who can author a widget -- workspace_editor and up; viewers are refused at the create route -- could put script in a text widget and read the session of whichever admin clicked Preview. That is an editor -> admin escalation, and it is not the risk the confirmation modal asks the admin to accept: a player runs on a kiosk with a device token, an admin's dashboard session is a different thing entirely. The org setting is what makes players able to embed those sites, so the /render path keeps consulting it. Preview is pinned to allow-scripts in both places that build it -- the dashboard iframe and the server-side render -- so neither a frontend change nor a new server caller can re-grant it alone. Also correct the modal copy, which claimed same-origin would expose the session of anyone viewing "a display or preview". Preview is now excluded, and the display case is really the device token, so say that instead. widget-preview-stays-isolated.test.js fails if either half is reverted; both mutations were checked to fail before committing.	2026-08-11 15:53:57 -05:00
screentinker	6aeb703efe	Merge pull request #254 from ChrisChrome/main ... Add org-level widget sandbox toggle.	2026-08-11 15:45:39 -05:00
ScreenTinker	983bee31b7	SSO-only: close the backdoor, the unilateral disable, and the fresh-install fail-open ... Three HIGH findings from the QA round. Each was demonstrated end to end against a running server, and each is now refused there. ENFORCEMENT PROTECTED A DOMAIN, NOT AN ORGANIZATION ssoOnlyForEmail answers about an address's domain, so any account in the tenant at an outside address kept password login — a contractor, an MSP, the one address nobody remembered. And it could be manufactured: POST /api/admin/users accepts workspace_admin and creates a LOCAL password account at any address bound to that workspace. A review created backdoor@notacme.test, logged in with the password, landed in the SSO-only org, and used it to create another. Enforcement is now keyed on MEMBERSHIP as well as domain (ssoOnlyForUser), and that route refuses to mint password accounts into an SSO-only organization at all. platform_admin keeps both, as the operator break-glass. THE APPROVAL WORKFLOW WAS DECORATIVE `sso_only` is honoured only while a provider is enabled and a domain is verified, so `PUT {enabled:false}`, `PUT {email_domains:""}` and `DELETE` each switched enforcement off — with sso_only still reading true, no request filed and the operator never told. The delete variant additionally rewrites every federated account to `local`, after which a password reset takes over accounts the identity provider was supposed to own. Anyone who could file a request could simply turn the provider off instead. All three now refuse with sso_only_locked when nothing else would still enforce, and say to ask for approval. FRESH INSTALLS FAILED THE MIGRATION AND FAILED OPEN The ALTER adding organizations.sso_only sat in the column-migration array, which runs BEFORE the multi-tenancy migration that creates the table: `[migrate] FAILED … no such table: organizations`, one line among ~85. The instance then ran its whole first boot with the SSO settings screen 500ing and ssoOnlyForEmail catching `no such column` and answering "not required" — password login proceeding for an organization that had switched it off. It self-healed on the second boot, which is what made it easy to miss. The column is now added after the table exists, and the catch distinguishes "this instance has no per-org SSO" (null, so single-tenant installs keep working) from drift on a table that DOES exist (throw). Login treats an undeterminable answer as "required" rather than letting a 500 escape or letting the login through. Verified live, all four refused with enforcement intact and the operator still able to sign in. 1609 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-11 07:23:28 -05:00
ScreenTinker	601b526264	Boot: install missing dependencies and rebuild the native module before starting ... scripts/upgrade.sh already runs `npm ci`, so this is not for the normal path. It is for the ways a box ends up with the wrong node_modules, both of which present as "server will not start" with an error naming a file rather than the action needed: ROLLBACK checking out an older tag to back out a bad release restores that tag's package.json but not its packages. This branch removes google-auth-library, so a rollback to main would not boot — and you are rolling back because something else already broke. NODE UPGRADE better-sqlite3 is compiled against one ABI. Upgrading Node makes every boot fail with NODE_MODULE_VERSION, which reads like database corruption and is not. Runs as the FIRST statement in server.js, before any dependency is required, and uses only Node builtins — anything it imported could be the thing that is missing. Repairs with `npm install --omit=dev` (never `ci` on a partly-populated tree, which would delete a working node_modules to fix one package) or `npm rebuild better-sqlite3`, and exits with the command to run if it cannot. ST_SKIP_DEP_PREFLIGHT=1 opts out. ⚠️ The first version of the native check was WRONG and I caught it only by running it under a real version mismatch: better-sqlite3's entry point is plain JavaScript that loads the compiled binding lazily, so `require()` succeeds under a Node the binary was never built for. It reported a genuinely broken install as healthy. It now opens an in-memory database, which is what actually pulls the binding in. A test pins that, because the failure is invisible — the check keeps passing on every machine where nothing is wrong. Verified: a deleted dependency is detected, installed and the server boots (200); the ABI mismatch is detected under Node 18 against a module built for Node 20 and reported clean under Node 20; a healthy tree costs 8ms and touches no network. 1609 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 22:44:26 -05:00
ScreenTinker	0e8ffa5444	SSO-only: an org may require its own identity provider, operator approves removal ... Per-organization toggle. Enabling is the safe direction and an org admin does it alone; turning it back off is a REQUEST that a platform admin has to approve, because that is the direction that re-opens password sign-in — the direction a compromised admin would take, and the one a customer will demand at their worst moment with the IdP down. - requires at least one VERIFIED domain, so nobody can lock a company out of a domain they only typed, and an org cannot leave its own people with no way in - the login page HIDES the password field for those domains rather than letting someone type a password that will be refused and then go reset it - the refusal is `sso_required`, distinguishable from a wrong password - the approval email carries NO action link: a token that acts on its own turns every forwarded copy into a way to switch off a customer's SSO. The decision is made signed in as a platform admin. INSTANCE PROVIDERS WERE A SIDE DOOR Blocking passwords while leaving "Continue with Google" is not requiring single sign-on, it is renaming the bypass — instance-wide providers are the operator's and are NOT domain-confined, so one could assert an address at an SSO-only domain and walk straight past the customer's MFA and deprovisioning. The callback now refuses any provider other than that organization's own, and the page stops offering them. Instance-wide stays the default everywhere else: an address whose domain has no org SSO still gets local plus every configured instance provider. The org only overrides for its own verified domains. PLATFORM_ADMIN IS EXEMPT, DELIBERATELY The operator approves turning this off. If the operator's own address sat at an SSO-only domain and that IdP broke, nobody could sign in to approve anything and the instance would be bricked. The exemption is the break-glass, and a test pins it as source so it is not "tidied away" as a convenience. BROWSER-FOUND Hiding the password by hiding its .form-group also hid the organization SSO button, which lives inside that same group — leaving a login page whose only action was "Create Account". Only visible by looking at a screenshot. Hides the field now, not the container. Player untouched: this branch changes no device, WebSocket or provisioning file, and the 358 device/player/socket/pairing tests pass. 1603 tests pass. Enforcement, the approval workflow and the login page verified in real Chrome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 22:19:40 -05:00
ScreenTinker	c91b96ab91	SSO: refuse delegated proof names, release lapsed and deleted claims ... Third review pass. It confirmed the crash wrapper holds (~13,000 hostile requests, no fourth crash), the SSRF rewrite holds (77 vectors, every CIDR boundary proven), the rate-limiter rewrite closed the login brute-force bypass, and /sso/claim rejects every wrong token kind. It also found that two things I built yesterday did not do what they claimed. THE 8-HOUR LIMIT DID NOT BOUND SQUATTING Pressing Verify on an expired claim REISSUED it in place, renewing the clock — so one request per window held a domain forever, through the endpoint meant to enforce the limit. Worse, a renewal was not a new claim, so the operator was notified exactly once, on day zero: a tenant could sit on a company's domain for a year off a single stale alert. A lapsed claim is now RELEASED. Re-adding it is an ordinary new claim: new token, and the operator is told again. Squatting is not impossible; it is loud. A DELEGATED PROOF NAME COULD FORGE A DOMAIN A TXT lookup follows CNAMEs, and RFC 4592 means a wildcard `*.victim.com` synthesizes `_screentinker-verify.victim.com` too — so a wildcard CNAME let whoever controls its target prove a domain they do not own, turning an ordinary subdomain takeover into every `@victim.com` login. A reviewer did this against a real authoritative zone. The proof name is now refused if it is a CNAME, which is stricter than ACME's dns-01, and the comment that claimed wildcards "cannot be mistaken for a proof" — true only for wildcard TXT — has been corrected. MY VERIFY BUTTON REPORTED FAILURE ON SUCCESS `await load()` — the loader is `loadSso()`. The ReferenceError went into a bare catch, so a correct DNS proof showed "Could not verify that domain" and left the card stale. On the expired branch the admin kept publishing a token the server had already rotated. ALSO FIXED - deleting a provider stranded its verified domains (no FK, UNIQUE, never expires) so the domain was blocked for EVERY org forever with no in-product recovery, and its users could neither sign in nor reset. Delete now releases the domains and returns the accounts to local, in one transaction; a cascade FK backstops it. - isOrphanedFederated read absence-of-config as proof-of-deletion, so unsetting GOOGLE_CLIENT_ID made every Google account password-resettable instance-wide, and irreversibly. Restricted to org-provider slugs. - `email_domains: null` (not undefined) took the destructive branch and deleted every DNS proof an organization had. - unbounded domain lists: 400 domains sent 401 emails; now capped at 50, one digest per save, and /api/organizations is rate-limited at all for the first time. - login and register responses carried password_reset_hash and email_verify_hash — live account-takeover credentials handed to the browser. One sanitiser now. - trailing-dot hostname (`https://localhost./`) slipped the SSRF guard. - asyncRoute's own catch could throw and kill the process it exists to protect. - a legacy DB whose typed domains were never verified now says so LOUDLY at boot instead of silently locking every federated user out. TESTS Two of the previous round's tests passed against the code they were named after: one asserted UNIQUE against the test harness's own CREATE TABLE rather than the shipped schema, the other used two different domains so no ordering was exercised. Both replaced and confirmed load-bearing. Seven mutations now turn the suite red, including removing the CNAME refusal, the verified_at filter, and the expiry itself. 1598 tests pass. Delete-release, lapse-release and the leak fix verified against a running server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 21:01:26 -05:00
ScreenTinker	9155370ae8	SSO: TXT only for domain proof, drop the CNAME form ... The CNAME alternative pointed at `<token>.verify.screentinker.com`. Making that work means operating a wildcard DNS zone that answers for every token ever issued — which this project does not have, so half the published instructions described a check that could never pass. Documenting a verification path that cannot succeed is worse than offering one form. TXT needs nothing outside the customer's own zone, and the dedicated `_`-prefixed name keeps it away from the apex where SPF and DMARC live. A wildcard `*.example.com` cannot be mistaken for a proof either way: it answers with its own value, never the token, so it lands in "exists but does not match". Also simplifies check() — one lookup, no Promise.allSettled, and NXDOMAIN is reported as "not published yet" rather than as an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 20:27:03 -05:00
ScreenTinker	d4b8d7dad4	SSO: prove domain ownership by DNS, and fix what the second review found ... A second review pass, run against the previous commit, found four blockers — two of them introduced by the fixes in that commit. It also confirmed the original account takeover is closed: a hostile IdP with real TLS, discovery, JWKS and RS256 driving the real routers now stops at domain_not_allowed, and all 16 bypass variants are refused. DOMAIN OWNERSHIP (the root cause, not the symptom) A claimed domain used to mean "nobody else claimed it". It now means the organization published a record in that domain's own DNS — TXT or CNAME, at a dedicated _screentinker-verify name rather than the apex, where an edit would sit beside SPF. - an unverified domain routes NOBODY and cannot be asserted; it reserves the name - an unverified claim LAPSES after 8 hours, so a domain cannot be held against its real owner, and lapsing rotates the token so a record left over from an abandoned attempt cannot satisfy a later claim - a verified domain never expires — re-proving on a timer would log a customer out over a DNS edit made months later - routing and confinement read the VERIFIED set only, never the typed column - configuring SSO now requires a verified email address - platform admins are emailed when a domain is claimed; nothing is ever sent to the claimed domain, which would let any tenant make this product email third parties Instance-wide providers are exempt from all of it: they are the operator's own configuration and keep the trust they have always had. BLOCKERS FROM THE REVIEW - two unauthenticated remote crashes, both one request, both "async handler throws before its try": `Cookie: st_oidc_tx=%` (unguarded decodeURIComponent) and the fail-closed secret added last commit, which turned a JWT_SECRET rotation into a permanent crash loop. Fixed the CLASS with asyncRoute() rather than the instances. - the SSRF guard was bypassable via IPv4-mapped IPv6 ([::ffff:127.0.0.1]) and also refused every host beginning "fc"/"fd" (fcm.googleapis.com). Addresses are now parsed and compared by RANGE. 42 cases verified. - the takeover fix had NO test — the test named after it asserted two struct fields and passed with the guard deleted. The decision is now a pure function and four mutations were confirmed to turn the suite red. - the PUT path never received the TOCTOU fix, so two orgs could end up holding one domain and forEmail handed routing to the attacker's older row. ALSO - linking compared slugs, so an org could never rotate its own IdP, and fell open on an empty auth_provider. It now asks which ORGANIZATION owns the slug. - an account stranded by a deleted provider can be reclaimed by password reset — proof of the mailbox, which is stronger than the IdP assertion that created it. - /sso/claim accepted a pre-TOTP mfa_pending token and returned the full user row; it now takes a purpose-built 120s claim token with a pinned algorithm and typ. - the rate limiter keyed on a caller-controlled path, so a trailing slash bought a fresh bucket — a real login brute-force bypass. - domain_not_allowed and account_exists_other_provider rendered as "please try again", advice that can never work. - malformed asserted addresses are refused rather than trimmed into shape. - dead config (microsoftTenantId defaulted to 'common', which the provider code now refuses) and the orphaned google-auth-library dependency removed. 1591 tests pass. Domain lifecycle verified end to end against a running server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 19:23:46 -05:00
ScreenTinker	d26aaebef6	SSO: fix an account takeover, a remote crash, and login CSRF found in review ... Five reviewers went at the two SSO commits. Three of them independently demonstrated a full account takeover, and it was the same defect each time. TAKEOVER. An org admin supplies the issuer and client_id, so they control that identity provider completely and can mint an id_token asserting ANY email with email_verified:true — including a platform_admin's. Every cryptographic check passed honestly, because the attacker IS the issuer. upsertFederatedUser then re-pointed the existing account at whichever provider spoke last, because the only guard was `password_hash IS NULL` — and every SSO-created account has a null password. Sessions were issued as the victim, and the victim's own login then failed forever with subject_mismatch. The rule came from the old Google handler, where it was safe: only the operator could add a provider. Making providers customer-configurable turned it into a takeover primitive and the assumption was not re-examined. Now an org provider may only assert emails inside the domains it registered, and may never adopt an account another provider established. REMOTE CRASH, unauthenticated. The state comparison guarded on UTF-16 character length while Buffer.from produces UTF-8 bytes, so a state of 43 characters containing one multi-byte character reached timingSafeEqual with mismatched buffers and threw — inside an async handler, which Express does not catch, which server.js turns into process.exit. One request per restart killed any instance with SSO enabled. Compared as bytes now, and /api/auth/oidc gained a rate limit. LOGIN CSRF. The callback returned the session token in the URL fragment, so a crafted link installed an ATTACKER'S token and silently signed the victim into their account. The token now goes in a one-shot httpOnly cookie exchanged at POST /sso/claim, which a link cannot forge. FRONTEND, dead on arrival twice over. login.js used `await` in a non-async function — a SyntaxError that takes the WHOLE app down, since app.js imports it statically and there is no bundler. And `esc` was never imported, so the org-SSO button could never render; the ReferenceError was swallowed by the catch written for network failures. Both slipped through because `node --check` parses these files as CommonJS and exits 0 on a broken module. The correct check is `node --input-type=module --check`, and all four frontend files now pass it. PUBLIC EMAIL DOMAINS cannot be claimed. A tenant had claimed gmail.com in review, after which every Gmail user typing their address was offered "sign in with your organization" pointing at that tenant's infrastructure — phishing from this product's own login page. server/lib/public-email-domains.js. MICROSOFT multi-tenant is refused rather than silently broken. `common` metadata advertises the literal template {tenantid}, so the issuer never matches and every login already failed; and loosening that check is nOAuth. A tenant GUID is now required, with a loud warning at boot. SSRF: https only, loopback/RFC1918/link-local refused, redirects not followed, and the test endpoint no longer echoes upstream status for a caller-supplied jwks_uri (it was a readable internal port scanner). Also: an omitted email_verified was accepted (the comment already said it should not be); the domain-uniqueness check raced an 8s network call before its insert and is now inside the transaction; same-org duplicate domains were allowed and made routing depend on table-scan order; routing is now ordered; a client secret that cannot be decrypted fails closed instead of silently downgrading to a public client; SSO audit rows were writing the org id into the deviceId column; and /sso/start was capped at 10/min per IP, which would 429 the 11th employee behind a corporate NAT. Adds per-provider editing in the org admin UI (replace-only secrets — never returned, blank means keep, explicit clear) and a Test button that checks discovery, endpoints and signing keys while stating plainly that it cannot verify the client ID, the secret, or the redirect URI registration. ⚠️ STILL MISSING: domain-ownership verification. A claimed domain means "nobody else had claimed it", not "they own it". DNS TXT proof is the remaining control. 1582 tests pass. New regression tests cover the takeover confinement, ordering, fail-closed secrets, the Microsoft refusal and the public-domain blocklist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 18:12:07 -05:00
ScreenTinker	e97228a502	SSO: per-organization providers, configured by the customer ... Instance-wide providers belong to whoever runs the server. These belong to a CUSTOMER: an organization points ScreenTinker at its own identity provider from Settings → Single sign-on, with no environment variable and no restart. The login flow is unchanged. An org provider is resolved through the same oidc-providers.get(slug) the env ones go through, so there is one authorization request builder, one token exchange and one verifier — not a second, less tested path for tenants. That seam is why Phase 1 put provider lookup behind a single function. ⚠️ An org provider is NEVER published. It is not in /api/auth/providers, because listing a customer's IdP would both offer it to people it does not belong to and leak the customer list from the login page. It surfaces only when someone types an address at one of that organization's domains; otherwise the instance-wide buttons are what you get. The discovery endpoint answers with a BOOLEAN and nothing else — no slug, no display name. Returning "yes, Acme Corp SSO" would turn a guessed domain into confirmation that Acme buys this product, and the slug would hand out a working entry point to their tenant. POST /sso/start repeats the lookup server-side and redirects, so the browser never learns which provider it is being sent to until the provider says so, and the address travels in a body rather than in a URL that lands in history, proxy logs and a Referer. Both endpoints rate limited to 10/min. Other properties, each with a test: - slugs are RANDOM, not chosen, so two customers cannot collide on or guess each other's URL - a domain may be claimed by ONE organization; a second claim is refused, so a tenant cannot capture another company's logins - the issuer is verified by live discovery BEFORE the row is written, so a typo is caught at configuration rather than by a user staring at a failed login - client secrets are optional (PKCE), stored AES-256-GCM via lib/secretbox, never returned; an absent secret on update leaves the stored one alone, which is how a settings form that cannot show it avoids blanking it - cross-org access answers 404, not 403, so an outsider cannot confirm that an organization id exists - signing in through an org provider grants membership of that organization, but never changes an existing member's role Verified live end to end: creation against a real issuer, domain normalisation (`@Acme.CO.UK` → `acme.co.uk`), boolean-only discovery, a rejected domain squat, a rejected bad issuer, and 404 for a foreign organization. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 17:33:32 -05:00
ScreenTinker	252854d31e	SSO: one OIDC flow for every provider, and verify the token properly ... The OAuth support that was here could not work and would not have been safe if it had. It could not work: the login page called google.accounts.oauth2 and new msal.PublicClientApplication, and NEITHER SDK WAS EVER LOADED by any page in this app — no script tag, no dynamic import, nothing. Both buttons threw ReferenceError on click. Even had they loaded, the CSP allows scripts only from 'self' and cloudflareinsights, and frames only from self and YouTube, so the libraries and their popups were blocked too. It would not have been safe: both endpoints authenticated with an ACCESS token and neither checked who it was issued for. POST /auth/google fell back to tokeninfo?access_token= and read the email out of the reply; POST /auth/microsoft handed the bearer token to Graph /me and trusted that. Graph and tokeninfo will both describe the user behind a token minted for SOMEBODY ELSE'S application, so any site a user signed into that requested `email` or `User.Read` could have replayed their token here and been issued a session as them. Both endpoints are deleted; nothing is lost, because nothing could reach them. Replaced by ONE generic flow — Authorization Code + PKCE (S256), run server-side, with the provider list resolved through a single function so per-organization SSO can extend it later without a second login path. Google and Microsoft become ordinary configured providers; Okta, Keycloak, Authentik, Auth0 and anything else that speaks OIDC now work with three env vars. Because the exchange happens server-side the browser never talks to the provider, so there is no SDK to load, no client id in the page, and no third-party origin needed in the CSP. Identity comes from an ID token that must survive: signature against the provider's JWKS (asymmetric algorithms only — alg:none and HMAC are refused outright, the latter because the only key we hold is public), `iss` exactly as discovered, `aud` and `azp` matching our client, `exp`, and a `nonce` this server minted for that login. State is compared in constant time against a value in an httpOnly SameSite=Lax cookie, so the callback is CSRF-protected and survives a restart mid-login. Account rules are the ones already in place: a verified email is required, an SSO login never takes over an account that has a password, and a changed `sub` for a known address is refused rather than handing the account to a recycled mailbox. 18 new tests, every one describing something the old code would have accepted: cross-audience tokens, azp mismatch, replayed nonces, alg:none, HMAC forgery, wrong signing key, expired tokens, a discovery document lying about its issuer, and a registry that never leaks a client id or secret to the browser. Verified end to end against Google's real discovery document: the redirect carries response_type=code, PKCE S256, state and nonce, and every callback guard rejects as intended (no cookie, wrong state, no code, provider refusal, unknown provider). ⚠️ TOTP is still not prompted on an SSO login, matching the documented behaviour of the previous SSO and API-token paths. That is a product decision and is left unchanged here rather than altered silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 17:19:28 -05:00
copilot-swe-agent[bot]	f725186905	Add org-level widget sandbox isolation toggle with warnings ... Co-authored-by: ChrisChrome <28414320+ChrisChrome@users.noreply.github.com>	2026-08-10 21:14:42 +00:00
ScreenTinker	28885d1a13	Merge branch 'feat/brightsign-ip-from-js' ... # Conflicts: # server/test/device-controls-hidden.test.js	2026-08-10 15:40:31 -05:00
ScreenTinker	9e5d05aa33	Merge branch 'fix/brightsign-local-ip'	2026-08-10 15:38:56 -05:00
ScreenTinker	9b6f0856e0	Merge branch 'fix/pi-installer-245'	2026-08-10 15:38:56 -05:00
ScreenTinker	08b3d7d404	BrightSign: report IPv6, the attached display and the active video mode ... Follow-on from the Node-stdlib work, guided by BrightSign's own dev-cookbook rather than by guessing at module names. IPv6 costs nothing extra — it comes from the same os.networkInterfaces() call the v4 address does. The column, the API field and the dashboard card have all existed since 1.9.29 and no player has ever filled them; the card is written to appear ONLY when set, precisely so the overwhelmingly v4 fleet does not pay screen space for an empty row. fe80:: is skipped for the same reason 169.254 is — a link-local address is scoped to one interface and cannot be dialled from a laptop across the office. A ULA is kept, because that one is reachable. The attached display and video mode are new columns, and they answer the first question anyone asks about a dark sign: which panel is that, and is the player outputting at all. screen_width/height could not answer it — they are what the PAGE believes it has, i.e. the widget's own geometry. Our XT245 drives a CX101 at 1920x1200@60 while the page reports its own canvas. Per telemetry row rather than on `devices`, because a display can be swapped, unplugged or renegotiated without the player re-registering. MULTI-OUTPUT: the output is chosen by screen number, not hard-coded. A dual-output player registers ONE DEVICE ROW PER OUTPUT (?screen=N → output_index), so each row must report its own panel — otherwise a box driving a lobby TV and a menu board shows the lobby TV twice. Both naming forms are tried: probed on hardware, "hdmi" and "HDMI-1" both resolve to output 1, while a second output that does not exist fails cleanly ("hdmi2" throws from the constructor, "HDMI-2" rejects), so a single-output player reports nothing rather than inventing a screen. That case has its own test. Dashboard: two cards, shown only when the player reports them, like every other card in that block. Verified end to end on the real XT245 (FW 9.1.93.2) — attached_display=CX101, video_mode=1920x1200@60, alongside local_ip 192.168.1.46, 119616 MB disk, 3656 MB RAM and live CPU. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 15:30:40 -05:00
ScreenTinker	def30e6d39	BrightSign: report the LAN address, real disk, memory and load ... Every one of these fields existed in the schema, the API and the dashboard, and every one was NULL or misleading on a BrightSign. The XT245 had 6000 consecutive telemetry rows with local_ip NULL while sitting at a perfectly reachable 192.168.1.46, and reported "1026 MB" of storage for a 119 GB NVMe. The host half (autorun.brs) does collect an address, but nothing the host sends was arriving at all — proven by the storage figure, which was the browser's cache quota rather than any disk. So the page has to read this itself, which is also the half that can be delivered: st-bridge.js is served per page load, while autorun.brs needs a release bump to reach a player. It is Node's standard library, not a @brightsign module. The widget is created with nodejs_enabled, so os and fs are simply there — this is what BrightSign's own dev-cookbook does in html5-app-template (both the .ts and .js variants). Looking for a platform module is the trap, and it cost most of a day: @brightsign/networkconfiguration EXISTS but exposes only callback, getNeighborInformation and enableLeds — no config reader. hostconfiguration has getConfig()/applyConfig() but returns host settings (forwardingEnabled, hostName, loginPassword, nameServers) with no address in them. Both enumerated on the live player, because the JavaScript API doc pages 404 and BrightSign's own roNetworkConfiguration page links to one of the dead URLs. getCurrentConfig() is BrightScript-only. local_ip os.networkInterfaces(), skipping internal and 169.254 ram_total/free os.totalmem() / os.freemem() cpu_usage 1-min load average / core count, as a clamped percentage uptime_seconds os.uptime() — the MACHINE, overriding the page's own performance.now(), so a widget rebuilt by the watchdog no longer hides weeks of real uptime storage_* fs.statfsSync over the mounts under /storage, largest wins (ours boots from NVMe with a dead card slot; others from SD) Dashboard: the RAM and CPU cards were gated on "is this Android?", which was right when Android was the only family that could measure them. They now render for any player that reports the value, so a BrightSign gets them and Android is untouched — including keeping its "--" cards when no reading has arrived, since an empty card is a known state and a missing one reads as "cannot". The BrightSign storage card loses its "player storage" caveat, because the number is now the disk it always claimed to be. Verified on the real XT245 (FW 9.1.93.2): 116.8 GB free of 116.8 GB, 2.68 GB of 3.57 GB RAM, 3% CPU, uptime tracking the machine, local_ip 192.168.1.46 — matching the address found independently by MAC-vendor scan, and a disk figure matching the kernel's own block count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 15:05:06 -05:00
ScreenTinker	c2033b95fb	BrightSign: find the LAN address on any interface, and say when there is none ... The dashboard has a "Local IP" field, the server stores it, the bridge relays it and autorun.brs collects it — the whole path has existed since 1.9.29. It has never once produced a value for a BrightSign. Our XT245 has 6000 telemetry rows with local_ip NULL while sitting on a healthy PoE network at 192.168.1.46, and every other field in the same payload arrives. Confirmed against the device itself over its DWS: the installed autorun.brs is ours (61055 bytes vs 61058 in tree) and contains this exact code, so it runs and yields nothing. Interface 0 alone is not enough. Now walks every interface the platform documents — 0/"eth0", "eth1", 1/"wlan0" — instead of assuming the first answers. The string forms are the point: per the Object Reference an INTEGER interface "must currently exist on the player; otherwise the object-creation function will return Invalid", while the string names carry no such condition. And when nothing answers it now says so on the host log. Silence is what made this invisible for a whole fleet: the column stayed NULL and read as a server-side gap rather than a player that never sent anything. Not fixed blind — the first attempt at this used roDeviceInfo.GetIPAddrs(), which is ROKU's API. BrightSign's roDeviceInfo has no network method of any kind; the string does not occur once in the published Object Reference. It would have raised "Member function not found" from inside SendHostTelemetry, once a minute, forever — while ostensibly fixing telemetry. Caught by checking the docs before shipping, and now added to the deny-list in brightscript-api-surface.test.js so the next person cannot repeat it. Verified the entry bites: injecting the call fails that suite. ⚠️ Untested on hardware. BrightScript has no interpreter outside a player, so this is docs plus block-balance checking. The XT245 is reachable at 192.168.1.46 (DWS on 8080, not 80) to confirm once the package updates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 11:47:23 -05:00
ScreenTinker	6ac272af57	Pi installer: stop advertising what was never installed (#245) ... Three reports from the same operator, two of them the script describing a state it never reached — the same shape as the first round of #245. Guide missing sudo: frontend/guides/raspberry-pi-digital-signage.html said `curl -sL … | bash` while the script's own header and its root check both say `| sudo bash`. The script fails loudly with the right command, so nothing is half-installed, but the guide should not have to be corrected by an error message. Also documents the --player-only form, which the guide never showed. MOTD advertised commands that mode did not create: section 11 creates screentinker-status/update/logs only when PLAYER_ONLY is false, while section 12 wrote an /etc/motd listing all three unconditionally. A Player-Only Pi therefore greeted its operator with three commands that were not on it, at every SSH login. The command list is now appended per-mode. The cheap fix would have been to print nothing on a player. That trades a wrong banner for a machine nobody can inspect over SSH, so Player-Only now gets its own screentinker-status (kiosk state, which server it points at, and whether that server is actually reachable) and screentinker-logs (kiosk). screentinker-update is genuinely not applicable — there is no local server to update — and is not offered. Wayland cursor never hidden: the launcher stated the compositor cursor config was written "below when wayfire.ini exists". It never was — wayfire.ini and hide_cursor each appeared exactly once in the whole script, both inside that comment. unclutter is installed but only runs on the X11 branch, so a Wayland Pi kept a mouse pointer on the sign while the install looked complete. Now configures wayfire's hide-cursor plugin at install time, idempotently and after backing the file up, and says plainly that labwc has no equivalent rather than failing silently. Tests: raspberry-pi-setup.test.js gains a check that no MOTD advertises a command its mode does not install (both modes, extracted from the script rather than re-typed), that a player is not left with zero diagnostics, and that the Wayland cursor claim is backed by code outside a comment. Both mutations verified to fail: putting screentinker-update back in the player MOTD, and removing the hide-cursor write. NOT fixed, and not guessed at: the ALT+F4-on-first-pairing symptom and the reconnect storm. The crash-restore fix those would need is already in 1.9.33 and targets a different symptom, and `observed=6/5 per 10000ms` is six reconnects in ten seconds, which matches neither the solo-widget cycle nor the kiosk RestartSec=10. Both need the kiosk-side log — which, until this commit, a Player-Only Pi had no command to read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 10:50:34 -05:00
ScreenTinker	3333d5e968	Stop Android panels losing controls when they update ... A declared capability set REPLACES the per-platform baseline rather than merging with it, so anything the baseline grants and the player omits is a control the operator loses by updating. Three were being lost. - display.brightness: the per-window dim (setWindowBrightness) is Tier 0 — no permission, no owner, no WRITE_SETTINGS — and MainActivity applies it unconditionally. It was simply never declared. - remote.screenshot / remote.stream: gated on the accessibility service, while captureScreen() falls through to ScreenshotCapture.captureView, a plain view draw with no permission check. A Tier-0 panel lost live view and screenshots by updating, and a GRANTED MediaProjection never became a capability either — consent given, capture working, server still refusing, because nothing re-declared. - system.device_owner: no player declared it, so the server accepted system.kiosk as a stand-in for every Tier-2 command. Declaring the canonical name makes refusals say what they mean; the stand-in can retire one release after this reaches displays. display.power stays conditional on purpose: screen_on works anywhere via a wake lock but screen_off needs owner/admin/accessibility, and a control that sleeps a panel it cannot wake is worse than no control. It is the sole entry in the DELIBERATE set in player-parity-baselines.test.js. Also fixes the capture-bootstrap gate in device-detail.js. It hung off can('remote.screenshot'), which hid the button from exactly the panels that need it. The gate is now Android-and-nothing-else, NOT "Android that lacks capture": /api/devices/:id ships capabilitiesFor(), which flattens declared and baseline into one array, and the android baseline contains remote.screenshot — so a "lacks capture" test hides the button from all ~440 undeclared panels in the field. isAndroidDevice() mirrors platformFamily() with all four signals in order; an Android-test-only helper classified every Tizen TV as Android, since Tizen registers android_version 'Tizen 6.5'. Tests: the suite could not see any of this. Mutation testing showed deleting either capability line, or reverting isAndroidDevice to its buggy form, left all tests green. Added an update-invariant test (declared set vs baseline, with an argued exception list), a test that executes the shipped helper rather than the harness stub, and a legacy-panel test using the shape the API actually returns instead of one it never produces. All four mutations now fail. Verified on a real Android 16 device across all three tiers: Tier 0 captures live video (no accessibility, no MediaProjection, no owner), Tier 1 gains display.power via accessibility, Tier 2 declares system.device_owner and every Tier-2 command delivers. An in-place upgrade from the pre-change build lost nothing and gained exactly these three. Baselines deliberately NOT moved — a baseline entry moves in the release AFTER the one carrying the player fix, once it has reached displays. Parity gaps 3 and 4 were implemented, audited and reverted; docs/player-parity.md records why so the next attempt starts from the traps. Gap 3 (wiring "Force update") meets an unbounded synchronous download against a 120s watchdog and a 3-attempt counter with no version binding, so three presses refuse a panel every future version. Gap 4 (deferring to BS.capabilities()) removes working screenshot/stream from diskless BrightSigns that capture to RAM, over-declares transitions, and rides a probe timeout that discards a late answer permanently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A	2026-08-10 10:31:33 -05:00
ScreenTinker	b9bd83c48f	Fix a boot-time TDZ that bricked a player across reboots ... A BrightSign XT245 on shipped 1.9.32 went dark and STAYED dark. The exit beacon: crashed: Cannot access '_videoCompositingOk' before initialization @ player:3730:12 Boot restores the CACHED playlist and renders item 0 immediately, from a call site ~2300 lines above where `_videoCompositingOk` was declared. When that item was a video carrying a transition, `isVideoBufferable` read the binding while it was still in the temporal dead zone. A TDZ read is a THROW, not a `null`, so the player died during boot. The nasty part is the loop. The offending playlist came from the device's own localStorage cache, so the player never stayed up long enough to receive a corrected one -- every boot re-read the same poisoned cache and died the same way. Rebooting the player, the one remedy an operator has, did nothing. Recovery took editing the served player; nothing reachable from the dashboard would have helped. Fixed by declaring the cache in State, above Boot, where no call path can reach it early. Left a comment at the old site saying why it must not move back -- next to its function is exactly where it looks like it belongs. Not BrightSign-specific: any web-based player could hit it. Prod is not currently triggering it -- the one exposed playlist starts on an image, and the video check short-circuits before the read -- but that is luck, not safety. Reordering that playlist, or a daypart making a video the first active item at boot, arms it for those displays. Found while testing hwz routing for video transitions; the crash is unrelated to that work and reproduced on the unmodified released file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 20:39:14 -05:00
ScreenTinker	047f95f40c	Freeze and copy the live debug log, and unstick the control row ... Three small things off the device page. The control row had margin-top but no margin-bottom, so the buttons sat flush on top of the STATUS card and the destructive ones read as part of the status panel. Freeze is the one with a decision in it: it holds the VIEW still and keeps buffering underneath rather than pausing the stream. The moment you freeze a log to read something is the exact moment the lines that explain it are still arriving, so dropping them would throw away the part you were about to want. Resume replays them in order. The held buffer is capped at the same 500 as the panel, and the status text says how many are waiting -- otherwise a frozen panel is indistinguishable from a device that went quiet, and silence reads as a symptom. Overflow says so too. Copy takes what is on screen (not the held lines -- the paste must agree with the panel) and stamps it with the device and an ISO timestamp, because a pasted log with no device in it is a log nobody can act on. It falls back to execCommand when navigator.clipboard is absent, which is every self-hosted dashboard on plain http: that is not a secure context, and the other copy buttons in this app quietly do nothing there. Clear earns its place next to Copy: without it you always copy 500 lines of history instead of the capture you just made. The hint promised the stream "turns off on its own when the device reconnects", which was never true and is not what happens now -- it turns off when you leave the screen, and on the device after 30 minutes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 19:26:03 -05:00
ScreenTinker	24e430b354	One broken clip, one skip: stop media errors advancing the playlist N times ... Found the day the live debug log started working, which is the only reason anyone saw it. A BrightSign XT245 playing a 40s clip as a SINGLE-item playlist logged four `Video error` events at every loop boundary and then three back-to-back "Playing:" lines, with `play() rejected AbortError` and `muted-fallback play() also failed` in between as the second mount aborted the first. On a one-item playlist that just re-plays the same file, so it looked like nothing. On a real playlist the identical storm skips one item per surplus event. Silently. The operator sees a playlist that drops content and nothing says why. Same family as 234. Two independent defects produced it: 1. `video.onerror` had no once-guard — its sibling in the buffered path has `if (done) return`, this one didn't — so every event scheduled its own nextItem. 2. Every call site wrote `advanceTimer = setTimeout(...)` DIRECTLY. A second write before the first fired ORPHANED the earlier timer instead of cancelling it: still pending, no longer referenced, so renderContent's clearTimeout could only ever cancel the last one. All the others fired. That made a dozen sites capable of leaking a timer, not just the error handlers — so the fix is a scheduleAdvance() helper that clears before it arms, and a test asserting nothing assigns the timer directly ever again. The four error handlers (buffered/non-buffered x video/image) had drifted apart because they were four copies; they now share one mediaFailureSkip(), which also reports the actual MediaError code. The old line logged the DOM event ({"isTrusted":true}) and never touched el.error, so the log could say a video failed but never why. Third guard: an element that is still playable is not a failure. `error` fires with el.error set; an event carrying no MediaError against an element with frames buffered ahead of it did not fail at anything, and discarding a healthy item on that basis is worse than the event being reacted to. Anything genuinely unplayable (no MediaError AND nothing decoded) is still skipped, so a broken clip can never stall the playlist. Verified on the XT245: 150s of playback went from 2-3 advances and an AbortError pair per loop boundary to exactly one advance and zero AbortErrors, and the surviving diagnostic now names the real cause -- `code=3 DECODE`, four raw error events collapsing to one reported failure. All three guards are mutation-tested: removing any one of them fails a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 19:12:16 -05:00
ScreenTinker	c594a1a67a	Make the live debug log work on the web player, and so on BrightSign ... The dashboard's per-device "Debug logging" checkbox has always sent a `set_debug` command. The Android player honours it — DebugLog.* mirrors its tagged lines over the device socket while the box is ticked. The web player never implemented the command at all, so the panel opened, revealed itself, and streamed nothing but the three unconditional reporters (sync, pip, zone). A display could be failing loudly in its own console and look mute from the dashboard. In a browser that is a nuisance — press F12. On BrightSign it is the whole diagnostic surface: no console, no adb, no logcat, a panel on a wall. Rather than hand-instrument eighty-seven call sites to match Android's tag by tag, this streams the ring buffer the error trap at the top of <head> has always filled: every console.log/warn/error, every uncaught error with file:line and stack, every unhandled rejection, every failed resource load. Turning the stream on also REPLAYS that backlog, so the operator sees the failure that happened before they opened the screen — the case they actually came to investigate, and one no log tail gives them. Replayed lines carry their real age, because the dashboard stamps on arrival and 200 lines would otherwise all claim to have happened this second. The bracket prefixes the player already uses ([wall], [bs], [group-sync]) become the tag column, so the panel reads the same shape as Android's, and the panel now colours by level — all four rendered identically before, so the one line explaining the fault sat in a wall of grey. Bounded three ways, because this sink is fed by console.*: - 40 lines/sec, over which lines are COUNTED and reported, not queued - auto-off after 30 min, for the checkbox nobody unticks - the dashboard also switches it off when the operator leaves the screen The reentrancy guard in pushLog is not theoretical: the sink runs inside the console wrapper, so a subscriber that logs anything would recurse until the stack gave out and the player would die of its own diagnostics. BrightSign host lines stand their direct emit down while the stream is on (the console path already carries them) but still go out unconditionally when it is off — the boot report is the one diagnostic nobody can ask for in advance, because it is over before the operator has a device to open. Verified on the XT245 on alpha: 34 lines across 7 tags, backlog replayed with real ages, levels intact, platform line reporting BOS 9.1.93.2 / XT245 / 1920x1200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 18:31:18 -05:00
ScreenTinker	4b6194884b	A BrightSign photographs itself, using BrightSign's own API ... This platform has never been able to screenshot itself. Video decodes onto a hardware plane the DOM cannot read, so an in-page canvas composite comes back with the content missing — the panel reported "Video is playing on the hardware plane and cannot be captured" while playing perfectly. @brightsign/screenshot composites the video and graphics layers, which is exactly the thing a canvas cannot do. It is reached through the same Node require() the widget already exposes — the one that also makes `module` visible to classic scripts, which is what broke the shared UMD modules on this platform. The same quirk caused that bug and enables this fix. WHY THIS WORKS WHERE THE LONG WAY ROUND DID NOT. The obvious route was to ask the HOST to capture through the player's own DWS, because BrightScript can reach it. That is a dead end here: page->host messaging stops working after page load, so the request never arrives — instrumenting the host to echo the reason of EVERY roHtmlWidgetEvent produced nothing at all while the page was posting. This API needs no host, no messageport and no DWS, so none of that is in the path. The host route stays as a fallback for firmware without the module, but it is no longer how this works. The API writes a FILE rather than returning bytes, so it is read straight back with Node's fs and sent over the socket the player already has. TO RAM, NOT TO FLASH. The remote-control view drives this once a second, and a screenshot per second written to the boot flash is a wear-out mechanism with nothing to show for it: the file is read back and deleted microseconds later, so it never needs to be durable. tmp is tried first and real storage only as a fallback for a unit that does not present it. The directory must already exist or the capture fails, so each candidate is checked rather than assumed. Ordering is part of the fix: the native API is tried BEFORE the host route, because trying the dead end first would spend an operator's patience on a 15s timeout before reaching the path that works. Every failure still falls through to the canvas, so a capture never comes back blank. Remote streaming inherits all of it — startStreaming already drives the same captureAndSend — so the live view now shows real video rather than a card explaining why it cannot. Verified on the hardware: a real 960x540 frame of the playing video, captured by the player, delivered to the dashboard over its own socket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 17:28:00 -05:00
ScreenTinker	1ec32197b2	Let a BrightSign host COLLECT its capture request over HTTP ... Server side of the inverted capture path. The host half is not here — see the end of this message. Every other player is TOLD to capture: the server emits device:screenshot-request over the device socket and the page photographs itself. A BrightSign cannot photograph itself. Video decodes onto a hardware plane the DOM cannot read, so an in-page canvas returns a frame with the content missing — which is why that platform has been answering screenshot requests with a card explaining that the video is uncapturable. Only the host, through the player's own DWS, can get a real frame. The obvious way to ask the host is through the page, and it does not work. On an XT245 (BOS 9.1.93.2) page->host messaging is dead after load: instrumenting the host to echo the `reason` of EVERY roHtmlWidgetEvent produced nothing at all while the page was posting, though the boot-time probe round-trips. The registry is not an alternative either — a running BrightScript does not observe registry writes made by anyone else, proven by writing the key externally through the DWS and watching the host ignore it. What the host CAN do is HTTP; it already fetches its own package updates that way. So the direction is inverted: the request waits here and the host collects it. The image comes back over a plain POST, which means a capture will work even when the page is wedged — exactly when an operator most wants to see the screen. Held in memory on purpose. A capture request is worthless a minute after it was made — someone clicked a button and is watching for the result — so persisting it would only add a way to deliver a stale screenshot after a restart. Bounded and TTL'd so a fleet going offline mid-request cannot grow it, and a repeat request REPLACES rather than queues so a 1fps stream builds no backlog. Authenticated with the same device_id + device_token pair the socket uses. /api/brightsign/package is public because a player fetches it before it has any identity; a screenshot is a picture of a customer's screen and belongs to one display. deviceSocket now exposes ONE ingestScreenshot() used by both the socket handler and the HTTP route, so a BrightSign screenshot reaches the dashboard by exactly the route every other player's does rather than becoming a second, subtly different feature. Note those exports must be attached AFTER `module.exports = function setupDeviceSocket`, which reassigns the object — attaching above it silently wipes them, which cost a debugging round. NOT INCLUDED, deliberately: the host-side poll. Adding it to autorun.brs's main loop kills the BrightScript script within seconds of boot — the page keeps playing, because the widget outlives the script, so from the dashboard it looks healthy. Cause unidentified; BrightScript runtime faults do not reach /api/v1/logs, so there is no error text to read. Half a feature that silently takes down the host is worse than none, so the server waits for a host that can safely ask. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 14:15:06 -05:00
ScreenTinker	95b8d1b293	Export shared modules to the browser even when Node is in the page ... Transitions have never run on BrightSign, and it was never a GPU problem. `transitionRuntimeReady()` is a presence check on three globals and touches no WebGL at all. A BrightSign roHtmlWidget is created with `nodejs_enabled: true`, which puts Node's `module` into classic-script scope — so every shared module that exported with an `else` took the CommonJS branch and never assigned its browser global. The runtime was absent before WebGL was ever asked a question. This is deducible from the fleet without touching the hardware: the player pushes system.reboot / display.power / display.resolution / system.self_update only behind BS.hasHost(), which needs require('@brightsign/messageport') to resolve. Our XT245's stored capability row carries all four, so Node integration was live in that page, so the CommonJS branch was taken. Transitions are the least of it. schedule-eval.js had the same shape, and the player falls back to "always active" when ScheduleEval is missing — so per-item DAYPARTING silently stopped applying on that platform and scheduled content played outside its window with nothing in any log. player-media-health.js the same. Four files, all fixed by exporting to BOTH targets rather than either/or. media-mute.js, orientation-style.js and wall-geometry.js already assigned their globals in a separate unconditional block and were never affected; the audit that reached me claimed all seven, and reading them is what separated the four from the three. THE GUARD, WITHOUT WHICH THE ABOVE IS A REGRESSION. Restore the globals alone and BrightSign starts attempting video wipes it cannot supply. On a hardware video plane drawImage(video) succeeds, throws nothing, and paints a fully TRANSPARENT frame — so the wipe fades from nothing, behind a video plane that is still lit. Worse than the hard cut it replaces. The discriminator already existed: videoFrameIsCapturable() probes ALPHA, so a genuine fade-to-black still reads as captured. It was wired into the screenshot path and not this one, which asked isMediaReadable() — a CORS question, "am I allowed to read this", not "did any pixels arrive". Both the outgoing frame and the incoming warm-play snapshot now consult it, cached per platform, defaulting to available while undetermined so a cold start is not crippled. Net effect on BrightSign: image-to-image transitions light up, anything involving video hard-cuts honestly, and dayparting starts working. Full video transitions are reachable later — BrightSign documents that video "captured as a canvas for WebGL processing must be routed to the GPU" via a per-element hwz="off", which keeps hardware decode at an 8-bit/1080p ceiling. That needs the hardware to validate and is not in this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 10:36:50 -05:00
ScreenTinker	0efbc6040e	Pi installer: ask the operator, not the pipe; and stop assuming X11 ... Five defects from #245, all found by a user on a Pi 5 because nothing in this repo has ever executed either of these scripts. THE MENU IGNORED THE OPERATOR. The documented install is `curl … | sudo bash`, which makes stdin the SCRIPT. bash has consumed it by the time any `read` runs, so every prompt got EOF instantly: the mode menu "chose" All-in-One without anyone touching it, and Player-Only could not be reached that way at all. Reported as the menu being skipped, because it was. Prompts now read the controlling terminal, and when there genuinely is no terminal the script SAYS which way it went instead of letting an empty answer look like a decision. X11 TOOLS ON A WAYLAND PI. Pi 5 on Bookworm defaults to Wayland, where xset, unclutter and xrandr are no-ops that log an error and do nothing. The Pi therefore got no blanking suppression and no cursor hiding while looking configured. The launcher now detects the session and branches: X11 keeps what it had, Wayland gets wlopm and --ozone-platform=wayland, and the compositor-side alternatives are documented rather than silently assumed. THE KEYRING PROMPT. "Choose password for keyring" on every boot is Chromium reaching for gnome-keyring. A kiosk has nobody to answer it. --password-store=basic. THE WHITE PAGE ON EVERY BOOT BUT THE FIRST. Chromium restoring a session it believes crashed — a kiosk is killed by shutdown and never exits cleanly, so it returns with a restore surface over the player. That is why ALT+F4 "fixed" it: it closed the surface, not the player. Rewriting exited_cleanly was never enough on its own because the previous window set is replayed from Sessions/, so that goes too. THE BANNER SPELLED THE PRODUCT WRONG. The ASCII art read "Scree Tinker" — the n was missing, and it is the first thing anyone sees over SSH. Also answered the reporter's Overlay FS question in the README, including the part that bites: an All-in-One Pi IS the server, so an overlay discards the database, uploads and JWT secret at every reboot. Safe for Player-Only; needs DATA_DIR moved off the overlay otherwise. The new test generates the kiosk launcher exactly as the installer writes it and runs bash -n over it, because `bash -n` on the outer script cannot see inside a heredoc — a syntax error in there is just text until it reaches a screen. Reported-by: carloblu74 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 09:08:17 -05:00
ScreenTinker	5b069b9665	Probe video asynchronously — the sweep would have blocked the loop per file ... The backfill is right, and it lands on a path that could not carry it yet. deriveMediaMetadata spawned ffprobe and ffmpeg with execFileSync, each with a 15s timeout. Synchronously, those two calls stop the whole server for their duration: no heartbeats, no socket traffic, no HTTP. That was survivable while the only caller was a human-initiated upload — one file, someone waiting on it, bounded by their patience. The boot-time sweep removes every one of those mitigations. It walks the entire library, unattended, on a server with live panels, once per boot. A library of video rows therefore becomes a per-file event-loop stall, which is #240's failure mode — blocked loop, missed heartbeats, panels marked offline, reconnect churn — arriving from our own maintenance instead of from a checkpoint. We spent yesterday removing one of those; this would have added another, on a schedule. So both spawns are awaited instead of blocked on. Both callers already awaited deriveMediaMetadata, so this is invisible to them, and the ingest path stops freezing the server for the length of an upload's probe as a side benefit — that sync ffprobe has been known tech debt for a while. Timeouts are unchanged and still asserted: async is not a licence to hang, or one wedged file stops the sweep dead instead of moving on. Also applied the PR's own phantom-path discipline to the video branch, which still named its thumbnail before the encode: a failed ffmpeg left the row claiming a file that was never written, which is the exact bug the image branch was fixed for two commits earlier. The new test measures the property rather than grepping for it — a timer keeps ticking across a real spawn — so a future edit that reintroduces a sync call fails here rather than in a customer's fleet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 08:42:34 -05:00
ScreenTinker	b00efa4f14	Merge main into thumbnail-backfill	2026-08-07 08:38:56 -05:00
ScreenTinker	9face2fdd4	Show a panel's IPv6, and size the pairing code to the screen it is on ... Two field-reported gaps, unrelated except that both are about being able to read something off a screen. A PANEL'S IPv6 WAS NEVER COLLECTED, LET ALONE SHOWN. DeviceInfo.getLocalIp() filters to Inet4Address, so a v6-only panel reported no address at all and the dashboard rendered a dash for a screen that was perfectly reachable. It now reports both stacks in their own fields: a dual-stack panel genuinely has two addresses and either may be the one you need, so collapsing them into one column would make it mean "whichever interface enumerated first". Link-local (fe80::/10) is deliberately excluded. Every interface has one, they tend to enumerate first, and none can be dialled without also knowing the zone index — so admitting them would fill the field with a string nobody can paste anywhere and hide the address that works. Any %iface suffix is trimmed for the same reason. The 45-char cap the writer already applied is exactly the longest legitimate IPv6 text form, so it needed no change. The dashboard card renders only when a panel actually has a v6 address, rather than showing an empty row to the overwhelmingly v4 fleet. THE PAIRING CODE DID NOT SCALE, WHICH IS WORST WHERE IT MATTERS MOST. Every size on the pre-playback screens was a hard-coded pixel value. A CSS pixel covers a quarter of the screen area on a 4K panel that it does on 1080p, and a sixteenth on 8K — so the 72px code that fills a 1080p screen is a smudge on the 4K wall it was installed on, which is where signage actually goes. What has to stay constant is ANGULAR size, so the root font size is now viewport-proportional and everything on those screens is a rem against it. The code holds 6.67% of screen height at every resolution: 72px at 1080p — bit for bit what it renders today, so nothing changes for the existing fleet — 144px at 4K, 288px at 8K. Verified in a browser rather than by arithmetic: at a 1409px viewport the root computes to 13.0473px, which is 0.926vmin to four decimals. vmin, not vw, because portrait-mounted panels are common here and vw would render a 1080x1920 screen at half size. Clamped at both ends so the dashboard's preview iframe stays legible instead of microscopic and an ultrawide does not get silly. Applied to the web player (which BrightSign also runs) and to Tizen, where a 1920x1080 logical viewport makes it arithmetically identical to the values it replaces — the point being the panels where it is not. A test asserts the scaling cannot reach playback content: the whole safety argument is that only the chrome uses rem, and a stage or zone rule adopting it would start resizing CONTENT, which is a worse bug than the one being fixed. Android is untouched — its pairing code already autosizes within a dp-scaled layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-07 08:06:45 -05:00
Rob K	bfe8a4c907	Backfill missing thumbnails at boot, and say when ffmpeg is absent ... Ingest-time thumbnail generation is best-effort by contract, so a row that misses it stays bare forever: video uploads on a host without ffmpeg (a SYSTEM dependency nothing surfaced), or content from before thumbnails existed. Operators read that as "thumbnails don't work". Two additions. A [MEDIA] startup diagnostic (async probe, cached) states loudly whether ffmpeg/ffprobe were found, mirroring the [EMAIL] block. And a once-per-boot sweep re-derives metadata for local image/video rows with no thumbnail — serial, paced, delayed past boot, unref'd. The sweep's row UPDATE re-checks that thumbnail_path is still empty so it never clobbers a thumbnail written concurrently by the replace flow, removes its just-written file when the row vanished mid-derive, salvages probed dims/duration even when the thumbnail itself failed, and stops after 25 failures per boot so a library of undecodable clips can't turn every restart into subprocess churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0131RYmVh8ePEhparD3mXBhU	2026-08-07 09:30:32 +01:00
ScreenTinker	5fa6b2d07d	A baseline moves when the fix reaches SCREENS, which is not one rule ... Two changes that are really one idea: the parity model treated all four players as if they update the same way, and they do not. WEB AND BRIGHTSIGN GET audio.volume BACK. The audit removed it because v1.9.28's index.html contained the string set_volume zero times, and because the handler read payload.value while the dashboard sends { level }. The second reason 1.9.31 fixed. The first was reasoning from the wrong artifact: this player is SERVED BY THE SERVER, so a browser panel runs whatever build is answering it, not the release its row was created under. There is no browser panel stuck on the v1.9.28 player once the server moves — and prod moved tonight. The slider works on those displays right now while the baseline says it does not, so the dashboard is hiding a working control from every display that declares nothing. BrightSign comes with it, on the same served player. The unit-specific doubt is whether a hwz player's media element is reachable at all — and that is already answered by audio.mute, which this baseline has always claimed: set_volume reaches setMediaVolume() and device:mute-changed reaches currentVideoEl.muted, same element, same path. If hwz swallowed one it would swallow both. TIZEN DOES NOT COME WITH THEM, AND THE TEST NOW KNOWS WHY. A .wgt sits on the panel until somebody updates it. Cutting 1.9.31 put nothing on any screen, so an un-updated Tizen panel still has the broken handler and moving its baseline would resurrect the dead slider on real hardware. The test could not express that. It judged every family against "shipped source", resolved as the newest tag — which is HEAD on a release commit, so tagging 1.9.31 flipped all four biconditionals at once and demanded a baseline change for displays that cannot have the fix yet. Green tree, red build, naming a baseline, with nothing in the diff to explain it. main would have gone red on the next commit whatever it contained; #242 just got there first. So the two families are now modelled separately. Server-served: judged against the working tree, both directions, because both are decidable from the build we are about to serve. Device artifact: judged against the previous release, and only in the over-claim direction — "the baseline claims it, so the shipped player had better implement it" is always true and worth failing on, while "HEAD gained the handler, so add it" is a guess about how many panels have updated. The cost is that a stale entry can outlive the artifact reaching the fleet; that is a judgement about screens, so a person makes it in player-capabilities.js and records why. player-capabilities.test.js carried the same stale reasoning hardcoded, and docs/player-parity.md stated the old facts in four places — a parity matrix that lies being the exact failure this whole model exists to stop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-06 22:54:45 -05:00
ScreenTinker	bf9ad16486	The checkpointer startup line no longer describes a policy it stopped having ... It still read "escalate >16MB or 3 growing runs" after #240 added a size floor and a cooldown to that second rule. Growth-across-three-runs on its own is exactly the half that no longer holds, so the line described a checkpointer that does not exist — and it is the line an operator reads to learn the policy. During an incident it would send you hunting for a blocking checkpoint that the new gates had in fact suppressed. [wal-checkpoint] off-thread checkpointer started (PASSIVE every 15000ms; blocking TRUNCATE when the WAL exceeds 16MB, or after 3 growing runs but only at >=8MB and at most once per 300s; respawn max 5/60000ms) A test now asserts the line reports every knob that governs the decision, since nothing else keeps a log string and the rule it describes in step. Writing it caught its own bug first: anchoring the slice back to `return worker;` matched the idempotence guard at the top of startWalCheckpointer(), not the log below it, so the window was empty and every assertion passed vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-06 22:12:01 -05:00
ScreenTinker	b29e3b4676	Judge the baselines against the PREVIOUS release, not the newest tag ... Cutting 1.9.31 turned a green tree red, and the failing assertion named a baseline rather than the tag that caused it. "Shipped source" was resolved as the newest v* tag. That is wrong at exactly one moment, and it is a moment that arrives at every release: on the release commit the newest tag IS HEAD, so shipped source becomes the working tree, every biconditional inverts, and the build demands BASELINE.web gain audio.volume — for displays that cannot have the fix until this very release reaches them. Tagging a release should not be able to change what the release is allowed to contain. A baseline describes an UN-UPDATED display, so the source it is judged against is the release BEFORE the one being cut. Skip any tag pointing at HEAD and use its predecessor: v1.9.30 here, and the newest tag as before during ordinary development. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-06 21:08:14 -05:00
ScreenTinker	89cdd1052a	CI: give the parity baselines the tags they judge against ... main has been red since 4f7b4e3 for a reason visible nowhere in its diff. The player-parity baselines describe what an UN-UPDATED display can do, so they are judged against the SHIPPED source — `git show <latest tag>:…` — rather than the working tree. actions/checkout defaults to a shallow clone with no tags, so that lookup found nothing and the suite fell back to the working tree. In a tree where today's QA had just fixed the players' set_volume payload bug, the biconditional then demanded that BASELINE.web/tizen gain audio.volume — for displays that cannot possibly have the fix yet. Green locally, red in CI, and the failing assertion names a baseline rather than the checkout that caused it. So fetch the tags in the test job, and make the biconditionals SKIP when there are none instead of asserting against the wrong source. That fallback was never a slightly-early assertion; it was an inverted one. A skipped assertion announces itself, a wrong one does not. Verified both ways: with tags 15/15 pass, in a tagless shallow clone 13 pass + 2 skip + 0 fail (previously 1 fail). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-06 20:54:47 -05:00
ScreenTinker	59489b3b20	#240: stop the morning wave buying itself a blocking checkpoint ... Bold reported loop lag that grew with uptime and reset on restart. The signature they saw — mean = p50 = p99 = max, identical to two decimals — is not a fixed cost paid on every cycle. It is what an IntervalHistogram window reports when it recorded exactly ONE delay: the mean is the raw value, and every percentile returns the bucket ceiling above it. Reproduced against their exact numbers (1329.07 / 1329.59). So the loop took one long turn that swallowed the sampling second, episodically — which is what they later confirmed independently. The turn is ours, and it is now measured rather than theorised. Probing the real worker against a real WAL with one reader mid-transaction: a single main-thread write blocked for 4,936ms behind the worker's wal_checkpoint(TRUNCATE), which then reported WAL 8.8MB -> 8.8MB. TRUNCATE is the blocking form and its locks are held ACROSS connections, so moving it to a worker kept the fsync off the loop but not the lock; and it does not throw when it cannot get those locks, it returns busy=1 having sat on SQLite's 5s busy timeout and reclaimed nothing. Five seconds of stalled loop for zero benefit, and silent. It was reached far too easily. The rule was "escalate if the WAL grew across three consecutive 15s runs" — which any sustained 45-second write burst satisfies. A customer's fleet powering on in the morning does it daily. Two gates, because either alone leaves the hole open. A size FLOOR, so a WAL in the lower half of its budget can't buy a blocking checkpoint it has nothing to reclaim from. And a COOLDOWN, because the floor alone fixes nothing for Bold — their WAL already sits at 6.2MB against a 16MB high-water, above any sane floor, so every burst would still escalate. However long the pressure lasts, our own maintenance may now stall the loop at most once per window. The high-water rule bypasses both and is untouched: a runaway WAL is the one case worth blocking for, so the "WAL cannot grow forever" invariant is exactly as strong as before. A busy TRUNCATE now says so in the log instead of reading like a success. Also softened the adjacent path: when the worker is declared unrecoverable, engageFallback() re-arms inline autocheckpoint on the main connection — a state that is STICKY for the life of the process, i.e. exactly the shape of "degrades with uptime, a restart fixes it". It used to also run an unconditional main-thread TRUNCATE on the way in; that now happens only when the WAL is genuinely over high-water, and the fallback state is served on /api/status rather than being inferable only from a log line that may have rolled. Telemetry, so the next report is self-explanatory: loop_lag carries `samples` (~50 when healthy, 1 when a single turn swallowed the second), `tick_gap_ms` measured on the WALL CLOCK independently of the histogram, and `worst_tick_gap_ms`/`worst_tick_at` — monotone, so five-minute polling can no longer miss an episode. Band semantics are deliberately unchanged. A one-sample window during a real stall is the correct trigger for the shed valve; suppressing it would blind the protection at exactly the moment it is needed. Separately, device_telemetry gets the age sweep it never had. The per-heartbeat row cap only ever trims the device whose heartbeat is being handled, so a device that STOPS reporting leaves its rows behind forever. The new sweep is per-device (rides idx_telemetry_device rather than scanning), chunked and yielding like the device_status_log one, and defaults to 30 days to match the uptime report's own default window — so it cannot remove rows that report would have shown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS	2026-08-06 20:22:21 -05:00
ScreenTinker	60deefd992	BrightSign: ask the volumes for their size instead of trusting the mount check ... GetStorageStatus() is documented for SD:/SSD:/USB: only, so it can never confirm internal flash, and roStorageHotplug may be absent entirely. Gating the probe on it made 'cannot say' read as 'no disk': a player with an NVMe reported 1025 MB, which is the widget's cache quota arriving through the page-side fallback. roStorageInfo is asked directly as a second pass, with the mount check kept first so a removable volume still wins over internal flash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL	2026-08-06 18:40:12 -05:00
ScreenTinker	b4363d8d26	Keep display.power on the Android baseline ... screen_off blanks a fielded panel for real (owner/admin FORCE_LOCK, else the accessibility lock); screen_on is a logged no-op. One capability renders both dashboard buttons, so withholding the pair to hide the dead ON button also takes blank-at-night — the half that gets scheduled — away from every panel that has not updated. Panels that have updated declare for themselves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL	2026-08-06 18:10:52 -05:00
ScreenTinker	4f7b4e3989	Judge capability baselines against the SHIPPED source, not the working tree ... Two tests disagreed after the QA merges, and both were right about their own half — which is what made the disagreement worth resolving rather than silencing. A baseline describes what an UN-UPDATED display can do. The baselines were justified against `git show v1.9.28:<source>` and then asserted against the working tree, so the moment a player's payload bug was fixed the biconditional demanded a baseline change for displays that cannot possibly have the fix yet. A baseline entry moves when a fix SHIPS. It now reads the newest release tag, and falls back to the tree when tags are unavailable (a shallow CI clone), because a missing tag is a worse reason to fail a build than a slightly-early assertion. While fixing it the helper threw a ReferenceError — the require was missing — and its own broad catch swallowed it and quietly compared against the working tree anyway. The catch now rethrows ReferenceError and TypeError. A fallback that hides a programming error is the same failure shape as everything else this QA pass found. The BrightSign assertion encoded the older, more generous baseline: reboot needs the BrightScript host bridge, and an undeclared unit is precisely the one we cannot know has it. What that test is really pinning is that the row still classifies as brightsign rather than decaying to `web` — so it now asserts that, plus the video playback that is genuinely safe to assume. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL	2026-08-06 18:01:24 -05:00
ScreenTinker	ac2389716c	Merge QA: make the parity matrix and the capability baselines true ... # Conflicts: # server/lib/player-capabilities.js	2026-08-06 17:47:28 -05:00
ScreenTinker	c778f050a9	Merge QA: gate the ungated device commands, and stop a register erasing a panel's platform ... # Conflicts: # server/server.js	2026-08-06 17:46:46 -05:00
ScreenTinker	dc056a5a59	Merge QA: Tizen storage that actually works, and BrightSign APIs that exist	2026-08-06 17:46:22 -05:00
ScreenTinker	3b7cd67ef5	Merge QA: working volume on browser players, and mute is no longer collateral ... # Conflicts: # server/player/sw.js # server/test/player-sw-scope.test.js	2026-08-06 17:46:15 -05:00