Skip to content

fix: forget browser notification choice when removing a Remote Host - #2549

Merged
DoodleBears merged 4 commits into
mainfrom
t3/2500-forget-remote-notifications
Oct 22, 2026
Merged

DoodleBears merged 4 commits into
mainfrom
t3/2500-forget-remote-notifications

Conversation

@DoodleBears

Copy link
Copy Markdown
Contributor

Requested by Vain. Refs #2500 · F01 finding · bounded claim.

Agent-Task: codex/local/01a15ee9-f10e-7e21-90ba-727ffccb2183. Follow-up to merged #2534; its historical evidence is unchanged.

Why the change

Permanently removing a Remote Host should forget its browser notification choice so pairing it again cannot silently restore the old enabled state.

Special things to note

  • Merge risk: one-way preference reset, small blast radius — only the explicitly removed Host's browser choice is deleted; revert stops future cleanup, while a deleted choice must be selected again manually. Review focus: cancellation, per-Host isolation and storage refusal. Temporary disconnect, page disposal and phone-global Device Notification Preference are unchanged. No new setting or Host API; existing browser notification CLI remains, and automatic Remove cleanup is recorded under the existing pairing lifecycle inventory exemption.
  • Verification at production head 05e00a20c50dce2ef0b00c8c00683ccd36717bc6: complete GO2 build, root/mobile typecheck, lint, format and 97 notification/Host/diagnostic tests + 20 viewer-boundary tests all exit 0; independent R2 Standards 0 / Spec 0. Final head d359c225b412c1a511ccf11b2a5d1596536ef0b5 adds only four screenshots. Fresh two-Host UI run exit 0 proves removal, exact pending cancellation, real Host revocation, reload/re-pair defaults, temporary-disconnect retention and redacted storage-refusal reporting. Notification API used its real constructor with automated permission; OS banner, native/phone CLI, release-client parity, DSH Desktop and physical two-machine tailnet are unqualified. F01 changes no native inputs. No full suite or local docs build; ci-plan selects affected tests and docs build, not yet a CI success claim. All four tracked Browser/Computer bundles were generated naturally and remained byte-identical.
  • If notification storage refuses deletion, runtime cleanup and remote revocation still complete, but remote-notification-forget-persistence-failed reports that the persisted choice remains; reload/re-pair cannot be promised clean in that case. Pairing-storage failures are outside this fix. Original F01 red, R1 refusal red and first UI-driver timeout remain recorded; the corrected driver uses Messages → Add remote. Historical persistedChoiceUnchanged acceptance was wrong for permanent Remove and is not reused. Existing O(seen identities) session memory and migration106 recovery risks are unchanged; F02 layout is separate.

Change outline

Permanent Remove
  release viewers
  notification owner.forget(Host)
    cancel pending activation
    try deleting only this Host's persisted choice
    clear in-memory choice, seen history and notification handles
    report fixed redacted code if persistence failed
  stop shared feed → forget local pairing → revoke on Remote Host

Reload + re-pair after successful deletion → off, default kinds
Temporary disconnect / page disposal → retain browser choice
Phone global preference → unchanged

No layout or copy change. The images below are one new-build lifecycle journey, not a base-version screenshot comparison: 1440 × 900, English, same isolated Hosts. UI selection and browser storage are corroborated by Host revokedAt and the old credential's HTTP 401 device-revoked; late activation returns notification-activation-cancelled. The storage-refusal case also reaches the existing Host diagnostics stream. Local evidence and original failures are retained under .humanlayer/tasks/2500-f01/; private profiles and credentials are not published.

Before permanent Remove — enabled, Tool approvals disabled (light / dark).

Before Remove, light: notifications on and approvals off

Before Remove, dark: notifications on and approvals off

After Remove → reload → re-pair — disabled, all four kinds reset (light / dark).

After re-pair, light: notifications off and default kinds

After re-pair, dark: notifications off and default kinds

Issue: #2500

Agent-Task: codex/local/01a15ee9-f10e-7e21-90ba-727ffccb2183
Issue: #2500

Agent-Task: codex/local/01a15ee9-f10e-7e21-90ba-727ffccb2183
Issue: #2500

Agent-Task: codex/local/01a15ee9-f10e-7e21-90ba-727ffccb2183
Issue: #2500

Agent-Task: codex/local/01a15ee9-f10e-7e21-90ba-727ffccb2183
@DoodleBears DoodleBears self-assigned this Oct 22, 2026
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cc960eef-53fb-4349-bfbc-40a2ad44d22e
📥 Commits

Reviewing files that changed from the base of the PR and between 9361934 and d359c22.

⛔ Files ignored due to path filters: 4
  • .github/pr-screenshots/f01/a-d.png is excluded by !**/*.png
  • .github/pr-screenshots/f01/a-l.png is excluded by !**/*.png
  • .github/pr-screenshots/f01/b-d.png is excluded by !**/*.png
  • .github/pr-screenshots/f01/b-l.png is excluded by !**/*.png
📒 Files selected for processing: 11
  • .agents/skills/verify-deepseekbot/features/remote-hosts.md
  • changes/2500-remote-notification-removal.md
  • docs/architecture/botharness-architecture.md
  • packages/client/src/client/remote-hosts.ts
  • packages/client/src/client/remote-notifications.ts
  • packages/client/test/client-observer.test.ts
  • packages/client/test/remote-hosts.test.ts
  • packages/client/test/remote-notifications.test.ts
  • packages/core/src/diagnostics/client-diagnostics.ts
  • packages/core/src/diagnostics/client-observer.ts
  • scripts/cli-ui-inventory.json
+---------------------------------------------------------------------------------------------------------------+
| Make quality a requirements issue. Involve your users in determining the project's real quality requirements. |
+---------------------------------------------------------------------------------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@DoodleBears

Copy link
Copy Markdown
Contributor Author

Exact-head review aid for d359c225b412c1a511ccf11b2a5d1596536ef0b5 (base 93619345855402ee4a89e8777d7f077b7adfe553): permanent Remove calls the existing notification owner to cancel activation and forget that Host's choice, then continues normal feed/pairing teardown and remote revocation. Notification-storage refusal records remote-notification-forget-persistence-failed while cleanup continues; persisted deletion is explicitly unproven in that refusal case. Temporary disconnect and the phone-global preference remain unchanged.

The detailed Chinese HTML explainer was generated with the repository's answer-me-with-html CLI and checked at 1100px and 360px. It is retained in the task-local handoff; no authorized public HTML hosting path was used, so the local file/T3 attachment is not claimed as GitHub-reviewer-accessible HTML. This comment and the PR body provide the reviewable explanation here. The four PR images are public, and all four were checked rendered in GitHub at 1440×900.

Production evidence binds 05e00a20c50dce2ef0b00c8c00683ccd36717bc6; the final commit adds only screenshots. Build/typecheck/lint/format and 117 focused tests passed. Fresh two-Host UI/canonical/storage verification passed on its second drive; the first driver-entry timeout and original red tests are retained. Native, phone owner CLI, OS banner, release parity and physical two-machine tailnet remain unqualified. CI and merging stay with the parent coordinator.

@DoodleBears
DoodleBears marked this pull request as ready for review October 22, 2026 01:03
@DoodleBears
DoodleBears merged commit 87fc414 into main Oct 22, 2026
12 checks passed
@DoodleBears
DoodleBears deleted the t3/2500-forget-remote-notifications branch October 28, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant