Skip to content

fix(hub): keep notify-cause ingest bounded by advancing the resume watermark - #1844

Open
miuiadmin wants to merge 1 commit into
tiann:mainfrom
miuiadmin:fix/work-graph-notify-cause-watermark
Open

miuiadmin wants to merge 1 commit into
tiann:mainfrom
miuiadmin:fix/work-graph-notify-cause-watermark

Conversation

@miuiadmin

Copy link
Copy Markdown
Contributor

Problem

On a self-hosted hub, automation-style sessions — a CLI runner that turns for hours without new user inbounds and ends every assistant message with an AGENT_NOTIFY_SUMMARY footer — progressively stall the hub. Each notify blocks the event loop for seconds and issues thousands of SQLite page reads; /health latency grows with session length. Restarting the hub does not help, and trimming old messages only relieves it temporarily (trim shortens the tail), which is what made it look like a data-volume problem.

Root cause

workGraphNotifyIngest re-derives the work_ad cause on every notify-bearing assistant message. When a notify arrives with no new invoked inbound, it takes the sticky path: copy the previous event's cause verbatim. That copy also carries the resume point (causeSeq / causeCursorMessageId), which therefore stays frozen at the original inbound.

Every subsequent notify then loads and decodes the whole session tail since that inbound (getMessagesAfterSeq(frozenSeq)), and message content is zstd-decompressed + JSON-parsed per row. Cost is proportional to session length, unbounded, and repeats on every notify: on a ~10k-message session whose last consumed inbound sits at seq ~900 (v0.29.1), one footer = ~4.7s event-loop stall and ~5000 page reads, again and again.

A second, smaller gap: if retention trims away the cursor message row while the event ledger row survives, getSeqById returns null and the loader degrades to getAllMessages — the same full-read cost class on every later notify.

Fix

  • Sticky advance (resolveWorkAdCause): keep the cause identity (messageId/text/kind) but advance the resume point past the current assistant message — unless an unconsumed queued or unmatured-scheduled inbound still sits below the watermark. Those can mature into a later cause and must stay inside the scan window (the existing queue-maturation and seq-shift tests protect exactly this).
  • Trimmed-cursor fallback (loadMessagesForCause): when the cursor row was removed by retention, trust the numeric causeSeq watermark only while every remaining row sits above it (new getMinSeq check). A restarted seq space (source session of a mergeSessionHistory move) still selects the full read of what is then a small session.
  • Seq lookup by id (ingestNotifySummaryFromMessage): resolve the assistant seq via getSeqById instead of scanning the loaded window, since after the watermark advance the window no longer contains the notify-bearing row.

Measurements

Same box, same frozen-cursor session (~10k messages, last consumed inbound at seq 891):

before (v0.29.1) after (this PR)
notify #1 /health 4756 ms, 5073 page reads 4591 ms (one-time catch-up scan of the backlog)
notify #2 /health 4756 ms, 5073 page reads <100 ms (no stall detected)
notify #3 same on every notify 155 ms

The per-notify cost collapses from whole-tail re-parse to a single catch-up scan, after which the watermark tracks the session head.

Tests

  • 4 new tests in workGraphNotifyIngest.test.ts: watermark advance keeps sticky cause identity; a sticky chain resumes each load after the previous watermark instead of the original inbound; a retention-trimmed cursor row still loads incrementally via the numeric watermark; legacy rows without causeSeq/cursor still full-read and consume correctly.
  • Full hub suite passes (1291 pass / 3 skip); bun typecheck clean.

AI disclosure

Per the AI-Generated Code Policy: this change was developed with AI assistance (Claude Code agent, powered by GLM by Z.ai) and validated against the existing hub test suite, which is what forced the queue-maturation and merge-safety guards.

🤖 Generated with Claude Code

…ounded

