Skip to content

Push plugin and AI result changes to clients instead of having them poll - #305

Merged
C-Hipple merged 5 commits into
mainfrom
ccr-36327e3a-djegbq
Oct 26, 2026
Merged

C-Hipple merged 5 commits into
mainfrom
ccr-36327e3a-djegbq

Conversation

@C-Hipple

Copy link
Copy Markdown
Owner

Summary

PR #303 gave the server a way to notify a client (Subscribe + Notifier.Notify), but the only thing it sent was a heartbeat. This PR has the server announce plugin and AI feature results as they change. All three clients (the web client, the Chrome extension and Emacs) now follow those announcements and no longer poll for either.

The server sends two new notifications. Each one names the PR, the plugin or feature, and its new status. Neither carries the output, which can be large. A client showing that PR reads GetPluginOutput or GetAIOutput again when it gets one.

  • Server.PluginOutputChanged is sent every time a plugin result is written (pending, success, error, deferred) or cleared by RerunPlugins.
  • Server.AIOutputChanged is sent when an AI run is claimed (pending) and again when its result is stored. This covers every run: ones a client asked for, the applied features, and the automatic ones.

A notification sent while a client is disconnected never arrives. So each client also reads everything again when it (re)connects or a new server subscribes.

Changes

  • Server
    • All plugin result writes now go through storePluginResult, which sends the notification after the upsert. RerunPlugins announces each result it clears before it replies.
    • The ai package can't import server, so ai.Runner gets an OnChange hook. RunServer points it at the notifier.
    • store now returns the status it actually stored.
  • Web client
    • A new shared useServerChanges hook replaces the poll loops in usePluginOutputs, useAIReport and useAIBackgroundRuns. It re-reads when a change names the PR, and on SSE reconnect via a new ServerEvents.onOpen. Reads run one at a time, and a burst of changes becomes one extra read.
    • useAIReport only applies a reply newer than the one it's showing, so a stale RunAIFeature reply can't overwrite the landed result.
    • The "still running, check back later" notice and the poll timing constants are removed.
  • Chrome extension
    • The native protocol now recognises notifications, whole or chunked. NativeClient subscribes each new server before sending it anything else, asking for the slowest heartbeat.
    • The service worker relays the two events to extension pages over a crs-events port and refuses that port to anything else, the same rule as for RPC. When a new server subscribes, it tells pages to resync.
    • PRTools re-reads on these events. The 3 s poll, the 10 s "just requested" grace period and poll_utils are gone.
    • The content script no longer re-asks for the file order every 5 s. The service worker tells a PR's tabs when a file-ordering result lands.
  • Emacs
    • crs-start-server subscribes, and the process filter dispatches notifications to crs-server-notification-functions.
    • Open plugin, AI report and change-diagram buffers for the PR refresh themselves and keep point.
    • A plugin run started from Emacs is announced in the echo area when it lands. The docstrings no longer tell you to refresh by hand.
    • The AI poll timer is removed, along with crs-ai-poll-interval and crs-ai-poll-timeout.
    • Fixed along the way: the plugin refresh used to render into whichever buffer was current when the reply arrived.
  • Docs: protocol.md documents both notifications. The client docs, the extension README and CLAUDE.md are updated.

Note: the clients expect a server that sends these notifications. Against an older crs, plugin and AI views show what they read when opened, and you refresh them by hand.

Test Plan

  • go build ./... passes
  • go test ./... passes. New tests check that every plugin write and AI claim/landing is announced, that each announcement matches what a read would see at that moment, and that a skipped or up-to-date run announces nothing.
  • bun_client: lint, format, type-check and unit tests pass, and the e2e suite passes (86 passed, plus the one known test.fail() bug). The e2e fake now runs reruns and AI runs on timers and announces each step, instead of finishing them after a set number of reads. New specs check that nothing is read while a held run is pending, and that runs the page didn't start still show up.
  • chrome_extension: lint, format, type-check, unit tests and build pass. New unit tests cover notification classification, subscribing per server, the relay, port refusal, resync and the file-order nudge. All the e2e specs, new and existing, passed here. New specs cover a server-started run, no reads while one is pending, and a newly landed file order applied to an open Files changed tab. One caveat: this container's Chromium doesn't match the pinned Playwright version, so each full run had about one random spec fail in the service-worker startup fixture (chrome is not defined). The base commit fails the same way here, so it's environmental, but CI's run is the real check.
  • Emacs: 68/68 ERT tests pass. Byte-compiling adds no new warnings in the changed files.

Generated by Claude Code

… them

The server now sends two notifications to a subscribed client, through the
Notifier PR 303 added:

