Skip to content

fix: alert again when a watch condition clears and reappears - #83

Merged
jerelvelarde merged 2 commits into
CopilotKit:mainfrom
wangtaotaotao95:fix/recurring-watch-notifications
Oct 5, 2026
Merged

jerelvelarde merged 2 commits into
CopilotKit:mainfrom
wangtaotaotao95:fix/recurring-watch-notifications

Conversation

@wangtaotaotao95

Copy link
Copy Markdown
Contributor

A contains or price_below watch can silently miss every later occurrence of the same condition. For example, Sold out → Available now → Sold out → Available now produces only one alert: the second match reuses the first notification's page-hash key even though the condition cleared in between.

Persist an alert sequence with the task outcome and use it in condition-watch notification keys. Each false-to-true transition gets a new identity, while republishing the same saved outcome remains idempotent. Existing tasks start the sequence at zero. The existing content-based deduplication policy for change watches is preserved.

Verification on Node 24.21.0 / pnpm 11.19.0:

  • New contains and price_below regressions fail on main and pass here. They cover repeated matches, maintenance replay of saved notices, and unchanged pages remaining quiet.
  • Full suite: 208/208 pass, including lost-outcome recovery and the existing repeated-page-change deduplication test.
  • Lint, root/mobile typecheck, worker typecheck, server build, and web/iOS/Android exports pass.

Tests use sample pages and local storage; no live browser/provider calls or native device testing were performed. This changes notification identity, not the page comparison or condition-matching rules.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful recurring-condition alert fix: each false-to-true occurrence gets a new persisted notification identity, while replay and unchanged pages remain quiet and change-watch deduplication stays intact. All 17 focused monitor tests pass locally. Description documents checks and provider/device limits. No actionable correctness or security findings. The old-head browser/container jobs need a current-main update and passing rerun before merge.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the current-main update: the PR's fix and its regression tests remain unchanged in scope, and recently merged behavior/tests are retained. No new actionable findings. Approval applies to this updated head; merge after all seven required CI checks pass.

@jerelvelarde
jerelvelarde merged commit 96c2525 into CopilotKit:main Oct 5, 2026
7 checks passed
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.

2 participants