.pr_agent_accepted_suggestions

PR 1845 (2026-06-04)

[maintainability] `TestSection*_S4_Golden` missing doc comments

---

`TestSection*_S4_Golden` missing doc comments

Several exported Go test functions are declared without an immediate preceding documentation comment that starts with the function name. This violates the exported-symbol documentation requirement and reduces discoverability/consistency in the codebase.

Issue description

The exported test functions TestSectionConfirmDialogs_S4_Golden, TestSectionDatabaseMaintenance_S4_Golden, TestSectionBackup_S4_Golden, TestSectionExportImport_S4_Golden, TestSectionLogSettings_S4_Golden, and TestSectionLogViewer_S4_Golden do not have documentation comments immediately preceding their declarations.

Issue Context

Per the compliance checklist, all exported functions/types must have non-empty doc comments directly above the declaration, and in Go these comments should begin with the identifier name.

Fix Focus Areas

• web/templates/settings_s4_golden_test.go[83-135]

---

---

PR 1828 (2026-06-03)

[correctness] patchRule handler broken

---

patchRule handler broken

`settings.templ` calls `patchRule(...)` from the rule automation-mode `` onchange, but `rule-toggle.js` now defines `patchRule` inside an IIFE and never exports it to `window`, so the handler will throw at runtime and the automation_mode change will never be saved. Even if exported directly, the new rejecting `patchRule` would create unhandled promise rejections from inline handlers unless wrapped. Issue description The Rules tab markup calls patchRule(...) directly (inline onchange), but after extraction patchRule is scoped inside an IIFE and not available globally. This causes a ReferenceError on change and prevents saving automation_mode. Additionally, the new internal patchRule rejects on non-2xx; if you simply expose it as window.patchRule = patchRule, inline handlers that don’t attach .catch(...) will produce unhandled promise rejections and no user feedback. Issue Context settings.templ uses onchange="patchRule(...)". rule-toggle.js defines patchRule as an inner function and exports only other handlers to window. Fix Focus Areas Export a global patchRule that is safe for inline handlers (handles errors and does not leave unhandled rejections). Keep an internal rejecting variant if rollback logic needs it. Suggested implementation approach Rename the existing rejecting function to something like patchRuleStrict (internal). Add window.patchRule = function(ruleID, changes) { return patchRuleStrict(ruleID, changes).catch(function() { if (typeof showToast === 'function') showToast('Failed to update rule.'); }); }. Update internal callers (toggle rollback/config submit) to use patchRuleStrict so rollback still works. Fix Focus Areas (paths/lines) web/static/js/settings/rule-toggle.js[37-53] web/static/js/settings/rule-toggle.js[238-249] web/templates/settings.templ[4263-4269]

---

[security] `showConfirmDialog` unsanitized `msg` HTML

---

`showConfirmDialog` unsanitized `msg` HTML

The code builds an HTML string (`msg`) using values from an API response (`data.library_count`, `data.artist_count`) and passes it to `showConfirmDialog` with `{html: true}`. If those fields can be influenced by user-controlled data, this can enable DOM XSS.

Issue description

settingsDeleteConnection_click() constructs an HTML string (msg) using values from res.json() (data.library_count, data.artist_count) and renders it via showConfirmDialog(..., {html: true}), which can create an XSS sink if the response fields are ever user-influenced.

Issue Context

Even though the values are expected to be numeric counts, they are not coerced/validated before being interpolated into an HTML string that is explicitly rendered as HTML.

Fix Focus Areas

• web/static/js/settings/connections.js[52-103]

---

[reliability] Toggle rollback race

---

Toggle rollback race

`toggleRuleEnabled` and `toggleConnectionFeature` roll back UI state using a captured pre-click value (`isOn`) without any sequencing/locking, so rapid repeated clicks can cause out-of-order failures from earlier requests to revert the UI to a stale state even after a later request succeeded. This can leave the UI showing the wrong toggle position and disabled/enabled controls until a refresh.

Issue description

The rollback logic for optimistic toggles uses a locally captured isOn value, but does not prevent overlapping requests. If the user clicks quickly (or network is slow), earlier requests can resolve/reject after later ones; a late failure will call applyState(isOn) and overwrite the newer state.

This affects at least:

• toggleRuleEnabled (rule-toggle.js)
• toggleConnectionFeature (connection-feature-toggle.js)

Issue Context

The codebase already demonstrates a safe pattern in romanization-fallback.js: it serializes saves (romanizationPending) and uses a monotonic sequence (romanizationSeq) to ignore stale completions.

Fix Focus Areas

• Add per-button in-flight guarding (disable the switch / ignore clicks while pending), OR
• Add request sequencing (increment a seq on each click; in .then/.catch only act if seq matches current), optionally with AbortController to cancel the prior request.

Fix Focus Areas (paths/lines)

• web/static/js/settings/rule-toggle.js[55-90]
• web/static/js/settings/connection-feature-toggle.js[27-75]
• web/static/js/settings/romanization-fallback.js[24-92]

---

---