- Server.PluginOutputChanged, each time a plugin's stored result for a PR is
  written (pending, success, error, deferred) or cleared for a rerun. Every
  write goes through storePluginResult, which notifies after the upsert, and
  RerunPlugins announces each result it clears before it replies.
- Server.AIOutputChanged, as an AI run is claimed (pending) and as its result
  is stored. The ai package can't import server, so the Runner gets an
  OnChange hook, which RunServer points at the notifier.

Both name the PR and the plugin or feature with its new status, never the
output, so a client showing it reads GetPluginOutput / GetAIOutput again.

The web client stops polling for either. A shared useServerChanges hook
re-reads when an announcement names the PR being shown, and when the
/api/events stream reconnects (ServerEvents.onOpen), since anything sent
while it was down is lost. Reads go one at a time, a burst asking for one
more. usePluginOutputs, useAIReport and useAIBackgroundRuns use it in place
of their poll loops; useAIReport applies only a reply newer than the one it
shows, so a run reply written before the run landed can't overwrite it. The
"still running, check back later" notice goes, along with the poll timings.

The e2e fake now runs plugin reruns and AI runs on timers and announces
each step, as the server does, rather than landing them after so many
reads. New specs check that nothing is read while a held run is pending,
and that runs the page never asked for show up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011prZsf8nHYe7e2Ke1rrq6z
The service worker now subscribes each new server (RPCHandler.Subscribe,
with the slowest heartbeat) before anything else it asks it. The native
protocol recognises notifications, whole or chunked, and the service
worker relays Server.PluginOutputChanged and Server.AIOutputChanged to the
extension pages listening on a `crs-events` port, refusing a port from
anything but the extension's own pages, as for RPC. A new server, or a
page's port dropping and reopening, tells the page to resync.

PRTools no longer polls: it reads plugin or AI output again when an event
names its PR, one read of each kind at a time (coalesce), and everything on
a resync. The ten-second "just requested" grace and poll_utils go; only the
latest read of a kind is applied, a run's reply isn't applied over a read
that started after it, and a rerun missing from the output reads as running
until a read shows it landed.

The content script no longer re-asks for the file order every few seconds:
the service worker tells a PR's tabs when a file-ordering result lands
(crs-file-order-changed) and they ask once.

The e2e fake answers Subscribe, announces each run as it starts and lands,
and lands runs from the scenario by itself rather than on a read; a new
serverRuns scenario has it start runs on its own. New specs cover a run the
server started, nothing read while one is pending, and a newly landed file
order laid over an open Files changed view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011prZsf8nHYe7e2Ke1rrq6z
crs-start-server now subscribes to the server's notifications (with the
slowest heartbeat, which the client doesn't use), and the process filter
hands a notification — a line with a `method` and no id — to
crs-server-notification-functions instead of the reply table.

An open plugin output buffer reads its output again when the server
announces a change to its PR's plugins, one read at a time with a burst
coalesced into one more, keeping point; the refresh now targets its own
buffer rather than whichever one was current when the reply came back. A
run asked for with crs-rerun-plugin or crs-run-on-demand-plugin is
announced in the echo area when it lands, instead of the docstrings
telling the user to poll by hand.

AI report buffers and the change diagram no longer poll on a timer: they
fetch again when Server.AIOutputChanged names their feature and PR. The
poll timer, its kill-buffer cleanup and crs-ai-poll-interval /
crs-ai-poll-timeout go. A new server's subscription has every open plugin
and AI buffer read its output again, since a run the old one had in
flight would never be announced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011prZsf8nHYe7e2Ke1rrq6z
…ge events in CLAUDE.md

An announced change is fetched one at a time per plugin buffer, but a
reply that never comes (the server died mid-request) would have left the
buffer marked as fetching for good, swallowing even a refresh by hand.
A refresh asked for by hand now always fetches, and a new server's
subscription clears the mark before catching the buffers up.

CLAUDE.md describes the two change events, the AI runner's OnChange hook,
and how each client follows them instead of polling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011prZsf8nHYe7e2Ke1rrq6z
CI's Emacs curves the quotes `message' writes (text-quoting-style), so the
test looking for a straight 'lint' failed there while passing locally.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011prZsf8nHYe7e2Ke1rrq6z
@C-Hipple
C-Hipple force-pushed the ccr-36327e3a-djegbq branch from cdbceec to c93266c Compare October 26, 2026 17:55
@C-Hipple
C-Hipple merged commit af09a6b into main Oct 26, 2026
8 checks passed
C-Hipple pushed a commit that referenced this pull request Oct 26, 2026
Conflicts were additive on both sides: server/notify.go keeps main's
Server.PluginOutputChanged / Server.AIOutputChanged beside this branch's
Server.PRUpdated (NotifyPRChanged now copies notifier the way main's helpers
do); server_events.test.tsx imports both sets of helpers; protocol.md,
building_clients.md, web_client_features.md and CLAUDE.md describe all three
notifications.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMPYZg3x4vXfFzqgvJ1AbU
C-Hipple pushed a commit that referenced this pull request Oct 26, 2026
- Read again, quietly, when the event stream comes back after dropping
  (onReconnect, used by usePRUpdates), since a Server.PRUpdated sent while it
  was down is lost, as #305 does for plugin and AI changes. Unlike
  useServerChanges it skips the first connect, which only repeats the read
  the page made on load.
- Stop reloading the plugin output after a push: the plugins the push
  re-runs now announce themselves, and the reload put the panel through a
  loading state.
- Use #305's isChangeFor in place of the identical isUpdateFor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMPYZg3x4vXfFzqgvJ1AbU
C-Hipple added a commit that referenced this pull request Oct 26, 2026
* Refresh the web client live when a webhook delivery changes a PR

With [Webhooks] configured the server already patches its caches from each
delivery, but an open page only found out on a manual sync or a reload. Now
the server pushes a Server.PRUpdated notification (over the channel #303 added)
for every PR a delivery changed, and the web client re-reads the PR in place
and says what happened.

- workflows: each apply function reports what it rewrote for each PR
  (cache fields, "head" after a refetch, "pr" for a pull_request payload)
  and a few-word summary ("alice approved", "bob commented on x.go",
  "alice pushed new commits"). CI is announced only as it settles (passing,
  or failing where it wasn't); a check starting refreshes quietly.
  applyWebhookEvent hands each changed PR to a hook registered with
  SetPRChangedHook, and the membership re-check sends a "sections" change
  when it rewrote rows. Deliveries that changed nothing send nothing.
- server: NotifyPRChanged sends Server.PRUpdated (owner, repo, number, event,
  changes, head_sha, actor, own, summary, time) to a subscribed client;
  main.go wires it in server mode. "own" marks the configured user's changes.
- web client: onPRUpdates/usePRUpdates batch a burst (400 ms from the first
  update). The review re-calls GetPR without a loading state and takes in
  comments, reviews, metadata and a changed diff, keeping open threads,
  expanded hunks, drafts and the feedback being typed; a new diff waits
  while an inline comment is being written. The list re-reads its rows in
  place and highlights the changed ones. A toast says what happened; the
  user's own changes and quiet ones refresh silently.
- e2e: E2E.UpdatePR / backend.updatePR play a delivery; live_updates.e2e.ts
  covers the review, the held diff, the user's own change, sections-only
  updates and the list.
- docs: protocol.md (Server.PRUpdated), webhooks.md, web_client_features.md
  §3.18, building_clients.md, CLAUDE.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMPYZg3x4vXfFzqgvJ1AbU

* Follow #305's change events in the live refresh

- Read again, quietly, when the event stream comes back after dropping
  (onReconnect, used by usePRUpdates), since a Server.PRUpdated sent while it
  was down is lost, as #305 does for plugin and AI changes. Unlike
  useServerChanges it skips the first connect, which only repeats the read
  the page made on load.
- Stop reloading the plugin output after a push: the plugins the push
  re-runs now announce themselves, and the reload put the panel through a
  loading state.
- Use #305's isChangeFor in place of the identical isUpdateFor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMPYZg3x4vXfFzqgvJ1AbU

---------

Co-authored-by: Claude <noreply@anthropic.com>
C-Hipple pushed a commit that referenced this pull request Oct 26, 2026
Brings in the live refresh (#304) and the pushed plugin/AI changes (#305).

Conflicts:
- CLAUDE.md: kept main's notification and live-refresh text, with this
  branch's comment positions on filtered diffs in place of "a filtered
  diff doesn't share" them / "skips inline comments while one is displayed".
- Review.tsx: kept main's unlessSame / sameFilteredDiff helpers and this
  branch's LINE_NOT_IN_FULL_DIFF; COMMENTS_NEED_FULL_DIFF and NO_COMMENTS
  are gone, as nothing uses them since comments show on filtered diffs.

main's live_updates e2e specs counted one file in #42's filtered diff and
pushed a change to a line it no longer shows. The fake's filtered diff now
takes a PR's ReplaceInDiff pushes as the full diff does (both end at the
head), and the specs count its two files and push a line both diffs show.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BSunxLHtpVEQmGaGyiP217
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