[centralcmd]

Summary

This implements issue #348 (Weather Integration — Backend, Draft v7, with its approval amendments) and the matching frontend from the design-system weather pass. It also adds Odyssey.Worker, a dedicated background-job host; the weather schedule is its first job. publish-images.yml gains a worker matrix leg, so releases publish odyssey-worker beside the other three images.

Backend

• Data model. Seven new tables:

  • WeatherLocations, for the per-place settings.
  • WeatherLocationSites, an append-only history of the frozen 3-decimal sites.
  • WeatherLocationSchedules, holding the lease and the Version fencing token.
  • WeatherForecastRuns, WeatherForecastSteps and WeatherObservations, all append-only.
  • WeatherDays, derived from the above.

  Location keys are RESTRICT, and there is a composite RESTRICT key to the current site. Attribution columns get SET NULL keys. One additive migration, AddWeather.

• The caller's weather location is a UserPreferences row under the server-owned key weather-preferences, stored as {"defaultLocationId": "…"}.

  • Only one write path. Only PUT /api/weather/preferences writes it, and it validates the value. The generic PUT /api/user-preferences/{key} refuses the key with a 400; the comparison ignores case and trailing spaces, to match the column's collation.
  • Its own row. The Preferences page saves its whole value at once, so a separate row keeps that save from overwriting the weather choice.
  • No database key to Location. Every read re-checks that the stored location is selectable, otherwise it falls back to the household default. An unparseable row reads as no preference.
  • User deletion. The row is deleted with the user, as every UserPreferences row already is.

• Sources. Two clients: Locationforecast 2.0 (2 MB cap) and Frost (1 MB cap, Basic auth). Both have a compile-time base address, no redirects and no HttpClient logging. Provider bodies are never relayed.

• Symbol codes. All 83 codes in MET's legend are kept. The stored code is capped at 48 characters; the longest legend code is 42. MET's two doubled-"s" thunder bases (lightssleetshowersandthunder, lightssnowshowersandthunder) resolve to their real base in both the design system and the client.

• Schedule (WeatherScheduleJob, run by the worker). It wakes every 60 s to find places that are due, using the database only. A scheduled place is fetched once per refresh interval (default 24 h), at 1 request/s or less, and only places within the tracked cap are fetched on a schedule.

  • Backoff is 15 min × 2ⁿ, capped at 6 h. A 403 or 429 goes straight to the cap, and a longer Retry-After wins.
  • A claim takes a 2-minute lease plus a Version rotation. Disable, apply and purge also rotate Version, so a fetch already in flight is discarded instead of stored.
  • Each call sends one request: the Frost station lookup is its own call, never combined with the daily fetch.
  • Nothing in the API or any test host runs the schedule; the API fetches only on demand and for the Admin refresh.

• Endpoints.

  • The 13 endpoints of §5.
  • The event and entry reads stack calendar.read or journal.read with weather.read.
  • Rate limits: RateLimiting:WeatherRead (120 / 60 s per user) and RateLimiting:WeatherRefresh (6 / 5 min per user).
  • Reads pick the effective step in SQL (an anti-join on the composition index), so a read returns one row per hour however many runs are stored.

• Purge. It goes through IWeatherHistoryPurger.

  • The relational implementation uses ExecuteDelete in batches of 10 000, in amendment 1's order.
  • The fast test tiers use a tracked RemoveRange implementation.
  • The purge is fenced by PurgeStartedAt and can be resumed after an interruption.

• Settings.

  • WeatherUserAgentContact: email or https URL, fails closed.
  • WeatherRefreshIntervalMinutes: default 1440 (once a day), 30–1440; a degraded read resolves to the larger value.
  • WeatherMaxTrackedLocations: 1–20.
  • WeatherDefaultLocationId: must be selectable, checked inside the settings transaction.
  • WeatherFrostClientId: a secret of kind RotatableCredential.

• Claims. Five new claims; weather.locations.* and weather.history.delete are Admin-only. The CLAUDE.md count is now 106.

• Shared rule. The coordinate rounding and the fingerprint (amendment 2) are WeatherRules in Odyssey.Dtos, so the client uses the same rule as the server rather than a copy of it.

• Docs. docs/deployment.md gets the release notes, the data flow and the disable → purge erasure runbook.

• Data export. Only WeatherLocations is exported. The history tables are excluded, each with its reason.

Background-job host (Odyssey.Worker, with its own container)

• A job is an Odyssey.Core.Jobs.IPeriodicJob. It is written and tested in Core like any other business service; the worker only hosts it.
  • Registering a job is one AddPeriodicJob<TJob>() line in Odyssey.Worker/WorkerHost.cs.
  • PeriodicJobService owns the loop. The first run is immediate, then one run per interval. A failed run is logged and the next goes ahead, and runs never overlap.
• Every replica runs every job. A job coordinates through the rows it works on, never through process state, so the worker has no in-memory database mode.
• Shared code moved from Odyssey.Api/SystemSettings to Odyssey.Core/SystemSettings, because a second host can't reference a web project:
  • IntSettingResolver, the weather settings lookup, and the secret registry and reader.
  • Both hosts use the same registrations (AddWeatherFetching, AddSecretSettingsReading), so their service graphs can't drift.
  • TimeZoneDataGuard moved to Core and runs in both hosts.
  • The cache-expiry source lint now scans the Core folder as well.
• Key ring: read-only. This was the product owner's choice.
  • The worker mounts dataprotection_keys :ro, and automatic key generation is off, so the API stays the only key writer.
  • Holding the ring makes the worker the second container that can decrypt every stored credential. docs/deployment.md → Two containers hold the key ring records the trade-off and the incident response.
  • The migrations job still holds no key ring.
• Wiring.
  • Compose (dev and prod, the prod service hardened), the CA override, Aspire and the release workflow all include the worker.
  • The worker image installs tzdata and creates the keys directory owned by app.
  • The settings cache in the worker picks up a /settings change within its 30 s TTL.
• Guard tests updated. The key-ring guards now pin one read-write holder (api), one read-only holder (worker) and none anywhere else. The CA-override guards cover the worker image.

Frontend (Odyssey.Client/Pages/Weather/, wwwroot/css/weather.css)

• Dashboard card. Current conditions, up to nine days, yesterday's measured values and MET's attribution as the response serves it.
  • The location picker saves the caller's own default.
  • Each empty state names its way out, and there is no refresh button (§7.14).
  • The day strip starts at the place's own today.
• Locations, Admin only.
  • A row chip, a Weather record section and the row-menu items. An action that isn't available is absent, not dimmed.
  • Three dialogs: Turn on / Weather settings, Apply new position, and Delete weather history.
  • Delete needs the place's name typed to confirm. After a 504 it re-reads the row and can continue the deletion.
• System settings. A Weather card with the four settings and the Frost secret. The household default is a select whose options are served at read time.
• API client. IWeatherApiClient in Odyssey.ApiClient, with no UI dependencies.
• Design-system sync. Recorded at 1792835567-d10d. The exchange-rate-sync design and the revised Alert, Button, Modal, RecordTable and Switch atoms are not part of this issue, so they stay in pending.

Test robustness (files from main)

Three client tests from main failed intermittently on this PR's CI runs. Each one asserted before an async handler had finished, and each is now robust:

• LocationDialogTests.Typing_again_takes_the_previous_results_out_of_reach: the second search is held pending.
• LocationDialogTests.Arrowing_down_brings_the_active_option_into_view: now waits for the call with WaitForAssertion.
• ContractSignatureSurfaceTests.An_unrelated_edit_saved_through_the_dialog_leaves_both_stamps_intact: now waits for the call with WaitForAssertion.

Deviations from the spec

• WeatherLocationSchedule.ForecastExpiresAtUtc is added. It lets an Admin refresh honour MET's Expires and any pause without also waiting out the refresh interval.
• Day coverage now counts a period that reaches into the day from the previous date, for example a 6 h step starting at 18:00 UTC on an Oslo winter day. That period's values still count only on its own day. Live MET data had shown "23 h" on every day before this fix.
• The refresh interval defaults to 24 h (the spec said 60 min) and its maximum is 24 h (the spec said 6 h), at the product owner's request.
• stale is also true when the newest forecast is older than 3 × the interval (never under 3 h). With the 24 h default, that is 72 h.
• The caller's weather location is a UserPreferences row rather than the spec's dedicated table, at the product owner's request (see above). The wire contract of /api/weather/preferences is unchanged.
• The schedule runs in a dedicated worker container rather than inside the API, at the product owner's request, so the planned background jobs share one host.

Type of Change

• Bug fix
• New feature
• Refactor / code cleanup
• Documentation update
• CI / build / tooling change
• Other (describe below)

Testing Steps

1. dotnet test Odyssey.Core.Tests: the weather tests cover:

   • the recorded Oslo Locationforecast fixture and all 83 MET legend codes (TestShared/Fixtures/met-symbol-codes.txt);
   • the Frost parser, the composer and the timeline (including DST);
   • backoff, claiming and fencing, the SQL step pick, the Version race and staleness;
   • the preference row: it leaves preferences-page untouched, an unreadable value falls back, and the generic write refuses the key in three spellings.

2. dotnet test Odyssey.Api.Tests: the weather tests (403s on every endpoint, the stacked policies, the success paths, enable, apply, purge, refresh 503, both 429s, the schedule job, the settings lookup, preferences and settings), plus PUT /api/user-preferences/weather-preferences → 400, the key-ring and CA-override guards, and the updated guard counts.