Every assistant message that ends with an AGENT_NOTIFY_SUMMARY footer
re-ingests the work_ad cause from the session message store. When a
session runs many turns without a new invoked inbound (automation-style
runners), the sticky path copied the previous cause verbatim, which also
froze the resume point (causeSeq + causeCursorMessageId) at the original
inbound. Each subsequent notify then re-loaded and re-parsed the entire
session tail since that inbound — content decode is zstd + JSON.parse
per row, so the cost grows without bound as the session grows and
blocks the hub event loop for seconds per notify.

Measured on a self-hosted hub with a ~10k-message session whose last
consumed inbound sat at seq 891 (v0.29.1): every notify footer stalled
/health for ~4.7s and cost ~5000 page reads, repeating per notify. With
this change the first notify pays a one-time catch-up scan; the resume
point then advances past each assistant turn and subsequent notifies
complete in the low milliseconds.

- resolveWorkAdCause: on the sticky path, advance causeSeq and
  causeCursorMessageId to the current assistant message while keeping
  the cause identity (messageId/text/kind). The advance is skipped
  while unconsumed queued or unmatured-scheduled inbounds still sit
  below the watermark — those can mature into a later cause and must
  stay inside the scan window.
- loadMessagesForCause: when the cursor row was trimmed away by
  retention, resume after the numeric causeSeq watermark (validated
  against the session's current minimum seq) instead of degrading to a
  full session read on every later notify. A history move
  (mergeSessionHistory) empties the session and restarts its seq
  space, so a stale low watermark still selects the full read there.
- ingestNotifySummaryFromMessage: resolve the assistant seq by message
  id instead of scanning the loaded window, which after the watermark
  advance no longer contains the notify-bearing row.
- store: add getMinSeq helper for the watermark validation.

Co-Authored-By: HAPI <noreply@hapi.run>
@heavygee heavygee added bug Something isn't working area:hub Hub server (API, sync, store) community-pr PR from non-collaborator contributor labels Sep 13, 2026

@github-actions github-actions 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.

Review mode: initial

Requirement — Pass

Avoiding repeated decoding of a growing message tail addresses a supported hub responsiveness problem in long-running automation sessions.

Evidence

  • The PR describes repeated notification stalls when no new inbound prompt arrives.
  • Base hub/src/sync/workGraphNotifyIngest.ts:349 copies the previous cause unchanged; loadMessagesForCause resumes from that retained cursor.
  • hub/src/store/messages.ts:336 loads and decodes every message after the supplied sequence.

Approach — Pass

Advancing the resume cursor while preserving cause identity addresses the repeated tail scan. The waiting-inbound guard preserves later attribution, and cursor-ID lookup accommodates sequence shifts. Pending inbounds intentionally keep the scan window open.

Evidence

  • hub/src/sync/workGraphNotifyIngest.ts:339 detects queued or future-scheduled inbounds before advancing the sticky cursor at line 399.
  • hub/src/sync/workGraphNotifyIngest.ts:234 prefers the cursor's current sequence and conservatively checks the minimum remaining sequence before using a missing cursor's numeric watermark.
  • hub/src/sync/workGraphNotifyIngest.ts:529 resolves the assistant sequence independently of the loaded window.

Code — Reviewed

Reviewed the entire four-file merge-base diff and relevant storage, queue, and merge behavior. No actionable defects introduced by this PR were identified. Runtime performance measurements were not independently reproduced.

No reportable code issues found.

Testing

Not run (automation; PR code execution is prohibited).

  • The Test workflow succeeded for fixed head bb24e6b; test, integration, and windows-codex-mcp checks passed.
  • Four added tests cover sticky watermark advancement, successive incremental loads, a retention-trimmed cursor, and legacy cause metadata. Existing tests cover queued-message maturation and session merge behavior.
  • No PR code, tests, builds, or scripts were executed during this read-only review.

HAPI Bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:hub Hub server (API, sync, store) bug Something isn't working community-pr PR from non-collaborator contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants