Skip to content

feat: add channel mention notifications - #116

Open
jjjhenriksen wants to merge 3 commits into
openclaw:mainfrom
jjjhenriksen:codex/mentions-notifications
Open

feat: add channel mention notifications#116
jjjhenriksen wants to merge 3 commits into
openclaw:mainfrom
jjjhenriksen:codex/mentions-notifications

Conversation

@jjjhenriksen

@jjjhenriksen jjjhenriksen commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add per-channel all, mentions, and muted notification preferences;
  • attach resolved mention user IDs and author/body metadata to durable message and thread-reply events;
  • enforce preferences for Pushover and browser/desktop realtime alerts;
  • keep mention parsing out of Markdown links and all supported URL-like tokens;
  • limit composer suggestions to users with resolvable handles;
  • rebase onto current main and refresh the embedded web client.

Real behavior proof

The focused Playwright scenario runs against the real ClickClack Go server and embedded web app. It:

  1. gives the signed-in user a mentionable profile handle;
  2. creates an active channel and a background channel;
  3. sets the background channel to muted, posts as a second real user, and observes zero browser notifications;
  4. changes the setting through ClickClack to mentions, reloads, and verifies the persisted UI state;
  5. posts an ordinary background message and observes zero browser notifications;
  6. posts a mention and verifies the durable event contains the current user in mentioned_user_ids and exactly one browser notification appears;
  7. changes the channel remotely from mentions to muted and then all, proving the already-running client suppresses and resumes alerts without a reload or channel selection;
  8. posts a thread reply while all is active and verifies a third notification containing the real reply body, proving thread.reply_created uses the durable author/body payload and the same channel preference path;
  9. clicks the original notification and verifies navigation to the originating channel.

Command and result:

pnpm exec playwright test tests/e2e/chat.spec.ts --grep "channel preferences filter browser notifications outside the active conversation" --reporter=line
1 passed

The captured browser proof shows the live channel header in the muted state:

Channel notification preference proof

The screenshot above is attached directly to this PR description; the proof image is not part of the feature branch.

Validation

pnpm check
passed

bash scripts/check-go-coverage.sh
passed; aggregate filtered coverage 85.1%

pnpm generate:sqlc
passed

go test ./apps/api/internal/store/... ./apps/api/internal/httpapi/...
passed

The seven current review findings are covered by parser tests, SQLC query guards, handle-filtered composer sources, in-memory preference fallback, post-fetch notification rechecks, selected-channel preference convergence, and durable thread-reply payload coverage.

Current branch head 4608f047 is rebased onto upstream main at 4843afe2.

@jjjhenriksen

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@jjjhenriksen
jjjhenriksen marked this pull request as ready for review July 26, 2026 03:38
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 26, 2026
@clawsweeper

clawsweeper Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 1, 2026, 7:47 PM ET / 23:47 UTC.

ClawSweeper review

What this changes

Adds per-channel all, mentions, and muted notification preferences, persists resolved mention recipients on message events, and applies them to Pushover plus browser/desktop alerts.

Merge readiness

⚠️ Ready for maintainer review - 5 items remain

Keep open for maintainer review: the patch is a coherent, proof-backed feature, but it creates durable SQLite/PostgreSQL schema, REST/OpenAPI/SDK, and realtime-event contracts that current main does not provide and that need explicit product sponsorship before merge.

Priority: P2
Reviewed head: 4608f047910ba22655a0a4bb926c9abb49875d48
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A well-tested cross-layer implementation with credible real behavior proof, held for maintainer acceptance of its durable product and API contracts rather than a demonstrated patch defect.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body provides a focused real ClickClack server and embedded-web Playwright run with a reported passing result, covering persisted preferences, live remote changes, mention filtering, a thread reply, and notification navigation.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body provides a focused real ClickClack server and embedded-web Playwright run with a reported passing result, covering persisted preferences, live remote changes, mention filtering, a thread reply, and notification navigation.
Evidence reviewed 5 items Current main does not contain the feature: The PR is based on current main, but its head is not reachable from main and introduces the notification-settings HTTP handler, persistent preference tables, durable mention metadata, web controls, and API/SDK contracts absent from the base.
Upgrade-safe default is implemented: Both stores return all when no channel preference row exists, preserving existing alert behavior for upgraded users; the branch also includes SQLite and PostgreSQL migration tests for pre-existing databases.
Delivery logic uses creation-time mention metadata: The browser/desktop path reads mentioned_user_ids for mentions mode, while the server passes the event's resolved IDs into Pushover recipient filtering instead of reparsing later message content.
Findings None None.
Security None None.