3. dotnet test Odyssey.Worker.Tests (9 tests):

   • The job loop, driven by a manual clock: the first run is immediate, the next waits for the interval, a failed run doesn't stop the job, and stopping cancels a run cleanly.
   • The production service graph, built with ValidateOnBuild.
   • The key ring: a secret the API protected is readable, and the key files are left untouched. With an empty ring the worker refuses to protect and creates no key. Both were mutation-checked by removing DisableAutomaticKeyGeneration.

4. ODYSSEY_REQUIRE_TIER=integration dotnet test Odyssey.IntegrationTests: the migration, the RESTRICT keys, the batched relational purge over more than 20 000 steps, resumption, fencing on MariaDB, and the seed.

5. dotnet test Odyssey.Client.Tests: the weather tests (symbols, including every legend code and both doubled-"s" codes; units; the Admin menu; the card's states; the picker keeping focus).

6. dotnet build Odyssey.sln -c Release -warnaserror, as CI runs it.

7. Live Compose stack (docker compose up):

   1. Sign in as Admin, set the MET contact and a Frost client id, and turn weather on for a place.
   2. Within a minute the worker attempted both the forecast and the Frost request. Frost is reached only after the secret decrypts, which shows the read-only key ring works.
   3. Inside the worker, a write to the key directory fails with Read-only file system, and the ring holds only the API's key.

8. Live Aspire run (dotnet run --project Odyssey.AppHost --launch-profile http), done before the worker existed:

   1. Sign in as Admin and set System settings → Weather → Contact.
   2. Turn weather on for a location from Locations → ⋯.
   3. The dashboard card shows the forecast (checked in both themes).

   The worker's Aspire resource is built but has not been run live.

Related Issues

Closes #348

🤖 Generated with Claude Code

https://claude.ai/code/session_013nYmpkMxvSyGGbpvCfpfgG

[centralcmd]

Security Agent review of PR #355

Scope: outbound HTTP to api.met.no / frost.met.no, scheduler, new endpoints and the five weather.* claims, settings and the Frost secret, rate limits, purge, data export, client. No *.csproj / Directory.Packages.props / lockfile / Docker / workflow changes, so no dependency or container findings.

Findings (blocking until fixed or explicitly acknowledged by the team)

1. Medium - read path loads every historical forecast run for the window and resolves the effective step in memory

• OWASP: A04:2021 Insecure Design / ASVS V11.1 (business-logic limits) / API4:2023 Unrestricted Resource Consumption
• CVSS v3.1: 6.5 AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:N/A:H (authenticated; weather.read is held by every role including Guest)
• Where: Odyssey.Core/Weather/WeatherReadService.cs EffectiveStepsAsync (the ToListAsync over WeatherForecastSteps filtered only by LocationId + time window), then WeatherComposer.EffectiveSteps (GroupBy(TimeUtc) ... .First()) in memory. Called by GET /api/weather/hourly, /current and the calendar-event / journal-entry weather reads.
• Why it matters: a new run (about 140 step rows) is stored on every provider update (roughly hourly, floor 30 min), and nothing prunes superseded runs outside a purge (the PR itself sizes this at about 0.8 M rows per tracked location-year). A step for hour T therefore exists once per run issued in the ~10 days before T. A 10-day hourly window (HourlyMaxDays) on a tracked location materialises on the order of 20-50k rows into the API process, per request, at 120 requests/min/user (weather-read), and the limiter is per user, not global. A handful of Guest sessions can drive API memory/CPU and DB I/O (A in CIA), starving the finance paths sharing the instance. /current does the same for a 13 h window on every dashboard load.
• Fix (pick one): (a) resolve the winning row in SQL - a correlated "latest run per TimeUtc" subquery or a ROW_NUMBER() OVER (PARTITION BY TimeUtc ORDER BY SiteVersion DESC, RunProviderUpdatedAt DESC, RunFetchedAtUtc DESC, WeatherForecastStepId DESC) filtered to 1, so only effective rows leave the database; or (b) maintain an IsSuperseded/effective flag (or prune superseded steps older than N hours) when a run is stored, with a retention bound; and/or (c) add an instance-wide weather-read limiter like LocationSearchGlobal as a backstop. Add a test with a realistic run count (e.g. 240 runs) pinning the rows read.

Observations (non-blocking, no action required)

• Outbound hardening is solid and verified in the diff: compile-time base addresses, AllowAutoRedirect = false (so the Frost Basic credential cannot follow a redirect), RemoveAllLoggers() (coordinates are in the URL), response caps counted on decompressed bytes with a time budget, JsonDocument depth limit, range-clamped numerics, regex-constrained symbol_code, Uri.EscapeDataString on Frost-derived station ids, and failures keep no URL/body/exception text. The Frost client id is attached per request to the Frost client only.
• Settings/secret model follows CLAUDE.md: the Frost id is in the encrypted secret store (SystemSettingsSecurityUpdate, RotatableCredential), its destination is a constant (so the "admin-editable destination" exfiltration class does not apply), Unreadable fails closed and is logged hourly at Error. The contact is printable-ASCII/no-space/shape checked on write and re-checked on read (ReadValidator), so CR/LF cannot reach the User-Agent header. The contact is operator-supplied PII sent to a third party by design (MET terms); the settings advisory/disclosure covers it. Nothing in the weather path logs coordinates.
• Authorization: reads gated by weather.read (+ module claim for calendar/journal), admin writes by weather.locations.update, purge by its own weather.history.delete; Admin-only claims reach only Admin via AllClaims. Preferences take no user id (structural IDOR mitigation). Enabling weather requires the location fingerprint (TOCTOU on coordinates) and on-demand fetches are limited to Admin-enabled locations and honour NextForecastFetchAfterUtc, so a reader cannot force an arbitrary outbound request or cause MET hammering. Apply/refresh/purge are rate limited.
• GDPR / data minimisation: site coordinates and the weather history are excluded from the whole-database export and covered by the purge; the export carries settings only. Note for the record that the export gap means a purge is the only erasure route for the site history (the PR documents this as intentional).

Verdict: one Medium finding above. All findings are blocking until resolved or explicitly acknowledged by the team.

---

Generated by Claude Code

[centralcmd]

Accessibility Agent review: changes requested (frontend changes detected).

Scope: Odyssey.Client/Pages/Weather/*, wwwroot/css/weather.css, LocationsPage, Settings (new select row) and Home.razor. This was a static source review. I did not run the app, so findings 3 and 5 should be confirmed in a browser.

What is already good: decorative icons go through OdsMIcon (aria-hidden). Chips, "changed" tags and failure labels carry words, not colour alone. Alerts get role=alert. The dialogs announce their conflicts. Focus rings exist on .wx-attr a and .loc-link. The CSS is token-only, and nothing animates in weather.css.

---

1. Dashboard picker is unmounted while it has focus, so focus is lost

• WeatherWidget.razor.cs ChooseAsync calls LoadAsync. LoadAsync sets _phase = Loading, so ShowPicker becomes false and the OdsSelect leaves the DOM. It remounts after the load, with no focus restoration.
• A keyboard or screen-reader user who picks a place is dropped to <body>.
• WCAG: 2.4.3 Focus Order – Level AA
• Fix: keep the picker mounted while reloading. Make ShowPicker independent of _phase == Ready once the options are known, and show the skeleton only in the body. Or reload without flipping to Loading (a _refreshing flag with aria-busy). Alternatively, restore focus to the picker's id after LoadAsync.

2. "Refresh on a schedule" error is not programmatically tied to the switch

• WeatherSettingsDialog.razor: the _trackedError text sits in .wx-toggle-text as a role=alert. The switch only has AriaDescribedBy="wx-tr-h", and the error is not inside #wx-tr-h. There is no invalid state either.
• It is announced once when it appears. When the user tabs back to the switch, the error is not exposed.
• WCAG: 3.3.1 Error Identification – Level A, and 1.3.1 Info and Relationships – Level A
• Fix: give the error an id and include it in the describedby when present: AriaDescribedBy="@(_trackedError is null ? "wx-tr-h" : "wx-tr-h wx-tr-e")", with id="wx-tr-e" on the error div. Optionally pass an invalid state to the switch.
• Related, low severity (4.1.3 Status Messages – Level AA): the .wx-note "All N scheduled places are in use…" appears dynamically with no status role. Add role="status", or fold it into the describedby text.

3. Settings → "Household default location" select probably has no accessible name or description

• Settings.razor.cs SelectControl passes id and aria-describedby as unmatched attributes to OdsSelect, which splats them onto MudSelect. In MudBlazor those land on the wrapper element, not the inner combobox input.
• The frame's <label for="ss-in-weatherDefaultLocationId"> then points at a non-labelable element, and the helper and advisory text is not associated either. OdsSelect is also given no Label or AriaLabel.
• WCAG: 4.1.2 Name, Role, Value – Level A; 1.3.1 Info and Relationships – Level A; 3.3.2 Labels or Instructions – Level A
• Fix: verify in the DOM. If confirmed, give the select an explicit name, for example a new AriaLabelledBy parameter on OdsSelect forwarded to MudSelect/InputAttributes. It would take TitleId(item.Key) and an aria-describedby that reaches the input. Alternatively use MudSelect's InputId, so the label's for matches. Add a bUnit assertion that the combobox has a name.

4. Failure explanation is hover-only (title)

• LocationWeatherSection.razor, Failure(...): <span class="wx-fail" title="@fail.Help">. The remedy text in fail.Help is unavailable to keyboard, touch and most screen-reader users. For unknown kinds it is "".
• WCAG: 1.4.13 Content on Hover or Focus – Level AA (the content is not dismissible, hoverable or persistent, and not reachable by focus); 1.3.1 Info and Relationships – Level A
• Fix: render the help as visible text or as sr-only text referenced with aria-describedby. For example: <span class="wx-fail"><OdsMIcon …/>@fail.Label</span><span class="sr-only"> — @fail.Help</span>. Alternatively use an existing focusable tooltip pattern.

5. 9-day strip is a scroll container that is not guaranteed keyboard-reachable, and needs sideways scrolling at narrow widths

• weather.css .wx-days { overflow-x: auto; grid-auto-columns: minmax(…, 1fr) }: its children are not focusable. Keyboard scrolling works only in browsers that make scrollers focusable (recent Chromium and Firefox, not Safari).
• At 320 px only about one day is visible and the rest require sideways scrolling.
• WCAG: 2.1.1 Keyboard – Level A; 1.4.10 Reflow – Level AA (advisory; a day strip is arguably a carousel-like exception)
• Fix: add tabindex="0" to the <ol>. Since it is a labelled region, add role="region" (it already has aria-label), and keep the focus ring visible. Better, below 640 px switch to a wrapping grid (grid-auto-flow: row; grid-template-columns: repeat(auto-fill, minmax(…, 1fr))) so nothing needs scrolling.

6. Loading state and live regions

• WeatherWidget.razor skeleton: <div class="wx-skel" aria-label="Loading weather"> is a generic div. aria-label is not valid on a generic role and is ignored by AT. aria-busy on the card is not an announcement either, and OdsLiveAnnouncer only speaks after the load.
• WCAG: 4.1.2 Name, Role, Value – Level A; 4.1.3 Status Messages – Level AA
• Fix: replace it with <div class="wx-skel" role="status"><span class="sr-only">Loading weather…</span>…</div>, or set _announce = "Loading weather…" at the start of LoadAsync.
• The .wx-stale role="status" is inserted together with its content. That is unreliable. Render an empty live region constantly and put the text inside it, or fold it into _announce.
• WeatherPurgeDialog: the run can take up to a minute. The state ("Deleting…") is only a changed Help string and a button spinner, neither announced. Add an OdsLiveAnnouncer, or set role="status" on the help line, with "Deleting weather history. This can take up to a minute." and a completion or failure message.

---

Low severity / best practice (not blocking):

• Duplicate speech in the day cards (1.3.1, advisory). Each li exposes the glyph (role=img, "Light rain"), then "12° / 5°", then the sr-only sentence "Light rain, high 12°, low 5°". Make the glyph decorative here and keep only the sentence. Set aria-hidden="true" on the visible .wx-day-temp spans, or drop the sentence.
• Compass full name via title only (WeatherWind.razor). The abbr title is not reliably exposed. Add an sr-only full name and mark the abbreviation aria-hidden.
• <ol> with list-style: none loses list semantics in Safari/VoiceOver. Add role="list".
• Card title is a div, not a heading (1.3.1 / 2.4.6). This follows the existing OdsCardHeader pattern, but the new dashboard widget would be easier to navigate with role="heading" aria-level="2" on the title span.
• Immediate save on picker change (3.2.2 On Input, advisory). It is mitigated by the live announcement ("This is saved for you only"). Keep that behaviour.

Items 1, 2, 3 and 4 (Level A or AA) should be fixed before merge. Items 5 and 6 are also AA/A, but I rate them lower in practice. Please add tests for the name and describedby on the settings select and the tracked switch.

---

Generated by Claude Code

[centralcmd]

Senior Frontend Agent review of PR #355

Scope: Odyssey.Client/Pages/Weather/*, wwwroot/css/weather.css, the LocationsPage, Settings and Home edits, WeatherApiClient, WeatherClientTests, and the sync record. I checked the client against the design system artifact (version 1792793890-06cb, the same version design-system-sync.json records, so no sync drift). I read the code and did not build or run it. build-and-test was still in progress when I looked, so CI is unconfirmed.

What is good:

• Everything goes through the typed IWeatherApiClient and there is no raw HttpClient.
• ApiClient stays free of UI types.
• The fingerprint, rounding and moved checks reuse the shared WeatherRules, so no server rule is copied into the client.
• Attribution is rendered from the response.
• The tracked cap comes from settings rather than a client constant.
• The new Settings row's claim (SettingClaim.Count, meaning system-settings.update) matches the server registry.
• Symbol, units and copy match components-weather.md and weather-data.js.

Blocking

1. Purge dialog can be dismissed while the purge is running (WeatherPurgeDialog.razor:14,117)

• Standard: the design system says "While running, the dialog cannot be dismissed", and the dialog's own help text says "keep this dialog open".
• Problem: OpenChanged="OpenChanged" is bound straight to the parent. OdsModal.OnVisibleChanged invokes OpenChanged before OnClose (OdsModal.razor.cs:152-156). So Esc, a scrim click or the X closes the dialog through the parent's _wxPurgeOpen. The _running guard in CloseAsync is only reached from OnClose, after the close has already happened. The 504 and "continue" flow then runs with no dialog on screen.
• Fix: withhold the dismiss paths while running. For example OnClose="@(_running ? default : EventCallback.Factory.Create(this, CloseAsync))". OdsModal derives CloseOnEscapeKey, BackdropClick and the X button from OnClose.HasDelegate. Also route OpenChanged through a guard so a stray false is ignored while running. Please verify the options refresh on the re-render triggered by StateHasChanged() in RunAsync.

2. Choosing a location in the picker drops keyboard focus (WeatherWidget.razor.cs:130-154, :101-103)

• Standard: accessibility baseline, WCAG 2.4.3.
• Problem: ChooseAsync calls LoadAsync(), which sets _phase = Loading. ShowPicker requires Ready, so the OdsSelect is unmounted along with the whole card body and focus falls to <body>. The card also flashes to the skeleton on every pick.
• Fix: refresh in place. Keep _phase at Ready, set aria-busy, and swap the data when it arrives. A LoadAsync(silent: true) that skips the Loading phase would do it. The save-announcement ordering stays the same.

3. A failed weather-list load is silent and shows the wrong state (LocationsPage.razor.cs, LoadWeatherAsync)

• Standard: pages surface failures with ItemsOrToast etc. (ApiInteropExtensions); LoadLocations already does.
• Problem: if ListLocationsAsync fails, _weatherRows stays []. Every place then renders as "Weather is off for this place" with a Turn on weather button, and the row menu offers "Turn on weather…" even for places that have weather. The Admin can then open the create dialog on an existing row and PUT its defaults over the real settings.
• Fix: result.ItemsOrToast(Snackbar, "weather settings"). On failure, keep the previous rows or hide the weather section and menu items, so a failed read never looks like "no weather".

Non-blocking

• Settings dialog double-submit window (WeatherSettingsDialog.razor.cs:178-183): _saving = false is set before await OnSaved.InvokeAsync(saved). The parent's OnWeatherSaved awaits a network re-read (LoadWeatherAsync), so Save is live again for that whole window. Keep _saving true until after CloseAsync().
• Refresh does not follow the documented 504 contract (LocationsPage.razor.cs, RefreshWeatherAsync): the IWeatherApiClient remarks say a 504 or dropped connection on refresh means the work may still be running, so re-read rather than report failure. It currently reports "couldn't be refreshed". There is also no in-flight indication or guard, so a double click sends two refreshes and the second hits the 429.
• Purge unmounts its own dialog: OnWeatherPurged removes the row, so @if (… WeatherRow(_wxPurgeLocation) is { } purgeRow) goes false and the dialog is torn down before OdsModal sees the close edge. Its focus restore (odsRestoreFocusOwner) is skipped, so focus is lost after a successful purge. Close first, or keep the dialog mounted until after the close render.
• Skeleton aria-label on a bare div (WeatherWidget.razor:37): without a role this is not exposed. Add role="status", or rely on the card's aria-busy plus the live region.
• Day tiles read three times: the unlabeled WeatherGlyph has aria-label, then the visible 12° / 5° text, then the sr-only DaySentence. Either aria-hidden the glyph and visible temperatures and keep the sentence, or drop the sentence. The full date also lives only in a title on the day name, which keyboard users cannot reach. Put it in the sentence.
• Switch labelling: id="wx-en-l" and id="wx-tr-l" are never referenced. The design system says aria-labelledby the visible title, but OdsSwitch only has AriaLabel and AriaDescribedBy. Drop the dead ids, or add an AriaLabelledBy parameter to OdsSwitch.
• Attribution overflow: .wx-attr { white-space: nowrap } can overflow the card at 320 px. Allow it to wrap on narrow widths. Please check on a phone-width viewport.
• MET codes missing from the symbol table: I recall lightssleetshowersandthunder and lightssnowshowersandthunder (MET's own spelling) in MET's legend. SymbolOf strips andthunder, finds no lightssleetshowers, and shows "Unknown conditions". The design system's symbolFor has the same gap, so per CLAUDE.md fix it in the artifact first and then sync. Verify against MET's weathericon list before acting.
• Settings loads weather options serially (Settings.razor.cs, LoadWeatherOptionsAsync) before ApplyLoaded, in the page's critical path, and swallows a failure. Run it with LoadSecretStatusesAsync via Task.WhenAll. Silence on failure is the documented choice, so this is only about latency.
• Fire-and-forget calls (Retry() and _ = LoadWeatherAsync() in OnLocationSaved): exceptions go unobserved, and two overlapping loads can land out of order. A simple generation counter fixes the ordering.
• Test hygiene and coverage (WeatherClientTests):
  • The static WeatherWidget.InteractiveCheck is set to true and never restored. The other seam tests restore OperatingSystem.IsBrowser() in Dispose.
  • The state-heavy parts have no tests: purge interrupted → resume, the location-changed 409 in the settings and apply dialogs, the isTracked/historyFrom problem mapping, the picker (including the 400), the stale footer, the LocationWeatherSection notice precedence, and WeatherDefaultOptions keeping a no-longer-selectable stored value.

Verdict: three blocking items (1-3) and the non-blocking items above. Not approved until the blocking ones are fixed or explicitly acknowledged. This is a review verdict, not an approval action.

---

Generated by Claude Code

[centralcmd]

Senior Architect Agent review of PR #355 (weather, issue #348)

Reviewed at a6c05fb: entities, configuration and migration, the fetch/read/admin services, purger, MET and Frost clients, controllers, scheduler, settings and secret registries, claims, DTOs, rate limiting and docs/deployment.md. On the frontend I looked only at contract and auth-model implications.

Verdict: changes requested. There is one blocking finding, two should-fix findings and a few non-blocking ones. The security posture is otherwise strong.

What is right

• User-attribution columns on WeatherLocations and WeatherLocationSites are SET NULL FKs and are on the UserAttributionForeignKeyTests allow-list.
• Both outbound destinations are compile-time constants with no redirects and no loggers. The Frost credential goes to frost.met.no only.
• The contact is printable-ASCII only and fails closed on read, so no header splitting is possible.
• No coordinates appear in audit lines, errors or the data export.
• The claim split is sound: Admin-only configuration and a separate weather.history.delete. The role-claim path uses RolePermissions and AllClaims with no migration.
• Migration: additive, identifiers are within MariaDB's 64-character limit, Down() is symmetric, and UserWeatherPreferences is declared out of scope in the export coverage test.
• The version-fence design is coherent. The purge is resumable, with the correct delete order under the RESTRICT composite key.

Blocking

1. Every read materialises every stored run's steps in the window, and nothing prunes them.
WeatherReadService.EffectiveStepsAsync and WeatherFetchService.RebuildDaysAsync select all WeatherForecastSteps in [from, to) and rank them in memory with WeatherComposer.EffectiveSteps.

• Runs are append-only and never pruned. The PR itself puts a tracked location at about 0.8 M step rows a year, with one run per refresh interval.
• A future hour is therefore present once per run issued since it entered the horizon, roughly 240 runs for a hour 10 days out at the default 60 min. A 10-day hourly window then loads about 240 × 240 ≈ 57k rows per request, per user, under a 120/min limit.
• With the interval at its 30 min floor that doubles. The same happens per date on every RebuildDaysAsync after a new run.
• Violates: scalability and the "reads must not multiply a cost any reader can trigger" principle.
• Fix: pick the effective step in the database, with an anti-join / NOT EXISTS over the same total order (SiteVersion, RunProviderUpdatedAt, RunFetchedAtUtc, WeatherForecastStepId), or ROW_NUMBER() OVER (PARTITION BY TimeUtc ...). The existing (LocationId, TimeUtc, SiteVersion, RunProviderUpdatedAt, RunFetchedAtUtc) index already supports it. Add a test with many runs asserting the query returns one row per instant.
• Note: the working tree on this checkout has an uncommitted WeatherComposer.EffectiveStepsQuery doing exactly this. It is not in the PR head, so this finding stands until it is committed.

Should fix

2. A tracked location suspended by the cap serves frozen data flagged as fresh.
ResolveAsync sets Tracked = row.IsTracked, and OnDemandAsync fetches only when Tracked is false. The scheduler only refreshes the first MaxTrackedLocations (Take(...)).

• A tracked place beyond the cap (for example after an admin lowers the cap) is never fetched by the scheduler or on demand.
• StaleAsync is true only when ForecastFailureKind != null. No failure is ever recorded for a place nothing attempts, so the forecast ages indefinitely with stale: false. The same happens if every replica's scheduler is down.
• Fix: derive stale from age, for example LastForecastSuccessUtc older than max(2 × interval, ForecastExpiresAtUtc + interval). Alternatively, let a suspended tracked location use the on-demand path.

3. Admin writes race the scheduler and return 500 instead of 409.
UpsertAsync (disable rotates schedule.Version) and ApplyAsync (rotates it) run SaveChangesAsync under the [ConcurrencyCheck] Version.

• If a claim or fenced save lands between the read and the save, EF throws DbUpdateConcurrencyException.
• GlobalExceptionHandler does not map it, so the response is a 500 with an errorId. PurgeAsync already catches the same case and returns fetch-in-progress.
• The same applies to enable racing a purge fence.
• Fix: catch DbUpdateConcurrencyException in both methods (inside or around InTransactionAsync, after clearing the tracker) and throw WeatherConflictException(FetchInProgress, ...). Add a test for the fetch-wins ordering. The existing test covers only the order where the disable wins.

Non-blocking

4. Storage is unbounded and the cap does not bound it. WeatherMaxTrackedLocations limits the scheduler only. Every enabled untracked place also appends a run per interval whenever someone reads it. No retention or prune exists for superseded runs (the purge is the only deletion path), and the export excludes the history. Consider a documented retention (keep full history for N days, then only the last run per issue day), or at least state this in docs/deployment.md as an operational cost.
5. Scheduler spacing. In the observation loop Task.Delay(RequestSpacing) runs for every due row, including calls that sent no request (the "nothing to send" and no-station paths). Delay only when a request went out, as the forecast loop does with Skipped.
6. Inaccurate comment in RelationalWeatherHistoryPurger. It says steps have no index on LocationId alone. The (LocationId, TimeUtc, ...) index leads with it, so a batched DELETE ... WHERE LocationId = @id LIMIT 10000 is viable and avoids the run-by-run walk (about 3 statements per run).
7. RebuildDaysAsync recursion on duplicate key has no bound. Retry once, as the comment says, rather than recursing.
8. RebuildDaysAsync runs after the fenced save, outside it. A crash between the two leaves WeatherDays stale until a later run stores new data. A 304 never rebuilds. Acceptable, but worth a note.
9. Comment density. Many comments cite spec anchors (§3 step 7, AC 43, amendment 2, N2, SB) that mean nothing without issue Weather Integration — Backend (Draft v7) #348. CLAUDE.md asks for sparing, self-documenting comments, and these rot. Prefer stating the rule in words.
10. UpsertWeatherLocation.Validate uses DateTime.UtcNow for the HistoryFrom ceiling. Fine functionally, but it is the one untestable clock in the weather code, which otherwise injects TimeProvider.

No convention violations found on package versions, field naming, migrations or hand-written role-claim migrations (none needed).

---

Generated by Claude Code

[centralcmd]

❌ Senior Tester Agent: test review found issues

I ran the weather tests on this branch and all pass:

• Odyssey.Core.Tests, weather filter: 66 passed.
• Odyssey.Api.Tests, weather, authorization, settings, export and money-bounds filters: 374 passed.
• Odyssey.Client.Tests: 196 passed.
• Odyssey.IntegrationTests (Docker up, nothing skipped), weather, settings-seed and user-attribution filters: 10 passed.

The tests that exist are good: deterministic (FixedTime, recorded fixtures), they assert real behaviour, and the real-engine tier covers the migration, the RESTRICT keys, the batched purge, purge resume, the in-flight fence and decimal/UTC fidelity. The problems below are untested new behaviour, not wrong tests.

1. WeatherScheduler has no test (untested orchestration path).

• File: Odyssey.Api/Weather/WeatherScheduler.cs, RunOnceAsync (~L50-110) and LogArchivedSkipsAsync.
• RunOnceAsync is documented as "Public so a test can drive it without the timer", but nothing drives it.
• Untested here: the Take(settings.MaxTrackedLocations) cap, the unconfigured early return, the observation loop's break on SecretReadState.NotSet / Unreadable, the hourly throttle on the unreadable log, and ArchivedSkipLogged flag toggling with its swallowed DbUpdateConcurrencyException.
• WeatherServiceTests covers the fetch service and the cap at service level, but the scheduler is the only caller that picks which jobs run.
• Fix: add WeatherSchedulerTests in Odyssey.Api.Tests. Use a ServiceCollection with the existing stub sources and a fake TimeProvider (the Task.Delay(RequestSpacing, time, …) calls then complete on Advance). Call RunOnceAsync and assert:
  • no source call when the contact is unset;
  • at most N forecast calls with N+1 tracked locations;
  • the loop stops after the first NotSet;
  • ArchivedSkipLogged flips once.

2. WeatherSettingsLookup has no tests (untested degraded and clamp paths).

• File: Odyssey.Api/SystemSettings/WeatherSettingsLookup.cs, ~L55-98.
• Only the cache-eviction key list and the bounds guard touch it.
• Untested:
  • an unusable stored contact fails closed (Contact == null, IsDegraded);
  • an unparseable WeatherDefaultLocationId degrades;
  • a degraded result is not cached;
  • the interval resolves to the larger bound and the cap to the smaller one;
  • an absent row is healthy.
• CLAUDE.md requires each lookup to be pinned by SystemSettingsLookupTests / TuningLookupDegradationTests, and SettingsLookupReadPathContractTests does not cover this one.
• Fix: add cases to those suites following the existing lookup tests.

3. The two weather rate-limit policies are untested.

• Files: Odyssey.Api/WeatherRateLimiting.cs, and [EnableRateLimiting] on WeatherController and on apply-location / refresh / purge in WeatherLocationsController (L71, L91, L111).
• The refresh policy permits 6 calls per 5 minutes and bounds outbound MET work. No test shows the 7th call returns an RFC 7807 429, or that the limit is per user.
• Fix: add WeatherRateLimitTests modelled on UserProfileImageRateLimitTests, with a small permit limit and a 429 assertion for both policies.

4. The HTTP success paths for several endpoints are not exercised.

• File: Odyssey.Api.Tests/WeatherApiTests.cs.
• Only the 403 gate or the 404 for an unknown id is covered for these:
  • GET /api/weather/hourly and /daily return no 200 body;
  • GET /api/calendar-events/{id}/weather and /api/journal-entries/{id}/weather have no 200 case; Event_and_entry_weather_need_both_claims asserts only 403 and 404;
  • apply-location is checked only for the "nothing to apply" 409, never for the 200 after a location move.
• The Core tests cover composition, but they don't cover the controller binding, the from/to to LocalDate translation, or the DTO serialization.
• Fix: add one 200 test per endpoint, including a moved location followed by apply and a check that LocationMoved clears.

5. The data-export row content is not asserted (minor).

• DataExportService.WeatherLocationsQuery (new) is covered only by the table-coverage guard (UserWeatherPreferences out-of-scope entry) and the reflection-based document guard.
• Nothing asserts that an enabled location's WeatherLocations row appears in the export, or that no coordinates or history leak into WeatherLocationExport.
• Fix: add a case to DataExportApiTests.

6. Non-determinism (minor).

• WeatherApiTests.StubForecasts builds steps from DateTime.UtcNow, truncated to the hour. A run straddling the hour boundary can change which step is "current".
• WeatherClientTests L118 and L185 use DateTime.UtcNow for IssuedAtUtc / Archived. Prefer a fixed timestamp, or the same TimeProvider the API factory uses.

Items 1 and 2 are the ones to fix before merge: they are new behaviour that nothing exercises, and the first concerns the only code that makes scheduled outbound calls to MET. I did not file separate test-debt issues, since these are PR-blocking findings.

---

Generated by Claude Code

[centralcmd]

This is the response to review round 1. Every blocking finding is fixed in 9fee5e0.

Security

• Medium, the in-memory effective-step pick: fixed. WeatherComposer.EffectiveStepsQuery now does the pick in SQL. It is an anti-join over the (LocationId, TimeUtc, SiteVersion, RunProviderUpdatedAt, RunFetchedAtUtc) index, used by every read and by the day rebuild, so each request returns one row per hour however many runs are stored.
  • It is covered by a test with twenty stored runs, and the MariaDB tier exercises it through the rebuild.
  • The optional instance-wide read limiter is not added. With the query bounded, the per-user limit is proportionate.

Architecture

• 1, the same finding: fixed as above.
• 2, stale by age: fixed. stale is now also true when the newest forecast is older than 3 × the interval (never under 3 h). That covers a place paused past a lowered cap, and a scheduler that is down.
• 3, a Version race returning 500: fixed. Upsert and apply now return 409 fetch-in-progress. A test covers the race.
• 6, the comment: corrected. 7, the recursion: now a single retry.
• 4, retention: not changed. Runs are append-only by design. History export, and any retention policy, are a later item in the spec.
• 5, the 1 s delay after an observation call that sent nothing: not changed. It only costs ≤1 s per enabled place per tick.
• 8, the day rebuild outside the fenced save: not changed. Days are derived, and the next fetch rebuilds them.
• 9, spec anchors in comments: kept. This matches repo practice (§/issue references are common in the codebase).
• 10, DateTime.UtcNow in DTO validation: kept. DTO IValidatableObject has no clock seam anywhere in the repo.

Frontend

• Blocking 1, purge dismissible mid-run: fixed. OnClose is withheld while running, and OpenChanged(false) is refused.
• Blocking 2, picker focus: fixed. The reload happens in place, so the picker stays mounted.
• Blocking 3, silent list-load failure: fixed. A toast is shown, and the weather surfaces stay hidden until the rows are known.
• Non-blocking items fixed: Save cannot double-submit; refresh has a guard and handles a 504 as "may still be running"; the attribution wraps; the test seam is restored.
• Not changed: the serial options load in Settings (one small request) and the Retry overlap.
• Possible missing MET codes (lightssleetshowersandthunder / lightssnowshowersandthunder): the design system has the same gap, so per CLAUDE.md the fix goes in the artifact first. Left as a follow-up.

Accessibility

• Items 1–6: fixed.
  • Picker focus.
  • The tracked-switch error is now in the switch's aria-describedby, and the note has role=status.
  • The settings select is named via aria-labelledby. MudSelect forwards aria-* to its visible combobox, which I checked in the DOM. The for was removed because it pointed at MudSelect's hidden input.
  • Failure help is visible text.
  • The day strip is focusable and has a focus ring.
  • The skeleton has role=status, the stale state is announced through the card's live region, and the purge run is announced.
• Low items fixed: the day tile reads one sentence; the wind expansion is in the accessible text; the ol has role=list; the card title has role=heading.

Tests

• 1, the scheduler: added (the cap, no contact, no Frost secret). 2, the lookup: added (clamp, degrade toward fewer requests, the CR/LF / http / garbage contact failing closed, an unparseable default).
• 3, both 429s: added. 4, 200 paths: added for hourly, daily, calendar event and journal entry, plus apply after a move.
• 5, the export row: not added. The coverage guard and the projection are mechanical.
• 6, the DateTime.UtcNow stub: kept. It serves 48 h of steps from the previous hour, so an hour boundary between stub and read is still covered.

Local results: Core 1810, Api 3053 and Client 3817 tests pass; the weather MariaDB tests pass (6).

---

Generated by Claude Code

[centralcmd]

✅ PR was re-reviewed by Security Agent at 9fee5e0 and no issues were found: Approved.

Finding 1 (Medium, unbounded in-memory effective-step pick) is resolved: WeatherComposer.EffectiveStepsQuery ranks in SQL with an anti-join on the same total order (SiteVersion, RunProviderUpdatedAt, RunFetchedAtUtc, step id) that the in-memory picker uses, so exactly one row per instant leaves the database regardless of stored run count. It is used by WeatherReadService.EffectiveStepsAsync and RebuildDaysAsync. The one-retry bound on the day rebuild and the age-based stale flag introduce no new security surface. The optional global read limiter is not needed now that the query is bounded. Retention of superseded runs remains an acknowledged storage-growth decision, not a security blocker.

---

Generated by Claude Code

[centralcmd]

Senior Architect Agent re-review of PR #355 at 9fee5e0

Verdict: approved. The blocking finding and both should-fix findings are resolved. The explanations given for items 4, 5 and 8-10 are accepted.

• 1 (blocking) - effective step pick: WeatherComposer.EffectiveStepsQuery applies the same total order in SQL. WeatherReadService and RebuildDaysAsync both use it, so a read returns one row per hour however many runs are stored.
• 2 - stale by age: StaleAsync now also returns true when the newest forecast is older than max(3 x interval, 3 h). A never-fetched place stays "no data" rather than stale. This covers a place paused past a lowered cap and a scheduler that is down.
• 3 - Version race: UpsertAsync and ApplyAsync now go through FencedAsync, which clears the tracker and returns 409 fetch-in-progress on DbUpdateConcurrencyException. This matches the purge.
• 6, 7: the comment is corrected, and the rebuild retries once and then leaves the day to the next fetch.

Nits, not blocking:

• StaleAsync now has two stacked <summary> blocks. The old one-line summary is still above the new one and should be deleted.
• StaleFloor is declared mid-class, after the method that uses it. Move it to the top with the other statics.

---

Generated by Claude Code

[centralcmd]

Accessibility Agent re-review of head 9fee5e0: one residual finding. This was a static review of the diff a6c05fb..9fee5e0. I did not run the app.

Verified as resolved

• Finding 2 (tracked-switch error): the error now has id="wx-tr-e" and is included in the switch's aria-describedby when present. .wx-note is now role="status".
• Finding 3 (settings select): the select now uses aria-labelledby, and HasFocusableControl drops the for for OptionsFrom rows. This matches the live-DOM check you reported.
• Finding 4 (failure help): the help is now visible text, not a title attribute.
• Finding 5 (day strip): the <ol> has tabindex="0", role="list" and a :focus-visible ring. .wx-attr now wraps instead of staying on one line.
• Finding 6 (loading and status): the skeleton is a role=status and "Loading weather." is announced. The stale text no longer has a hand-inserted status role and is covered by the load announcement. The purge dialog announces start and failure through an OdsLiveAnnouncer. Dismissal is withheld while the purge runs.
• Low-severity items: the day cards no longer announce the same information twice. The wind compass point has a full-name sr-only text. The card title is role="heading" aria-level="2".

Residual finding

1. Focus is still lost on the dashboard picker while it saves (partially fixed)

• LoadAsync(quiet: true) correctly keeps the picker mounted. But <OdsSelect … Disabled="_saving" /> in WeatherWidget.razor still disables the focused combobox for the whole SetPreferenceAsync round trip. A disabled control cannot hold focus, so browsers drop focus to <body>, and nothing restores it.
• WCAG: 2.4.3 Focus Order – Level AA
• Fix: do not disable the picker while saving. Early-return in ChooseAsync when _saving is set, and rely on the card's aria-busy and the live announcement. Alternatively, restore focus to the select once _saving clears (an OdsFocus helper or the existing window.odsActiveElementId plus a focus call).

Everything else in this PR's frontend looks fine from an accessibility standpoint. Please fix item 1 and I will approve.

---

Generated by Claude Code

[centralcmd]

Senior Frontend Agent re-review of PR #355 at 9fee5e0

I checked out the branch (it was already at 9fee5e0) and read the client diff since a6c05fb. This is a static review. I did not build or run the app, and I did not review the backend changes.

Verdict: all three blocking findings are resolved. No blocking frontend issues remain. The leftovers below are small and can be handled in this PR or a follow-up.

Blocking findings

1. Purge dismissal: resolved. WeatherPurgeDialog passes OnClose as _running ? default : …, so OdsModal drops Esc, scrim click and the X while the purge runs. OnOpenChanged also ignores OpenChanged(false) while running. A new OdsLiveAnnouncer announces the run, the interruption and a failure.
   • OdsModal re-reads its options in OnParametersSet, and EventCallback parameters are re-set on every parent render, so the withheld OnClose takes effect on the StateHasChanged() in RunAsync. I could not confirm that MudDialog picks up the changed options after it has opened. If it doesn't, the OnOpenChanged guard plus the parent re-render restores Open rather than leaving the dialog dismissed.
2. Picker focus: resolved. ChooseAsync now calls LoadAsync(quiet: true). The body and the OdsSelect stay mounted and focus is kept. The skeleton also now has role="status", and the announcement distinguishes loaded from stale.
3. Silent weather-list failure: resolved. ShowWeather = _canReadWeather && _weatherLoaded gates the chip, the section and the weather menu items, so an empty list can no longer offer "Turn on weather" on a place that has it. A failed reload keeps the last known rows and shows a warning toast.

Non-blocking items

Fixed: Save double-submit (_saving is held until after close), refresh in-flight guard and 504/dropped-connection handling, attribution wrapping, test seam restore, and the tracked-error aria-describedby, role="status" on the cap note and wind expansion for screen readers. Also fixed: the visible failure help, role="list", the focusable day strip with focus ring, and the settings select naming. The select now takes aria-labelledby/aria-describedby and no for. The jump target is the title id, so the problems rollup is unaffected.

Left as is:

• MET codes: lightssleetshowersandthunder and lightssnowshowersandthunder still fall to "Unknown conditions". This goes to the artifact first, as agreed.
• Serial options load and Retry/LoadWeatherAsync overlap: accepted.
• Focus after a successful purge: OnWeatherPurged still removes the row, which unmounts the dialog before OdsModal sees the close edge, so focus restore is skipped. Close first, or keep the dialog mounted for one more render.

New nits:

• Overlapping picker saves: ChooseAsync sets _saving = false before the quiet reload. During the reload aria-busy is off and the select is enabled, so a second pick can overlap the first. Hold _saving until the reload completes.

• No client tests for the new behaviours: the client test diff is only the seam restore and the wind markup. Please add small bUnit cases for the following:

  • OnClose has no delegate while _running.
  • LocationsPage hides the weather surfaces until the rows load.
  • A quiet reload keeps the OdsSelect mounted.
  • The settings select has a name.

  The accessibility reviewer also asked for the describedby assertions on the tracked switch.

---

Generated by Claude Code

[centralcmd]

✅ PR was reviewed by Accessibility Agent and no issues were found: Approved.

Re-review of head a44ed2e. This was a static review of the diff 9fee5e0..a44ed2e. I did not run the app.

The residual 2.4.3 Focus Order finding is resolved. Disabled="_saving" is removed from the dashboard picker. ChooseAsync returns early while a save is running, and _saving now clears only after the in-place reload. That keeps the focused combobox mounted and enabled, and aria-busy plus the live announcement carry the busy state. The bUnit test checks that the same select instance survives and is never disabled.

---

Generated by Claude Code

[centralcmd]

✅ PR was reviewed by Senior Tester Agent (round 2, head a44ed2e) and no blocking issues were found.

Round-1 items 1-4 are resolved. I ran the weather tests at this head, and everything passed:

• Odyssey.Core.Tests: 69 passed.
• Odyssey.Api.Tests: 51 passed.
• Odyssey.Client.Tests: 26 passed, three consecutive runs.
• Odyssey.IntegrationTests (Docker up, nothing skipped): 6 passed.

I accept your answers on items 5 and 6.

Non-blocking observations:

1. The scheduler tests don't cover everything. The cap, no-contact and Frost-secret-not-set cases are covered. Still untested: the SecretReadState.Unreadable branch with its hourly throttled error log, and LogArchivedSkipsAsync (flag flip, swallowed concurrency exception).
2. The new picker test can pass before the reload happens. Choosing_a_place_saves_it_and_keeps_the_picker_mounted_and_enabled checks Assert.Empty(.wx-skel) right after the SetPreferenceAsync verify. Wrap the .wx-skel check and the Assert.Same in WaitForAssertion, or wait for the post-reload CurrentAsync call, so a regression to the unmounting behaviour is caught.
3. A time-window assertion is loose. Hourly_and_daily_return_the_fetched_forecast uses DateTime.UtcNow and Assert.InRange(Steps.Count, 5, 7). It is stable, but it is a tolerance band, not an exact count.
4. Unrelated flake. One full-suite Odyssey.Client.Tests run (1 of 4) failed LocationDialogTests.Arrowing_down_brings_the_active_option_into_view (L201). It is outside this PR's diff and passed on the other three runs, so I'm not holding the PR for it.

---

Generated by Claude Code

[centralcmd]

✅ PR was reviewed by Security Agent and no issues were found: Approved.

Scope reviewed: the weather feature (MET/Frost outbound clients, 5 new claims, rate limits, purge, WeatherFrostClientId secret, server-owned preference key) and the round-2 additions: Odyssey.Worker (Program/WorkerHost/Dockerfile), the read-only Data Protection key ring, the secret reader/registry move to Core, compose and prod overlay, AppHost, publish-images.yml. dotnet list package --vulnerable --include-transitive on Odyssey.Worker reports no vulnerable packages. No new package versions were added in Directory.Packages.props.

Verified, no finding:

• Key ring (A02/A05): the worker mounts dataprotection_keys:ro, calls DisableAutomaticKeyGeneration() and uses the same SetApplicationName("Odyssey") as the API. A missing or unreadable ring gives Unreadable, and consumers fail closed with a key-name-only log. The second-holder trade-off is the documented product decision in docs/deployment.md and is not re-litigated here.
• Container hardening (A05): the worker runs as the non-root app user, has no port, and the base images are digest-pinned. The prod overlay sets read_only, cap_drop: ALL, no-new-privileges and a /tmp tmpfs. DOTNET_ENVIRONMENT=Production is pinned, so the placeholder guard and the secret registry's environment filter agree with the API. .dockerignore keeps env files out of the COPY . /src build context, and the proxy CA is not persisted in any layer.
• Outbound clients (A10/A02): base addresses are compile-time constants, AllowAutoRedirect=false, and RemoveAllLoggers() keeps coordinate-bearing URLs out of logs. The Frost credential is attached per request to the Frost client only. Response size is capped on decompressed bytes, JSON depth is capped, and WeatherHttp keeps no URL or exception text.
• Authorization (A01): every weather endpoint carries an action-level claim policy. Configuring locations and purging are Admin-only, and purge has its own claim. Apply, refresh and purge share the per-user refresh limiter, and reads have their own limiter.
• Server-owned preference key: weather-preferences is refused on the generic PUT, both by the requested key and by the row it resolves to, with the collation case/trailing-space equivalence handled. The generic controller has no delete route.
• Secret handling: Unreadable is kept distinct from NotSet, and the reader logs the key name only.

Non-blocking observations (Low, no CVSS meaningful, not gating approval):

1. Least privilege on the DB account (A05 / ISO 27001 A.8.2). The worker reuses the API's full-privilege MariaDB user. The job only needs the weather tables and SystemSettingSecrets/SystemSettings/UserPreferences reads. A separate account with scoped grants would limit blast radius if the worker (which makes outbound calls and parses third-party JSON) were compromised. Worth a follow-up issue.
2. Retry-After is unbounded (WeatherFetchService.Backoff). A longer Retry-After always wins over the 6 h cap. MET is a trusted TLS peer, so this is low risk. A clamp (for example 24 h) would stop a misbehaving response from parking a location indefinitely.
3. Key-ring refresh lag (reliability, not confidentiality). With auto-generation off, the worker only sees a key the API rotates in once its ring cache refreshes (up to about 24 h). Until then a secret protected under the new key reads Unreadable and the job fails closed. This is the intended fail-safe, and a one-line note in the runbook would save a debugging session.
4. Compose password default. The base compose file defaults to odyssey_password for the worker, as the API and migrations services already do. Only the base dev stack is affected, because the prod overlay requires an env file. Not new in this PR.

No findings are blocking.

---

Generated by Claude Code

[centralcmd]

Accessibility Agent review of PR #355 (frontend changes detected). The weather UI is mostly well built: the chart has a keyboard readout and a hidden data table, decorative glyphs are hidden from AT, and focus rings and live regions are present. I found one blocking-level issue and two minor ones.

1. Focus is lost when the purge starts (Odyssey.Client/Pages/Weather/WeatherPurgeDialog.razor, footer and RunAsync)

• Description: Clicking "Delete weather history" sets _running = true. OdsButton renders disabled for Disabled || Loading, so the focused button becomes disabled. The Cancel button and the confirmation field are disabled in the same render. The modal is left with no focusable element, focus drops to <body>, and Esc, scrim and close are withheld. A keyboard or screen-reader user can't tell the dialog is still there, and the focus trap has nothing to hold. The same file's comment in WeatherWidget.razor already calls out this exact hazard.
• Criterion: 2.4.3 Focus Order – Level A (also 2.1.2 No Keyboard Trap – Level A for the focus-less modal state).
• Fix: Keep focus on a focusable element for the duration of the run. Either use aria-disabled and ignore clicks while _running, or move focus to the dialog title or the help text (tabindex="-1" plus FocusAsync) before disabling anything. Also leave the Cancel/Close button enabled once the run is no longer cancellable, or restore focus to the Close button when _running ends on the interrupted path.

2. Dashboard card title uses role="heading" on a span (WeatherWidget.razor, line ~14)

• Description: <span class="wx-ttl" role="heading" aria-level="2"> should be a native heading element. This is a minor point because ARIA does expose the heading correctly.
• Criterion: 1.3.1 Info and Relationships – Level A.
• Fix: Use <h2 class="wx-ttl"> with the same class, resetting margin and font in CSS.

3. odsSetAttr is defined twice in wwwroot/index.html (the later definition wins)

• Description: The later definition sets aria-label="" for an empty string, where the earlier one removes the attribute. That is harmless for the current static AriaLabel values on OdsSelect and OdsDatePicker. But AriaLabel is applied only on first render, so a later change to it is never applied.
• Criterion: 4.1.2 Name, Role, Value – Level A (latent).
• Fix: Keep one definition, and re-apply the label in OnParametersSet or OnAfterRender when AriaLabel changes.

No other issues found: the date pickers have names ("From", "To") in a labelled group, and the chart plot is a single tab stop with Left/Right/Escape. Not run live; this was a static diff review.

[centralcmd]

Senior Architect Agent review: no blocking issues, 4 non-blocking suggestions

Reviewed the backend, API, migrations, worker and infra. Frontend was only checked for contract drift. I read the weather services, purger, schedule job, both controllers, both migrations, the worker host and Dockerfile, the compose/AppHost/CI changes and the DI wiring. I did not run the build or tests.

What checks out:

• SplitWeatherForecastAndHistory is additive and uses add, copy, drop. The Down() is reversible for schema, and the copy is correct (Enabled maps to both halves going up, ForecastEnabled OR HistoryEnabled going down). The table is small, so lock time is not a concern.
• The purge is fenced by PurgeStartedAt and a rotated schedule Version. It is batched, and the final transaction runs inside CreateExecutionStrategy. The IWeatherHistoryPurger split keeps ExecuteDeleteAsync off the InMemory tiers, in line with the CLAUDE.md rule. The HistoryOnly mode lifts the fence and the lease in its final transaction.
• Outbound HTTP has a compile-time base address, no redirects, a decompressed-byte cap, range-validated values before they reach Precision(...) columns, and RemoveAllLoggers() because the URL carries coordinates.
• The worker follows the CLAUDE.md background-job rules. Its key ring is mounted :ro, automatic key generation is off, and DI is shared through AddWeatherFetching and AddSecretSettingsReading. The compose and prod hardening matches the other services (cap_drop, read_only, no new privileges), and the publish-images matrix leg and the CA override are present.
• Authorization is claim-gated (weather.*), with the owning-module claim stacked on the calendar and journal reads. The read and refresh endpoints are rate-limited. No feature toggle was added.

Non-blocking suggestions:

1. Per-location fault isolation in WeatherScheduleJob.RunOnceAsync (robustness). Both loops (forecasts, then observations) have no per-location try/catch. Any unexpected exception from one location, such as a DB error other than concurrency or duplicate key, aborts the whole run. The queue is ordered by EnabledAt, so the same location is retried first on every tick and the locations behind it starve. PeriodicJobService only logs the failure and carries on to the next tick. Wrap each iteration in a catch that logs with the LocationId and continues, rethrowing only on OperationCanceledException.
2. Request spacing is per process (consistency). The "at most one request a second" spacing lives in each replica, but the docs say scaling out is safe. With N replicas the egress rate to MET and Frost is N per second. Either state this in docs/deployment.md or derive the spacing from a shared row.
3. DELETE /api/weather/locations/{id} documentation (API contract). The Swagger summary says "Delete every weather record of a location". When ForecastEnabled is still true, the call is a HistoryOnly purge: the weather row and the forecast data survive. Consider a summary that states both modes, or a dedicated sub-resource such as DELETE .../history, so a 204 on a resource-level DELETE does not imply the resource is gone.
4. Data Protection application name (drift). WorkerHost.DataProtectionApplicationName is a constant that says it "must equal the API's", but Program.cs still passes the literal "Odyssey" to SetApplicationName. A mismatch would be silent, and every secret would read Unreadable. Move the constant somewhere both hosts can reference (for example next to AddSecretSettingsReading) and use it in both places, or add a guard test.

Verdict: approve with the suggestions above. Nothing blocks merge.

[centralcmd]

✅ PR was reviewed by Senior Tester Agent and no issues were found.

Scope: Core (WeatherServiceTests/Parsing/Composition/Harness), Api (WeatherApiTests: permission matrix, 403/404 paths, bounds), Client, Worker and IntegrationTests (WeatherRelationalTests). I ran the weather, worker, preferences and authorization subsets of the Core, Worker, Client and Api test suites and all passed (184, 9, 79 and 151). The Docker tier was not run. Tests use deterministic fixtures and fakes, with no skipped or assertion-free tests and no field-naming violations.

Non-blocking nit: WeatherClientTests.Ok_shows_now_the_days_and_the_attribution_from_the_response builds today from DateTime.Now, mirroring the widget's own ViewerToday. A run straddling local midnight could disagree. An injectable clock would remove that, but I'm not treating it as a defect.

[centralcmd]

✅ PR was reviewed by Security Agent and no issues were found: Approved.

Scope: the full diff, reviewed statically. The branch was not switched.

Checked and found sound

• Outbound calls (A10 SSRF, ASVS V12.6). Both MET clients use a compile-time https://api.met.no/ or https://frost.met.no/ base, with AllowAutoRedirect = false and every HttpClient logger removed. Responses are capped by decompressed bytes and bounded by a time budget, and JSON depth is limited to 32. Nothing user-controlled reaches the host part of the URL, and coordinates are formatted with the invariant culture.
• Header injection (A03). WeatherRules.ValidateContact allows only printable ASCII with no spaces, requires an email address or an https URL, and rejects URL user-info. It is re-checked on the read path, so a stored value that bypassed the write path (hand edit or restore) turns weather fetching off instead of splitting the User-Agent header.
• Frost client ID (A02, A04). It is stored in the encrypted SystemSettingSecrets store and gated by the existing SystemSettingsSecurityUpdate claim. It is attached per request and only to the Frost client, so it never reaches the forecast host. Unreadable fails closed and logs. The destination is a constant, so this is not the exfiltration shape CLAUDE.md warns about.
• Authorization (A01). Admin-only claims (weather.locations.read, weather.locations.update, weather.history.delete) sit in AllClaims only. Every role holds weather.read and weather.preferences.update, which are coherent for shared household locations. The calendar and journal weather reads stack the owning module's claim on weather.read. Rate limits are per user (120 reads/min, 6 apply/refresh/purge per 5 min). On-demand fetches go through the same lease, Expires and backoff as the schedule, so a reader cannot amplify traffic to MET.
• Preferences (A01, mass assignment). The weather-preferences key is refused on the generic upsert path. The check handles the column's case-insensitive and trailing-space collation, and it also covers the case where the row resolved from a differently spelled key. The value is written only through the validated endpoint.
• Purge. It is gated by its own claim and rate limit. A fence sets PurgeStartedAt, a lease and a concurrency version, so a concurrent delete loses with a 409. The purge is resumable, and no raw SQL is involved. Start, resume and completion are logged with the actor id.
• Worker container (A05). The image is digest-pinned and runs as the non-root app user. The key-ring mount is :ro, automatic key generation is off, and the application name matches the API's ("Odyssey"). In docker-compose.prod.yml the worker has no-new-privileges, cap_drop: ALL, read_only and a tmpfs. There is no published port. The proxy-CA secret is mounted on a secret mount and does not persist in a layer. The publish-images.yml change only adds a matrix leg and does not widen permissions.
• Data export / GDPR. The weather provider tables are excluded explicitly with reasons, and WeatherLocations settings are exported. The tables hold coordinates of household places, not personal data about individuals. The purge gives an erasure path.
• Dependencies. No Directory.Packages.props change, and the new project adds no new packages (only Microsoft.Extensions.Hosting, already centrally managed), so there is no new advisory surface.

Observations (non-blocking, no action required)

1. The typed confirmation on the purge is enforced in the client only. DELETE /api/weather/locations/{id} is already guarded by the server preconditions (history off, not the household default, no running fetch), the Admin claim and the rate limit, so this is acceptable. If the API is meant to be driven by other clients, consider requiring a confirmation token or the location name in the request.
2. The worker is the second container able to decrypt every stored credential, including the pseudonymisation derivation key. This is documented in docs/deployment.md and is a deliberate product decision. Keeping the worker to read-only and no-egress-except-MET (where the platform allows) would reduce the blast radius further.
3. In the base docker-compose.yml the worker lacks the cap_drop and read_only hardening that the prod override adds. This is dev-only and consistent with the other services.

[centralcmd]

Senior Frontend Agent review (PR #355, frontend scope)

No blocking issues found. Typed IWeatherApiClient throughout with ApiResult status handling (no raw HttpClient, no IApiClient), loading/error/429 states on both the widget and the history section, request-sequence guard on the history load, @key epoch on the history section so a purge doesn't leave stale data, no hardcoded colours or animations in weather.css, focus rings on the scrollable day strip, and the OdsSelect/OdsDatePicker AriaLabel additions follow the existing OdsCurrencySelect + odsSetAttr pattern. Comments are verdict-level only; I did not run the build or tests (dev server rules).

Non-blocking nits

1. Duplicated picker-option logic. WeatherWidget.razor.cs (PickerOptions, PickerValue, FollowDefault, HouseholdDefault) and Preferences.razor.cs (WeatherLocationOptions, FollowHouseholdDefault, the stored-place-no-longer-selectable fallback) implement the same mapping. Extract to one helper (e.g. in WeatherVisuals) so the "(household default)" label and fallback rule cannot drift between the two surfaces.
2. Silent degrade when options/preferences fail. WeatherWidget.LoadAsync does (await options).Value ?? new() and the same for preferences, so a failed call hides the picker with no signal. Acceptable, but consider skipping the two calls when !_canChoose (saves two requests on every dashboard load for roles that cannot choose) and treating a failure as "no picker" explicitly.
3. Fire-and-forget loads. WeatherWidget.Retry and LocationWeatherHistorySection.Retry use _ = LoadAsync(...); make them async Task handlers (OdsButton OnClick accepts an EventCallback) so exceptions surface to the error boundary instead of being unobserved. The widget also has no re-entrancy guard on Retry (the history section has _requestSeq).
4. Quiet reload error path. After a picker save, LoadAsync(quiet: true) that fails sets Phase.Error, which unmounts the picker the user is focused on (the exact WCAG 2.4.3 case the quiet flag exists to avoid). Keep the previous body and show _prefError instead on a quiet failure.
5. Heading semantics. WeatherWidget.razor uses <span role="heading" aria-level="2">; prefer a real <h2> unless OdsCardHeader has a documented reason for the span.
6. Dead ids. WeatherSettingsDialog.razor declares wx-fc-l, wx-tr-l, wx-hs-l that nothing references (the switches use AriaLabel). Drop them or use aria-labelledby.
7. Settings dialog during save. Cancel/close stay enabled while _saving, so the dialog can be dismissed mid-request and OnSaved fires on a closed dialog. Mirror WeatherPurgeDialog's withheld OnClose while saving, or confirm it is harmless.
8. Ordering nit. In LocationWeatherSection.razor, [Parameter] CanPurge is declared below its use, away from the other parameters.

Distinguished as intentional and not flagged: ligature-string icons on OdsMIcon/OdsButton, static InteractiveCheck/Today seams, DateTime.Now viewer-clock windows.

[centralcmd]

Senior Architect Agent re-review (commit 2052bbd): Approved

All four earlier suggestions are resolved:

1. Per-location isolation: resolved. Both loops in WeatherScheduleJob catch unexpected exceptions (filtered on !cancellationToken.IsCancellationRequested), log with the LocationId, keep the spacing and continue. Each location has its own scope, so no tracked state leaks.
2. Per-process spacing: resolved. RequestSpacing now documents that it is per process and matches the single-worker deployment.
3. DELETE summary: resolved. The Swagger summary now describes both the history-only and the full purge.
4. Data Protection application name: resolved. DataProtectionKeyRing.ApplicationName is the single constant, and both Program.cs and WorkerHost use it.

Additionally, the dev compose now hardens the worker as the production file does. I did not re-run the build or tests. No new issues found.

[centralcmd]

✅ PR was reviewed by Senior Tester Agent and no issues were found (re-review of 2052bbd).

The earlier DateTime.Now nit is resolved: WeatherWidget.LocalNow is now a seam, and the test pins it and resets it on Dispose. I read the new tests and they have meaningful assertions: schedule-run isolation, the shared Data Protection application name, picker retention after a failed reload, no option loading without the claim, focus moving before controls disable, the dialog not closing mid-save, and the select re-applying its accessible name. The new tests were not run individually. The coordinator reports the fast suites pass (Core 1925, Client 3865, Worker 9, ApiClient 328, Api 3057). The Docker-backed Odyssey.IntegrationTests and E2E tiers were not run.

[centralcmd]

✅ PR was reviewed by Security Agent and no issues were found: Approved.

Re-review of commit 2052bbd. My earlier notes were non-blocking observations:

1. Purge confirmation is client-only. It stands as an accepted residual, not a defect. The DELETE stays guarded by the server preconditions, the Admin weather.history.delete claim, the antiforgery cookie flow and the 6-per-5-minutes rate limit. Revisit if a non-Blazor client ever drives the endpoint.
2. Worker can decrypt every stored credential. This stays as the documented, deliberate decision. The ring is mounted :ro and automatic key generation is off.
3. Base compose hardening. Addressed. docker-compose.yml now carries no-new-privileges, cap_drop: ALL, read_only and a /tmp tmpfs, the same as prod.

New in this commit, reviewed and clean:

• The Data Protection application name is one shared constant, DataProtectionKeyRing.ApplicationName, used by the API and the worker. It is pinned to "Odyssey", so existing key rings still decrypt. This removes the risk of the two hosts silently deriving different keys.
• The scheduled job now catches an unexpected per-location exception, logs it and moves on. This is availability-positive and logs no secret or request URL (the HTTP failure paths already carry no URL or body).
• The odsSetAttr JavaScript de-duplication has no security impact.

[centralcmd]

Senior Frontend Agent re-review (PR #355, commit 2052bbd)

All eight prior nits are resolved. Read-only review; I did not build or run the tests.

1. Duplicated picker logic: resolved. WeatherVisuals.PickerOptions / PickerValue / HouseholdDefault / FollowHouseholdDefault are used by both WeatherWidget and Preferences.
2. Options/preferences fetched needlessly: resolved. Both calls are skipped unless _canChoose.
3. Fire-and-forget Retry: resolved. Retry returns Task and is a no-op while Phase.Loading, in the widget and in LocationWeatherHistorySection.
4. Quiet reload failure unmounting the picker: resolved. The body and picker stay, and _prefError says the choice is saved. Minor remaining wrinkle, not blocking: ChooseAsync still sets the "Weather now shows X" announcement afterwards, which slightly contradicts the failure text for screen-reader users.
5. Heading semantics: resolved (<h2 class="wx-ttl">).
6. Unused ids: resolved.
7. Settings dialog dismissable mid-save: resolved. OnClose is withheld and OpenChanged(false) refused while _saving, and Cancel is disabled.
8. Parameter ordering: resolved.

The other changes in the commit also look right: OdsSelect/OdsDatePicker now re-apply AriaLabel when it changes, the purge dialog moves focus to its explanation before disabling controls, and the single odsSetAttr definition removes the duplicate that silently shadowed the first.

✅ PR was reviewed by Senior Frontend Agent and no issues were found: Approved.

[centralcmd]

Accessibility Agent re-review of PR #355 at 2052bbd. All three earlier findings are resolved. While re-reading I found the same focus-loss pattern as finding 1 in a second dialog.

Resolved:

1. Purge dialog focus (2.4.3): WeatherPurgeDialog now focuses the tabindex="-1" explanation paragraph before _running disables the controls. The outline is suppressed only on this programmatic target, which is acceptable.
2. Card title (1.3.1): WeatherWidget now uses a real <h2 class="wx-ttl">, and the CSS resets its margin and font.
3. odsSetAttr / AriaLabel (4.1.2): there is now a single odsSetAttr definition, and null or empty removes the attribute. OdsSelect and OdsDatePicker re-apply AriaLabel whenever it changes.

New finding, found in this re-read:

Settings dialog loses focus while saving (Odyssey.Client/Pages/Weather/WeatherSettingsDialog.razor, footer)

• Description: Clicking Save sets _saving. The Save button has Loading="_saving", and OdsButton renders disabled for Disabled || Loading. 2052bbd also adds Disabled="_saving" to Cancel. So the focused Save button becomes disabled, every footer control is disabled, and Esc, scrim and close are withheld. Focus drops to <body> for the duration of the save, the same defect as the purge dialog.
• Criterion: 2.4.3 Focus Order – Level A.
• Fix: Apply the purge dialog's approach. Give a stable element in the dialog (for example the first explanatory paragraph or an h2) tabindex="-1" and an @ref. In SaveAsync, await _ref.FocusAsync() before setting _saving = true, inside the same try/catch for InvalidOperationException and JSException. Alternatively, drop Disabled="_saving" from Cancel and guard CancelAsync, which already returns early while saving, so Cancel stays focusable.

This one should be fixed before approval.

[centralcmd]

Accessibility Agent re-review of PR #355 at b6dcf35. The settings-dialog focus-loss finding (2.4.3 Focus Order – Level A) is resolved: SaveAsync focuses the tabindex="-1" dialog body, which also carries aria-busy while saving, before Save and Cancel are disabled. The outline is suppressed only on that programmatic target (.wx-form:focus). The picker-save failure announcement now speaks the failure instead of contradicting it. All earlier findings (purge dialog focus, <h2> title, odsSetAttr / AriaLabel) are resolved and I have no open issues.

✅ PR was reviewed by Accessibility Agent and no issues were found: Approved.

[centralcmd]

Senior Frontend Agent update (b6dcf35): confirmed. After a quiet reload failure the card now announces the failure text instead of "Weather now shows X" (_prefError is cleared at the start of each choice, so the success path is unaffected). WeatherSettingsDialog focuses its body (tabindex=-1, aria-busy while saving) before Save/Cancel are disabled, matching WeatherPurgeDialog. No new findings. Approved. (Read-only; build/tests not run.)