Refactor: cut the narrative out of the longest comments

api/src/auth.js

@@ -251,25 +251,17 @@ function clearSessionCookie(res, secure) {
 
 /* ── Fixed-window rate limiting ───────────────────────────────────────────────
 
-   One counter, two surfaces. Login attempts and the polling routes count the
-   same way and differ only in what they key on, how they word the wait, and
-   whether success clears the count, so the arithmetic lives here once. Two
-   copies of a timing rule is how a pair of them comes to disagree.
-
-   The surfaces stay separate on purpose: one is on the path that can lock
-   someone out of their own dashboard, and a reader of either does not want the
-   other's wording in front of them. */
+   One counter, two surfaces. Login attempts and the polling routes differ only
+   in what they key on and how they word the wait, so the arithmetic lives here
+   once and the surfaces stay separate. */
 
 /* Count this hit against `key`, and say how much of the window is left if it is
    refused.
 
-   Checking and counting are one synchronous step, with no await between, so a
-   burst of concurrent requests cannot all clear the check before any of them is
-   counted. Splitting this into a read and a write is the check-then-increment
-   race, which is why there is no exported way to ask without counting.
-
-   A refused hit is not counted. That is what lets a lockout expire on schedule
-   rather than being extended by the attempts it is refusing.
+   Checking and counting are one synchronous step with no await between, or a
+   burst of requests all clears the check before any is counted; there is no
+   exported way to ask without counting. A refused hit is not counted, so a
+   lockout expires on schedule instead of being extended by what it refuses.
 
    @param {Map<string, {count:number, first:number}>} store
    @param {string} key @param {number} max @param {number} windowMs

api/src/routes/badges.js

@@ -92,14 +92,12 @@ on('POST', '/api/badge-proxy', async (req, res) => {
     const body = JSON.parse(await readBody(req));
     const { url, itemId, skipTls = false } = body;
     if (!url) return json(res, 400, { error: 'url required', kind: KIND.INVALID });
-    /* Rows the user did not retype arrive as secret rows without a value. Fill
-       them from the stored item so a test after reload uses the real credential,
-       without ever sending it to the browser.
+    /* Rows the user did not retype arrive without a value, and are filled from
+       the stored item so a test after reload uses the real credential.
 
        Only when the request targets exactly the saved destination with exactly
-       the saved non-secret rows. Otherwise the request picks the URL while the
-       server picks the credential, and a stored secret would be delivered
-       wherever the caller asked. See secret-scope.js. */
+       the saved non-secret rows, or the caller picks the URL while the server
+       picks the credential. See secret-scope.js. */
     let headerRows = toRows(body.headers);
     let paramRows = toRows(body.params);
     let declined = false;

api/src/routes/config.js

@@ -132,15 +132,9 @@ on('POST', '/api/config', async (req, res) => {
       data.settings.background = data.settings.background || {};
       data.settings.background.apiKey = existing.settings.background.apiKey;
     }
-    /* settings.auth is owned entirely by the /api/auth/* routes, which is where
-       the admin UI changes it. Nothing a config write supplies is kept.
-
-       Merging field by field, as this did, left a gap: the merge only ran when
-       auth already existed, so before a password was ever set an unauthenticated
-       caller could write settings.auth outright. A hand-made passwordHash then
-       locked the install, or crashed it, since verifyPassword did not validate
-       what it was given. Taking the block wholesale closes both, and closes any
-       field added to it later without this line being revisited. */
+    /* settings.auth is owned entirely by the /api/auth/* routes; nothing a
+       config write supplies is kept. Taking the whole block rather than merging
+       field by field also covers any field added to it later. */
     if (data.settings) delete data.settings.auth;
     if (existing.settings?.auth) {
       data.settings = data.settings || {};

api/src/routes/icons.js

@@ -11,22 +11,12 @@ const { fail, KIND, errorBody } = require('../api-error');
    limit is the larger of the two because the request also contains boundaries
    and headers, so cutting it off at exactly the file limit would reject a file
    that is within it. */
-/* A safe, unused filename for an upload.
+/* A safe, unused filename for an upload. Reusing the submitted name overwrote
+   an existing icon that other apps still referenced, so a free name is found
+   instead and returned in the response.
 
-   Two problems with using the submitted name directly.
-
-   It overwrote silently. Uploading logo.svg twice replaced the first, and the
-   replaced icon is still referenced by whichever apps use it, so a tile's
-   picture changed without anyone touching that app. Nothing warned, and there
-   was no way back. Refusing the upload instead would punish a common and
-   innocent case, since a great many icons are called logo.svg; a free name is
-   found instead, and the response already returns the filename so the form shows
-   what was actually saved.
-
-   The name is also tidied. path.basename strips a Unix path, but on Linux it
-   does not treat a backslash as a separator, so a Windows-style name arrived as
-   the literal "..\..\etc\passwd.svg". That cannot escape the icons directory,
-   which is what matters, but it makes for a confusing file to find later.
+   Backslashes are stripped as well as slashes: path.basename does not treat one
+   as a separator on Linux.
 
    @param {string} dir @param {string} raw @returns {string} */
 function safeIconName(dir, raw) {

api/src/routes/system.js

@@ -9,20 +9,10 @@ let _netPrev = null;
 
 /* One /proc/net/dev line, as bytes received and transmitted.
 
-   The name is separated from its counters by a colon, not by whitespace, and the
-   kernel pads the name to a fixed width. Once the receive counter is wide enough
-   the value runs into the colon:
-
-     eth0: 1234567  890 ...        ->  fields line up
-     eth0:123456789012  890 ...    ->  every field shifts by one
-
-   Splitting on whitespace therefore read packets where it meant bytes, off by
-   roughly a thousand, and that is the normal case rather than an edge case: it
-   happens once an interface has carried about 10 MB. Splitting on the colon
-   first removes the ambiguity.
-
-   The name is matched exactly, too. `startsWith` meant a request for eth0 could
-   match eth0.100, and a request for eth could match eth0. */
+   Split on the colon, not on whitespace: the kernel pads the name to a fixed
+   width, so once the receive counter is wide enough it runs into the colon and
+   every field shifts by one. The interface name is matched exactly, or eth0
+   also matches eth0.100. */
 const RX_BYTES = 0; /* field order after the colon, per the header line */
 const TX_BYTES = 8;
 

api/src/secret-scope.js

@@ -1,15 +1,10 @@
 /* When a stored secret may be attached to a request the caller composed.
 
    Two endpoints let the browser post an in-progress config and have the server
-   fill in a credential. Both let the request choose the destination, so
-   restoring on the item id alone turns "can edit config" into "can read every
-   stored credential": point the URL elsewhere, omit the secret, and the server
-   sends the real one there.
-
-   The rule is deliberately blunt: restore only when every non-secret field is
-   identical to what is saved. Comparing only the fields that look like a
-   destination needs an assumption about which fields matter, which is the
-   assumption that failed here. */
+   fill in a credential, and both let the request choose the destination, so
+   matching on the item id alone would send a stored credential anywhere the
+   caller names. Restore only when every non-secret field is identical to what
+   is saved. */
 
 const { secretSpec } = require('./widget-secrets');
 const { toRows } = require('./badge-headers');

scripts/bump-cache-busting.js

@@ -4,15 +4,10 @@ const path = require('path');
 const crypto = require('crypto');
 
 const UI_DIR = path.join(__dirname, '..', 'ui');
-/* An asset reference, with or without a version stamp.
-
-   The stamp used to be required by this pattern, so a reference written without
-   one was invisible to this script and stayed unstamped forever. A browser then
-   kept serving that file from cache after an upgrade while every other file was
-   refreshed, leaving one page running mixed versions.
-
-   The stamp is optional here, so a new reference is picked up the first time
-   this runs rather than being silently skipped. */
+/* An asset reference, with or without a version stamp. The stamp is optional
+   on purpose: requiring it made a reference written without one invisible to
+   this script, so it stayed unstamped and that one file kept being served from
+   cache after an upgrade. */
 const REF_RE = /(["'])(\/(?:css|js)\/[a-zA-Z0-9_.-]+\.(?:css|js))(\?v=[0-9a-zA-Z]+)?/g;
 
 function listFiles(dir, exts, out = []) {

scripts/check-frontend-paths.js

@@ -1,17 +1,11 @@
 #!/usr/bin/env node
-/* Every module under ui/js needs two entries in tsconfig.frontend.json: the
-   plain path and the cache-busted one.
+/* Every module under ui/js needs two entries in tsconfig.frontend.json, the
+   plain path and the cache-busted one, because TypeScript allows one wildcard
+   per pattern (TS5061).
 
-   TypeScript allows at most one wildcard per pattern (TS5061), so "/js/foo.js"
-   and "/js/foo.js?v=*" cannot be globbed into a single entry. The frontend
-   typecheck is the only thing that resolves those specifiers back to real
-   files, and a module missing from the map is not a loud failure: the import
-   resolves to nothing, that module goes unchecked, and typecheck:ui still
-   passes. The gap only shows up as a bug nobody caught.
-
-   The file's own header tells an author to add both. This turns remembering
-   into a failing check, which is the point: the alternative is a build step,
-   and this project does not have one. */
+   A missing entry fails silently: the import resolves to nothing, that module
+   goes unchecked, and typecheck:ui still passes. This makes it a failing check
+   instead. */
 
 const fs = require('node:fs');
 const path = require('node:path');

scripts/is-prerelease.js

@@ -1,25 +1,10 @@
 #!/usr/bin/env node
-/* Does this tag name a stable release, or a candidate?
+/* Does this tag name a stable release, or a candidate? The answer decides
+   whether the build moves `latest`, which is what an unpinned pull gets.
 
-   The answer decides whether the build moves the `latest` tag, which is what an
-   unpinned `docker pull` gets. Getting it wrong in one direction ships a release
-   candidate to everyone who never pinned a version; in the other it leaves
-   `latest` behind on a real release.
-
-   The workflow used to ask whether the ref contained a hyphen. That is right
-   for the tags this project actually cuts, and wrong in two ways that matter:
-
-     v1.5.0+2026-08-08   build metadata, a stable release, but it has a hyphen
-     v1.5                not a version at all, no hyphen, so it moved `latest`
-
-   The second is the dangerous one. A mistyped tag should never be treated as a
-   stable release, so anything that is not a valid semver version is reported as
-   a prerelease: refusing to move `latest` is the safe failure.
-
-   Deliberately not api/src/semver.js. That module compares versions for the
-   update check and is forgiving by design, coercing a malformed part to 0 so a
-   strange version from GitHub cannot break the dashboard. Here, malformed input
-   is the case that has to be caught rather than smoothed over. */
+   Anything that is not valid semver is reported as a prerelease, so a mistyped
+   tag such as `v1.5` cannot move `latest`. Not api/src/semver.js: that one
+   coerces malformed input rather than rejecting it. */
 
 /* semver.org's own recommended pattern, with an optional leading v for git tag
    style. Note what it rejects: leading zeroes in the core, an empty prerelease

ui/js/admin-logic.js

@@ -129,10 +129,9 @@ export function widgetConfigMode(type, reg) {
 
 /** Which admin section to show, given a requested id and the sections present.
 
-    Exactly one section must always show. The requested id can be stale, since it
-    comes from localStorage and can name a section an older version had, and
-    falling through leaves the page blank. Falls back to the first available
-    section rather than a hard-coded name.
+    Exactly one section must always show, and the requested id comes from
+    localStorage so it can name a section an older version had. Falls back to the
+    first available section rather than a hard-coded name.
 
     @param {string|null|undefined} requested
     @param {string[]} available in document order
@@ -191,17 +190,11 @@ export function reorderItems(items, item, dir, { folderId = null, childIdx = nul
   return true;
 }
 
-/* The reasons a widget was refused, one line per reason, ready to be written as
-   text. Kept here rather than in the form so the two places that show refusals
-   cannot word them differently, and so the shape coming off the API can be
-   checked without a DOM.
-
-   The shape is request-adjacent: it is built by the server from files on disk,
-   but it reaches this code as JSON, so an entry that is not a named widget with
-   at least one string reason is dropped rather than rendered as "undefined".
-
-   `withName` distinguishes the two callers: the editor is already showing one
-   widget, the picker is listing several. */
+/* The reasons a widget was refused, one line per reason, so the two places that
+   show refusals cannot word them differently. The shape arrives as JSON, so an
+   entry that is not a named widget with at least one string reason is dropped
+   rather than rendered as "undefined". `withName` separates the two callers:
+   the editor shows one widget, the picker lists several. */
 export function rejectionLines(rejections, { withName = true } = {}) {
   if (!Array.isArray(rejections)) return [];
   const out = [];

ui/js/dashboard-logic.js

@@ -24,13 +24,9 @@ function fingerprint(c) {
 
 /** Where to send the browser once first-run setup is finished.
 
-    A fresh install has no items, and the dashboard is where items are shown, so
-    landing there shows an empty page with nothing to act on. Admin is where the
-    first item gets added, and is the page the user would have to open anyway.
-
-    Only the automatic step after setup. Opening `/` afterwards is not blocked,
-    and an empty dashboard reached deliberately stays empty: there are no
-    placeholder items to delete.
+    A fresh install has no items, so the dashboard has nothing to act on and
+    Admin is where the first one is added. Only the automatic step after setup:
+    opening `/` afterwards is not blocked, and an empty dashboard stays empty.
 
     @param {unknown} items the dashboard items the page was built from
     @returns {string|null} a path to navigate to, or null to stay put */

ui/js/link-url.js

@@ -1,11 +1,9 @@
 /* Which URLs are safe to put in a link the user can click.
 
-   A denylist on purpose: a tile link is handed to the OS, and a homelab
-   dashboard legitimately links to ssh://, vnc://, rdp:// and whatever else the
-   user has registered. Only the schemes that execute script in our own origin
-   are refused, and rel="noopener noreferrer" does nothing about those.
-
-   Enforced on save and again on render, since a config can arrive by import.
+   A denylist on purpose: a tile link is handed to the OS, and a homelab links
+   to ssh://, vnc:// and whatever else the user registered, so only the schemes
+   that execute script in our own origin are refused. Enforced on save and again
+   on render, since a config can arrive by import.
 
    The server requires this file directly, so it must stay free of anything only
    a browser has: no DOM, no window, no imports. */