fix: characterization-pass follow-up defects (#149, #150, #155, #156)

[mairas]

What

Fixes four correctness defects surfaced by the #27 characterization pass (PR #147/#146). Each fix intentionally re-pins the characterization test that previously asserted the buggy behavior.

• dashboard.service: delete() leaves activeDashboard pointing at a shifted index #149 — dashboard.service.delete() index bookkeeping. Deleting a lower-indexed dashboard left activeDashboard pointing at a different dashboard (only end-overflow was clamped). Now the active index follows its dashboard down and clamps. Folds in two same-file siblings: setActiveDashboardIndex accepted fractional indexes (a URL id like 1.5 passed the bounds check), and two console.error prefixes in duplicate() read "Dashboard Service]" with no opening bracket.
• dashboard.service: blank-dashboard seeding aliases DefaultDashboard's configuration array #150 — blank-seed array aliasing. Constructor blank-dashboard seeding shallow-spread DefaultDashboard, so the new dashboard's configuration array was the same object as the module-level constant; in-place widget edits corrupted the shared default. Now deep-cloned (same defect class as the already-fixed [PCS-05] Default-config singletons are mutated by reference instead of cloned #35/app-initNetwork mutates the shared DefaultConnectionConfig singleton by reference (sibling of #35) #100).
• data.service: timeout reset only reaches the first registration for a path #155 — data.service.timeoutPathObservable first-match only. Used _pathRegister.find(item => item.path == path) (loose ==, first match); when a path was registered under multiple $sources only the first reset. Now iterates all registrations via _pathRegisterByPath.get(path).
• signalk-requests: 500/502/504 described but routed to unknown-status toast; 200-message typo #156 — signalk-requests 500/502/504 routing + 200 wording. 500/502/504 had descriptions but were routed to the "Unknown Request Status Code" toast (which then printed the description it claimed not to know). Now handled as known server errors surfaced as an error toast with their description. Also fixes the 200 message typo ("successfully" → "successful").

Not included: #148

#148 (prev/next navigation inverted) was requested but is already fixed on main by c5b858c4 (2026-07-04), which corrected both cycling methods and their navigateTo* siblings, did the caller audit, and re-pinned the direction assertions. That commit recorded Closes #184, leaving #148 open as a stale duplicate — it should just be closed.

Verification

• npm run lint — clean.
• ng test on the three affected specs — 61/61 pass. Each updated/new assertion is a genuine regression catch (fails on the pre-fix code).
• npm run build:dev — clean.

CI runs the full matrix (lint, vitest 20/22/24, build 20/22/24).

🤖 Generated with Claude Code

[mairas]

Code review — multi-persona

Scope: 6 files, core services (dashboard, data, signalk-requests) + specs, diffed against main.
Intent: four correctness fixes from the #27 characterization pass (#149, #150, #155, #156), each re-pinning the test that formerly asserted the buggy behavior.
Reviewers (6): correctness, testing, maintainability, project-standards (always-on) + reliability (timeout/error paths) + adversarial (delete index arithmetic).

Findings are confidence-gated (≥0.60) and deduped; cross-reviewer agreement noted. All in-scope findings were addressed in the pushed commits; two pre-existing/out-of-scope items were filed as issues.

P2 — High

#	File	Issue	Reviewer(s)	Conf	Resolution
1	dashboard.service.ts:196	delete() with an out-of-range/negative itemIndex shifted (or, for a negative index at active 0, set -1) the active index for a delete that removed nothing — a regression vs the old safe no-op; -1 then crashes loadDashboard.	adversarial (correctness concur)	0.68	Fixed — bounds guard at top of delete() matching duplicate()/navigateTo(); test added.
2	dashboard.service.spec.ts	The < boundary that is the #149 fix was unpinned — no test deletes the active, non-last dashboard, so a <→<= regression survived the suite.	testing	0.82	Fixed — added active-non-last delete test (+ out-of-range test).

P2 — High (pre-existing, out of scope → filed)

#	File	Issue	Reviewer(s)	Conf	Resolution
3	dashboard.component.ts:235	Deep-link/reload to an invalid /page/:id (out-of-range, or fractional now that #149 rejects non-integers) leaves activeDashboard null; loadDashboard has no null guard → undefined.configuration TypeError / blank page. Fractional already crashed pre-#149 via dashboards()[1.5].	adversarial	0.66	Filed #205 — pre-existing; component-level, outside this PR's 3-service scope.

P3 — Moderate

#	File	Issue	Reviewer(s)	Conf	Resolution
4	data.service.ts:718	#155 now resets all source registrations for a path. Correct when the whole path is silent, but on redundant-sensor setups one source's timeout nulls a still-live sibling (+ default), flashing it blank and reverting alarm state until the next delta.	correctness + reliability	~0.80	Filed #206 — the all-reset is #155's requested fix and correct for the common case; source-scoping is the follow-up.
5	data.service.spec.ts:341	Test comment narrated the old implementation ("the old first-match lookup…") — violates the repo's comment policy (comments state WHY, never what changed).	maintainability + project-standards	~0.80	Fixed — trimmed to the invariant.
6	dashboard.service.spec.ts:96	#150 clone test asserted only configuration array identity; a shallow [...arr] clone sharing nested widget objects would still pass.	testing	0.85	Fixed — added nested-widget identity assertion.
7	signalk-requests.service.ts:139	Server-error branch: only 500-with-message was tested; 502/504 and the no-message path (detail = description) were uncovered.	testing (reliability/standards/correctness concur)	~0.80	Fixed — parameterized 500/502/504 + added a no-message case.

Not actioned (advisory)

• serverErrorStatusCodes repeats 500/502/504 already in deltaStatusCodes/handledStatusCodes — a three-way sync cost (maintainability, 0.60). Acceptable for three codes; reviewer agreed no change needed now.

Coverage

• Suppressed: findings below 0.60 confidence.
• Residual risk (pre-existing, not filed): the try/catch in updateRequest calls requestStatus$.error(err) on failure, which permanently terminates the shared request-status Subject for the session. splice on a found index effectively can't throw, so trigger probability is very low — noted, not filed.
• Verified by review: the data.service: timeout reset only reaches the first registration for a path #155 timeout key matches subscribePath's key (same normalizedPath); _pathRegisterByPath arrays are app-lifetime (no concurrent-mutation risk); the signalk-requests: 500/502/504 described but routed to unknown-status toast; 200-message typo #156 server-error branch falls through to dispatch+splice with no pending-request leak; 401 stays untoasted.

Verdict

Ready to merge once CI is green. All P2/P3 in-scope findings are fixed in the pushed commits (66/66 targeted specs pass, lint + dev build clean); the two pre-existing items are tracked in #205 and #206.

_{Review by 6 independent persona agents; synthesis deduped and confidence-gated.}