How this fits together

ClickClack creates durable channel and thread message events in its Go API, then delivers them through realtime clients and optional Pushover notifications. This PR adds per-user channel preferences between event creation and those alert outputs, with the web client exposing and enforcing the selected mode.

flowchart LR
  A[Channel or thread message] --> B[Mention resolution]
  B --> C[Durable event metadata]
  C --> D[Channel preference lookup]
  D --> E[Pushover recipients]
  D --> F[Browser and desktop alerts]
  G[Channel header control] --> D
Loading

Decision needed

Question Recommendation
Should ClickClack permanently support per-channel notification preferences and the accompanying public REST, SDK, and realtime-event contracts? Sponsor the durable notification feature: Accept the per-channel preference, migration, API, SDK, and realtime contracts and proceed with normal merge review.

Why: The patch is not repairing an established broken contract; it adds durable user settings and externally consumable API/event surface whose long-term support boundary is a maintainer product decision.

Before merge

  • Resolve merge risk (P1) - Merging establishes a long-lived per-channel preference setting plus SQLite/PostgreSQL migrations that operators must carry through upgrades and exports.
  • Resolve merge risk (P1) - The new REST, OpenAPI, TypeScript SDK, and realtime mentioned_user_ids fields become compatibility contracts for API consumers.
  • Resolve merge risk (P1) - Alert delivery changes span Pushover and already-running browser/desktop clients; maintainers should accept the notification policy before making this cross-surface behavior permanent.
  • Complete next step (P2) - A maintainer must sponsor or narrow the new durable notification and public API contract before this feature can safely land.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Cross-layer scope 64 files affected; 2,419 added and 218 removed The feature crosses storage, HTTP, generated SDK, web UI, embedded assets, docs, and E2E coverage rather than being a local alert tweak.
Persistent upgrades 2 database migrations SQLite and PostgreSQL installations receive new stored event metadata and channel preference tables.
Public contract surface 1 REST path plus OpenAPI and TypeScript SDK updates Consumers gain stable notification-setting operations and durable event metadata.

Merge-risk options

Maintainer options:

  1. Accept and document the public contract (recommended)
    Approve the durable settings, OpenAPI/SDK additions, and event metadata as supported compatibility surface before merging.
  2. Narrow the feature before merge
    Remove or redesign the externally visible settings and event contract if maintainers want notification policy to remain internal for now.
  3. Pause for product direction
    Keep the PR open but do not merge until a maintainer confirms the permanent notification and API boundary.

Technical review

Best possible solution:

If maintainers want per-channel notification controls as a supported product feature, land the branch with the existing migration and end-to-end coverage while treating the REST, SDK, and realtime fields as stable contracts.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction: this PR adds a new feature. The PR body supplies a focused real-server Playwright run that reports one passing scenario across muted, mentions, all, remote preference changes, thread replies, and notification navigation.

Is this the best way to solve the issue?

Unclear pending product direction: the implementation is internally coherent and preserves all as the compatibility default, but public per-channel settings and durable event metadata are a product/API commitment rather than the only way to provide notifications.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4843afe2e8eb.

Labels

Label justifications:

  • P2: This is a substantial but non-emergency notification and API feature requiring normal maintainer product review.
  • merge-risk: 🚨 compatibility: The PR adds persisted settings, database migrations, REST/OpenAPI/SDK operations, and durable realtime fields that external users may depend on.
  • merge-risk: 🚨 message-delivery: The new per-channel policy changes which Pushover and browser/desktop alerts are delivered for background messages and thread replies.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides a focused real ClickClack server and embedded-web Playwright run with a reported passing result, covering persisted preferences, live remote changes, mention filtering, a thread reply, and notification navigation.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a focused real ClickClack server and embedded-web Playwright run with a reported passing result, covering persisted preferences, live remote changes, mention filtering, a thread reply, and notification navigation.

Evidence

