Limit optional reminders to selected working hours - #2892
Conversation
# Conflicts: # docs/IMPLEMENTATION_MASTERPLAN.md # docs/STATUS.md # docs/product/WORKSPACE_OVERHAUL_VALIDATION.md
|
Independent bounded Luna review of the reminder-hours runtime at 65a8c44 was CLEAN. A final locally reproduced HIGH account-erasure race was repaired in 8b2ab36: the revision-guarded database update now also requires an active account. The new real API regression failed before the repair (HTTP 200 instead of 409) and passes afterward within 56 targeted API tests. Independent narrow fix review is CLEAN: the condition is atomic with the write and preserves ordinary active-account revision behavior. No remaining HIGH/CRITICAL finding was identified in these reviews. Final combined proof also includes 56 components, typecheck/build, five Chromium journeys, prior full-slice backend/frontend qualification and two Firefox journeys. No second full suite is claimed after the scoped privacy repair. Hosted exact-head CI remains pending. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01155f2425
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| watch(() => attention.settings, settings => { | ||
| const window = settings?.window | ||
| restricted.value = !!window | ||
| zone.value = window?.timeZoneId ?? 'UTC' | ||
| days.value = window ? dayOptions.filter(day => (window.daysMask & (1 << day.value)) !== 0).map(day => day.value) : [1, 2, 3, 4, 5] | ||
| start.value = formatTime(window?.startMinute ?? 540); end.value = formatTime(window?.endMinute ?? 1020) | ||
| }, { immediate: true }) |
There was a problem hiding this comment.
If a user edits the hours restriction, zone, days, or times and then toggles “Enable occasional reminders” before submitting this form, the toggle's save receipt replaces attention.settings with the previously persisted window. This watcher then silently overwrites every unsaved hours field. Rehydrate only on initial load/reload, or track whether the hours form is dirty so the independent enable/disable save cannot discard the draft.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Classified as MEDIUM: this can discard an unsaved hours-form draft after the independent enable toggle succeeds; saved server state and the privacy/budget guards remain intact. Tracked with a concrete regression/acceptance scope in #2893, per the bounded review policy. This does not reopen the completed privacy-fix review or block this delivery. |
|
Final base revalidation is CLEAN at 42965ed. The merged #2886 receipt f00cf1d has the identical tree to reviewed/tested fce97dc; bringing it into this branch changed no runtime files. The subsequent commit changes only STATUS, MASTERPLAN and the validation ledger to record the actual merge. Independent Luna revalidation found no false operational/safety claim and confirmed the earlier runtime/fix review still applies. Documentation links, governance and diff checks pass. Required exact-head run34453924959 is the current hosted gate; the cancelled pre-refresh run is not used. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42965edde1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| void attention.save(attention.settings.enabled, restricted.value ? { timeZoneId: zone.value.trim(), | ||
| daysMask: days.value.reduce((mask, day) => mask | (1 << day), 0), startMinute: minutes(start.value), endMinute: minutes(end.value) } : null) |
There was a problem hiding this comment.
When the user enters a nonempty but invalid or server-unsupported time-zone name, this path submits it because valid only checks that the field is nonempty; the API then returns the explicit 400 from WorkspaceAttentionService.SaveAsync, but the store treats that confirmed rejection like an uncertain save, clears settings, and removes the entire form. The user loses all entered hours and sees only a misleading reload instruction instead of the validation message, so retain the draft and surface the 400 response for this save path.
Useful? React with 👍 / 👎.
|
Comment3977017997 is MEDIUM: a definite validation rejection currently discards an unsaved settings draft and shows uncertain-save copy. No server mutation, privacy, budget or permission bypass is involved. It belongs to the existing draft-recovery follow-up #2893, which is being implemented alongside #2894; confirmed validation rejection will preserve the draft, while genuinely uncertain failures will still require reload. This does not block the original hours delivery. |
1 participant
Optional reminders previously offered no way to limit new offers to working hours. Settings now support explicit weekdays and an IANA time zone, including overnight windows and daylight-saving changes. The server applies the window before question lookup and budget admission; existing two-per-UTC-day and two-hour limits remain unchanged. Old-client toggles preserve the saved window, exports include it, and an active-account database condition prevents an in-flight save from restoring private hours after account erasure.
Related to #2808. #2886 is merged; this is the final reminder-hours continuation against main, with no automatic issue closure. Available across Classic, Studio, Companion and Unified with shared theme tokens. New reminders remain opt-in and reuse existing questions without automatic model calls.
Validation:
No migration. Temporary services are stopped and all databases are synthetic. Hosted exact-head CI remains required. Physical microphones/devices, live-provider quality, subjective reminder usefulness and existing OUTSTANDING_TASKS.md owner decisions are not inferred from this proof. Full evidence: docs/product/WORKSPACE_OVERHAUL_VALIDATION.md.