feat: add channel mention notifications - #116
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Codex review: found issues before merge. Reviewed August 2, 2026, 5:34 PM ET / 21:34 UTC. ClawSweeper reviewWhat this changesAdds per-channel all, mentions-only, and muted notification preferences across storage, HTTP and SDK APIs, Pushover, browser/desktop alerts, and the web UI. Merge readinessKeep open: current main still has only global message notifications, while this PR adds a distinct per-channel notification capability. Exact-head real-server proof is sufficient, but the branch still needs its release-owned changelog edit removed, its stale PR description corrected, and a maintainer decision on the new persistent preference contract. Priority: P2 Review scores
Verification
How this fits togetherClickClack turns created messages and thread replies into durable realtime events, then sends push or browser/desktop alerts to eligible users. This PR adds a per-user channel preference decision between event delivery and those alert surfaces. flowchart LR
A[Channel message or thread reply] --> B[Store durable event metadata]
B --> C[Realtime delivery]
B --> D[Push recipient selection]
E[Per-channel user preference] --> D
E --> F[Browser and desktop alert filter]
C --> F
D --> G[Pushover alert]
F --> H[Browser or desktop alert]
Decision needed
Why: This is a new persisted user setting and delivery contract rather than a repair to an established behavior; implementation quality alone cannot determine whether that permanent product surface is desired. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land a narrowly rebased feature branch that keeps reply events metadata-only, preserves Do we have a high-confidence way to reproduce the issue? Not applicable as a bug report. The requested capability is new, and the PR supplies a high-confidence exact-head real-server Playwright run that exercises its alert behavior. Is this the best way to solve the issue? Unclear. The implementation is comprehensive and has exact-head behavior proof, but only a maintainer can decide whether this new persisted per-channel notification contract belongs in core. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 187131ccbbda. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (31 earlier review cycles; latest 8 shown)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 423a41fac0
ℹ️ 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".
423a41f to
eb9abbc
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb9abbc3c9
ℹ️ 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".
eb9abbc to
62b88b8
Compare
|
@codex review |
62b88b8 to
668fe88
Compare
|
@codex review |
668fe88 to
3413a00
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3413a00928
ℹ️ 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".
3413a00 to
85a9a94
Compare
…aw#113) Adds direct characterization coverage for appearance preference normalization, validation, nil preservation, and empty-patch detection. Co-authored-by: Krasimir Kralev <krasi@idrobots.com>
Refreshes application, build, CI, container, and desktop dependencies; aligns pnpm 11.19.0 across every build path; and regenerates the embedded production bundle on current main.
Expose the existing topic model in the web composer and timeline with resilient filtering, draft recovery, and paginated API support. Co-authored-by: PollyBot13 <pollybot13@gmail.com>
Hydrate authorized thread messages for alert text after applying the canonical channel preference, and document the unified conversation-attention model.\n\nCo-authored-by: Jacqueline Henriksen <jjjhenriksen@gmail.com>
|
Exact-head proof for
That scenario exercised Exact-head GitHub verification is green:
|
|
@clawsweeper re-review Exact-head after-fix runtime proof is now attached above for |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
all,mentions, ormuted;Real behavior proof
The focused Playwright scenario runs against the real ClickClack Go server and embedded production web app. It:
muted, posts as a second real user, and observes zero browser notifications;mentions, reloads, and verifies the persisted UI state;mentioned_user_idsand exactly one browser notification appears;mentionstomutedand thenall, proving the already-running client suppresses and resumes alerts without a reload or channel selection;allis active, verifies the durable event omitsauthor_idandbody, and observes the real reply text after authorized message hydration;Exact-head command and result for
d44b42e9e60cef731aff3073d35eb823265ae592:The captured browser proof shows the live channel header in the muted state:
The screenshot above is attached directly to this PR description; the proof image is not part of the feature branch.
Validation
Exact-head GitHub verification:
The final design uses the channel preference as the single notification policy for alert delivery and later mention attention; local storage is only a last-known fallback when the server is temporarily unavailable. Durable events carry routing and resolved mention metadata, never thread-reply content.