Skip to content

feat(cpl-267): document sovereign-mode cache staleness + listener-lag banner#566

Open
GTC6244 wants to merge 7 commits into
mainfrom
santo-domingo
Open

feat(cpl-267): document sovereign-mode cache staleness + listener-lag banner#566
GTC6244 wants to merge 7 commits into
mainfrom
santo-domingo

Conversation

@GTC6244

@GTC6244 GTC6244 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Documents the read-after-write cache staleness window for CPL-267: each API-server instance serves reads from a per-instance cache refreshed by an event listener polling the chain ~every 10s, so after an on-chain write an instance didn't originate (notably ChainSecured/sovereign wallet writes) its reads can be stale for up to one poll interval — this is now spelled out in the SDK sovereign-mode reference (account_modes.mdx), the core_sdk.js header, and the dashboard user docs. The listener now tracks how far behind the chain head it is and exposes it as a new account_event_listener_lag_seconds field on the unauthenticated GET /health (informational; does not affect health status). The dashboard polls /health and shows an amber staleness banner whenever a connected instance reports more than 30s of lag. New unit tests cover the lag semantics (None before first poll → small → large) and the always-present /health field.

🤖 Generated with Claude Code

… banner

Each API-server instance serves reads from a per-instance cache refreshed by
an event listener polling the chain ~every 10s, so after an on-chain write an
instance didn't originate (notably ChainSecured wallet writes) its reads can be
stale for up to one poll interval. Document this read-after-write window in the
SDK sovereign-mode reference, the core_sdk.js header, and the dashboard user
docs. Expose the listener's lag via a new account_event_listener_lag_seconds
field on GET /health, and add a dashboard banner that warns when a connected
instance reports more than 30s of lag.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@GTC6244
GTC6244 requested review from a team and Copilot July 10, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds end-user and SDK documentation for the API server’s read-after-write cache staleness in sovereign mode, and introduces a listener-lag signal surfaced via /health that the dashboard uses to warn when an instance’s cached reads may be meaningfully out of date.

Changes:

  • Add account_event_listener_lag_seconds to the unauthenticated health response, derived from the account-event listener’s catch-up tracking.
  • Add a dashboard poller + UI banner that warns when the connected instance reports >30s listener lag.
  • Document cache staleness semantics and the new lag field in management docs and the SDK header.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lit-static/dapps/dashboard/styles.css Adds --warning theme color and styles for the new listener-lag banner.
lit-static/dapps/dashboard/listener_lag.js New module that polls health and toggles the listener-lag staleness banner.
lit-static/dapps/dashboard/index.html Adds the banner container with appropriate live-region attributes.
lit-static/dapps/dashboard/app.js Starts the listener-lag monitor on auth-ready.
lit-static/core_sdk.js Documents read-after-write cache staleness and points to /health lag field.
lit-api-server/src/core/v1/health.rs Adds account_event_listener_lag_seconds to /health JSON and tests for key presence.
lit-api-server/src/account_events.rs Introduces lag tracking via a process-global timestamp updated by the listener.
docs/management/dashboard.mdx Documents dashboard-visible staleness scenarios and the amber banner meaning.
docs/management/account_modes.mdx Documents the read-after-write staleness window and the /health lag field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +11 to +14
* `GET /health` reports `account_event_listener_lag_seconds`. On a healthy
* instance this stays at or below the poll interval; when it climbs, the
* listener is stalled or dead and reads may be meaningfully out of date. We poll
* /health and show a banner once the lag crosses LAG_THRESHOLD_SECS.
Comment on lines 310 to +314
// Only advance once logs are successfully processed; on error we
// retry the same range on the next tick.
last_checked_block = latest_block;
// Processed everything up to the head we observed this tick.
mark_caught_up();
@mintlify

mintlify Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
chipotle 🟢 Ready View Preview Jul 10, 2026, 7:12 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

# Conflicts:
#	lit-static/dapps/dashboard/app.js
@mintlify

mintlify Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
chipotle-dev 🟢 Ready View Preview Jul 10, 2026, 7:26 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

GTC6244 and others added 2 commits July 10, 2026 16:02
cargo-deny 0.20 moved `--config` from a `check` subcommand flag to a
top-level option, so the unpinned `taiki-e/install-action` (now resolving
to 0.20.2) rejects the existing `cargo deny check --config ...` step with
"unexpected argument '--config'", failing every rust-ci matrix job. Pin to
the last 0.19.x, which accepts the current invocation, until the command is
migrated to the new CLI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	.github/workflows/rust-ci.yml
@clawdbot-glitch003

Copy link
Copy Markdown
Collaborator

🤖 Code review (Claude)

Summary: This PR documents a read-after-write staleness window for sovereign-mode writes (in docs/management/account_modes.mdx, docs/management/dashboard.mdx, and the core_sdk.js header), adds a LAST_CAUGHT_UP_UNIX_SECS atomic to the account-event listener and exposes it as account_event_listener_lag_seconds on GET /health (informational only), and adds a dashboard module (listener_lag.js) that polls /health every 30s and shows an amber banner when lag exceeds 30s. Unit tests cover the lag semantics and the always-present health field.

Findings

1. The central documented claim is factually wrong: list reads are not cached. (docs/management/account_modes.mdx "Which reads are affected", docs/management/dashboard.mdx "When data looks out of date", lit-static/core_sdk.js header)

The docs state that API-mode reads (listApiKeys, listGroups, listWallets, listActions, ...) "go through the server cache and are subject to this window," and the dashboard doc says the server "serves those reads from a short-lived cache" refreshed every ~10s. That's not what the code does. Every list_* handler does a live eth_call against the contract on every request with no cache in the path:

  • lit-api-server/src/accounts/mod.rs:528-628list_groups, list_wallets, list_wallets_in_group, list_actions, list_actions_in_group, list_api_keys all call get_read_only_account_config_contract() and hit the chain directly; none consult blockchain_cache.
  • lit-api-server/src/core/account_management.rs:725-830 — the layer above adds no caching either.

The per-instance cache (lit-api-server/src/accounts/blockchain_cache.rs) holds permission-check results (can_execute_action, can_use_wallet_in_action, can_execute_action_and_use_wallet) and wallet-derivation lookups (accounts/mod.rs:503, 686, 726, 760) — the things consulted on the action-execution path. So the real staleness story is: after a sovereign write, action-execution authorization decisions (e.g. a newly granted or revoked group/action/wallet permission) can be stale for up to one poll interval / TTL — not that listGroups shows the old table. Dashboard tables read live chain state (modulo RPC-node lag) and are essentially fresh. The docs, the dashboard "When data looks out of date" section, and the core_sdk.js header all need rewriting around the actual cached surface, otherwise we're documenting a failure mode that doesn't exist while missing the one that does.

2. Wrong endpoint path in docs: GET /health is actually GET /core/v1/health. (docs/management/account_modes.mdx Note block; also the doc comment on listener_lag_seconds() in account_events.rs)

Health routes are mounted under /core/v1/ (lit-api-server/src/main.rs:406); there is no top-level /health. A user following the doc and curling /health gets a 404. The new dashboard code uses the correct path (listener_lag.js fetches ${baseUrl}/core/v1/health), so this is a docs/comment fix.

3. Staleness when the listener dies is bounded by TTL, not open-ended. (docs/management/account_modes.mdx Note: "reads may be substantially out of date until it recovers or the process restarts")

Cache entries expire on their own: 300s for positive results, 30s for negative (blockchain_cache.rs:43, :50). The listener shrinks the window from the TTL to ~one poll interval; a dead listener means staleness regresses to at most ~5 minutes, not indefinite. Related wording nit: the listener invalidates the cache (bumps generations, per the account_events.rs module header and process_logs) rather than "refreshes" it — "refreshed by an event listener" appears in all three doc surfaces.

4. Lag metric reports "caught up" after a restart that skipped events. (lit-api-server/src/account_events.rs, mark_caught_up() at listener start)

On (re)start, run_event_listener sets start_block to the current head and never scans blocks between the previous crash and the restart — events in that gap are dropped and covered only by TTL expiry (pre-existing behavior). The new mark_caught_up() at startup immediately reports lag ≈ 0, so /health says "current" even though events were just missed. This matches the field's literal definition ("current with head"), but the doc comment sells it as a staleness signal; a one-line caveat there would prevent ops from over-trusting it after a listener restart.

Nits

  • EVENT_POLL_INTERVAL is made pub (account_events.rs) but nothing outside the module uses it — the intra-doc links to it are same-module. Can stay private.
  • listener_lag.js: the monitor never stops on logout; it polls /health every 30s forever once started. Harmless (unauthenticated endpoint, banner sits in the authed layout), but a stopListenerLagMonitor() on sign-out would be tidy.
  • listener_lag.js checkOnce(): fetch has no timeout, and setInterval can stack overlapping in-flight requests against a slow server. Practically harmless at 30s cadence.
  • The test listener_lag_reports_none_until_first_poll_then_small mutates a process-global static; it's currently the only test that writes it and the health test deliberately asserts only key presence (good comment there), but any future test touching LAST_CAUGHT_UP_UNIX_SECS will race. Consider a #[serial] or a note on the static.

Verdict

Needs changes. The Rust lag-tracking and the dashboard banner are solid and correctly wired, but the documentation — the stated point of the PR — misattributes the staleness window to list reads, which are uncached and read the chain live. The docs should be rewritten around the actual cached surface (permission checks and wallet-derivation on the execute path), plus the /core/v1/health path fix.

Address the Claude PR review: the docs incorrectly claimed list reads
(listApiKeys/listGroups/listWallets/listActions) go through the per-instance
cache. They don't — every list_* handler does a live contract call, so
list/read endpoints (and the dashboard's tables) always reflect chain head.
The cache sits on the action-execution path (authorization decisions +
wallet-derivation lookups). Rewrite account_modes.mdx, dashboard.mdx, the
core_sdk.js header, the dashboard banner copy, and the Rust doc comments to
describe the actual cached surface: after an unobserved on-chain permission
change, execute-path authorization can be stale for up to one poll interval
(~10s) while the listener is healthy, or up to the cache TTL (30s denials /
5min grants) if it is stalled — never open-ended.

Also: use the canonical /core/v1/health path in client-facing docs (the root
/health mount still exists for NLB probes); note the lag metric resets on
listener restart (which skips events); revert EVENT_POLL_INTERVAL to private;
and document the process-global test-race constraint on the lag static.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@GTC6244

GTC6244 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review — the central finding was correct and I've pushed 5d7464cb to address it. Summary of what changed and what I deliberately left.

Addressed

Finding 1 — docs misattributed the staleness to list reads (the important one). Confirmed: every list_* handler in accounts/mod.rs does a live .call() on the read-only contract with no cache in the path; blockchain_cache only holds execute-path authorization decisions (can_execute_action, can_use_wallet_in_action, ...) and wallet-derivation lookups. Rewrote all four doc surfaces (account_modes.mdx, dashboard.mdx, core_sdk.js header, and the dashboard banner copy) around the real cached surface: list/read endpoints and the dashboard tables are live and never stale; the window bites on the execute path — after an unobserved on-chain permission change, a just-granted permission can still 403 or a just-revoked one still succeed when running an action.

Finding 3 — dead-listener staleness is TTL-bounded, not open-ended. Fixed. Docs now state the window is at most ~one poll interval (~10s) while the listener is healthy, falling back to the cache TTL (30s denials / 5min grants) if it stalls — explicitly "never open-ended." Also switched "refreshes" → "invalidates" throughout.

Finding 4 — lag resets on restart, which skips events. Added a caveat to the listener_lag_seconds() doc and the account_modes.mdx Note: the metric is time-since-last-poll (a liveness signal), not proof no event was missed, since a restart resets it to ~0 and skips events emitted while down (TTL covers those).

Finding 2 — health path. Partial correction: /health does not 404 — it's mounted at both / (line 403) and /core/v1/ (line 406) in main.rs. But you're right that client-facing docs should use the canonical path, so all consumer-facing references now say /core/v1/health (matching the dashboard code). The root /health mention that remains in health.rs is the NLB probe's own module doc, which is correct in context.

Nits: reverted EVENT_POLL_INTERVAL to private (removed the intra-doc link that motivated pub); documented the process-global test-race constraint on LAST_CAUGHT_UP_UNIX_SECS. Also fixed the Copilot inline note about the /health vs /core/v1/health comment mismatch in listener_lag.js.

Intentionally not changed

  • Copilot inline Simplification: OTLP Sidecar Observability #2 / part of Finding 4 — re-sample the head after process_logs so the metric reflects true behind-head lag. Left as-is by design and documented instead. The metric backs a 30s banner threshold (3 poll intervals); sub-interval drift while eth_getLogs/process_logs runs is well within tolerance, and re-sampling would add an RPC round-trip every poll for accuracy the consumer doesn't need. The doc comment now frames it as a coarse liveness/lag signal.
  • listener_lag.js polling nits (no stopListenerLagMonitor() on logout; no fetch timeout; possible overlapping in-flight requests at the 30s cadence). Harmless as the reviewer notes — the endpoint is unauthenticated and the banner lives in the authed layout — so I left the module minimal rather than add lifecycle wiring.

Verified after the changes: cargo +1.91 clippy --all-features -- -D warnings clean, cargo fmt --check clean, account_events + health unit tests pass, and the JS parses.

# Conflicts:
#	lit-api-server/src/core/v1/health.rs
pool_warm_hit and pool_memory_limit_bypass intermittently failed in CI with
"no pre-warmed worker became ready within 15s". Root cause: every
TestServer::start() warms LIT_ACTIONS_POOL_SIZE workers (production default
10), and ~a dozen server-spawning integration tests run in parallel — a
~120-way concurrent V8-snapshot bootstrap storm that, on a loaded/constrained
runner, starves the warmup gate so not even one worker becomes ready in time.
Reproduced locally by amplifying the storm (LIT_ACTIONS_POOL_SIZE=64 →
identical panic on exactly those two warmup-gated tests, while
pool_concurrent_exhaustion, which tolerates a cold pool, passes).

Cap the pool to 2 for the test process via the existing #[ctor] (respecting an
explicit LIT_ACTIONS_POOL_SIZE override, incl. 0 to disable), cutting the storm
~5x while still exercising pool hits/refills/fallbacks. Also bump the warmup
deadline 15s→30s for extra CI margin. Production DEFAULT_POOL_SIZE is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

3 participants