What I checked:

  • Current main does not contain the feature: The PR is based on current main, but its head is not reachable from main and introduces the notification-settings HTTP handler, persistent preference tables, durable mention metadata, web controls, and API/SDK contracts absent from the base. (apps/api/internal/httpapi/channel_notifications.go:1, 4843afe2e8eb)
  • Upgrade-safe default is implemented: Both stores return all when no channel preference row exists, preserving existing alert behavior for upgraded users; the branch also includes SQLite and PostgreSQL migration tests for pre-existing databases. (apps/api/internal/store/sqlite/notifications.go:85, 4608f047910b)
  • Delivery logic uses creation-time mention metadata: The browser/desktop path reads mentioned_user_ids for mentions mode, while the server passes the event's resolved IDs into Pushover recipient filtering instead of reparsing later message content. (apps/web/src/ChatApp.svelte:1722, 4608f047910b)
  • Repository SQLC policy was followed: The changed SQLite and PostgreSQL query/schema sources have corresponding checked-in generated storedb updates, consistent with the repository policy requiring SQLC generation rather than hand-maintained generated code. (AGENTS.md:3, 4608f047910b)
  • Feature-history routing: The current notification foundation dates to Shakker's appearance-preference work, while Peter Steinberger recently maintained adjacent channel and realtime behavior on main. (apps/api/internal/httpapi/notifications.go:22, 4e144f880bf6)

Likely related people:

  • Shakker: Introduced the existing notification and synchronized preference behavior that this feature extends across the server and web client. (role: introduced notification-preference foundation; confidence: high; commits: 4e144f880bf6, f8215a37d22d; files: apps/api/internal/httpapi/notifications.go, apps/web/src/ChatApp.svelte)
  • Peter Steinberger: Recent main-branch work covers channel presentation and realtime recovery, the adjacent surfaces most affected by the new durable event and notification behavior. (role: recent channel and realtime contributor; confidence: high; commits: e455d43809a0, bfc8fc6230e4; files: apps/web/src/ChatApp.svelte, apps/api/internal/httpapi/notifications.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Confirm whether the per-channel preference, REST/SDK operations, and mentioned_user_ids event field are supported long-term contracts.
  • Resolve the current Playwright E2E check failure before merging, even though the PR body includes a focused passing real-server scenario.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (25 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-31T20:44:39.414Z sha 4608f04 :: needs changes before merge. :: [P2] Normalize the SQLite mention predicate
  • reviewed 2026-07-31T22:17:07.778Z sha 4608f04 :: needs changes before merge. :: [P2] Normalize the SQLite mention predicate
  • reviewed 2026-07-31T23:35:58.411Z sha 4608f04 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T04:06:13.700Z sha 4608f04 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T09:34:18.376Z sha 4608f04 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T11:27:28.152Z sha 4608f04 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T14:52:59.823Z sha 4608f04 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T17:33:25.658Z sha 4608f04 :: needs maintainer review before merge. :: none

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread apps/web/src/ChatApp.svelte Outdated
Comment thread apps/api/internal/store/types.go
Comment thread apps/api/internal/store/postgres/notifications.go Outdated
Comment thread apps/web/src/ChatApp.svelte
Comment thread apps/api/internal/store/postgres/notifications.go Outdated
Comment thread apps/web/src/ChatApp.svelte
@jjjhenriksen
jjjhenriksen force-pushed the codex/mentions-notifications branch from 423a41f to eb9abbc Compare July 26, 2026 05:38
@jjjhenriksen

Copy link
Copy Markdown
Contributor Author

@codex review

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 26, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread apps/web/src/ChatApp.svelte
Comment thread apps/web/src/ChatApp.svelte
Comment thread apps/api/internal/store/sqlite/sqlc/queries.sql Outdated
@jjjhenriksen
jjjhenriksen force-pushed the codex/mentions-notifications branch from eb9abbc to 62b88b8 Compare July 26, 2026 05:51
@jjjhenriksen

Copy link
Copy Markdown
Contributor Author

@codex review

@jjjhenriksen
jjjhenriksen force-pushed the codex/mentions-notifications branch from 62b88b8 to 668fe88 Compare July 26, 2026 05:56
@jjjhenriksen

Copy link
Copy Markdown
Contributor Author

@codex review

@jjjhenriksen
jjjhenriksen force-pushed the codex/mentions-notifications branch from 668fe88 to 3413a00 Compare July 26, 2026 06:06
@jjjhenriksen

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread apps/api/internal/store/sqlite/sqlc/queries.sql
Comment thread apps/api/internal/store/types.go
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 29, 2026
@jjjhenriksen
jjjhenriksen force-pushed the codex/mentions-notifications branch from 3413a00 to 85a9a94 Compare July 29, 2026 20:11
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 29, 2026
@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant